[PATCH v3 09/10] chardev: rework filename handling

Vladimir Sementsov-Ogievskiy posted 10 patches 3 weeks, 4 days ago
Maintainers: Samuel Thibault <samuel.thibault@ens-lyon.org>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Stefano Stabellini <sstabellini@kernel.org>, Anthony PERARD <anthony@xenproject.org>, Paul Durrant <paul@xen.org>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>
[PATCH v3 09/10] chardev: rework filename handling
Posted by Vladimir Sementsov-Ogievskiy 3 weeks, 4 days ago
Instead of checking, did backend set the filename state or not, let's
be stateless: filename is needed rarely, so, let's just have a generic
function (with optional implementation by backends) to get it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 chardev/char-pty.c     |  8 +++++++-
 chardev/char-socket.c  | 31 ++++++++++++-------------------
 chardev/char.c         | 22 ++++++++++++++++------
 hw/misc/ivshmem-pci.c  |  4 ++--
 include/chardev/char.h |  5 ++++-
 5 files changed, 41 insertions(+), 29 deletions(-)

diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index 047aade09e..7e95132cdb 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -351,7 +351,6 @@ static bool pty_chr_open(Chardev *chr, ChardevBackend *backend, Error **errp)
         return false;
     }
 
-    chr->filename = g_strdup_printf("pty:%s", s->pty_name);
     qemu_printf("char device redirected to %s (label %s)\n",
                 s->pty_name, chr->label);
 
@@ -393,6 +392,12 @@ static char *pty_chr_get_pty_name(Chardev *chr)
     return g_strdup(s->pty_name);
 }
 
+static char *pty_chr_get_filename(Chardev *chr)
+{
+    PtyChardev *s = PTY_CHARDEV(chr);
+    return g_strdup_printf("pty:%s", s->pty_name);
+}
+
 static void char_pty_class_init(ObjectClass *oc, const void *data)
 {
     ChardevClass *cc = CHARDEV_CLASS(oc);
@@ -403,6 +408,7 @@ static void char_pty_class_init(ObjectClass *oc, const void *data)
     cc->chr_update_read_handler = pty_chr_update_read_handler;
     cc->chr_add_watch = pty_chr_add_watch;
     cc->chr_get_pty_name = pty_chr_get_pty_name;
+    cc->chr_get_filename = pty_chr_get_filename;
 }
 
 static const TypeInfo char_pty_type_info = {
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 908bbdbc12..5adeb90865 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -40,6 +40,7 @@
 
 static gboolean socket_reconnect_timeout(gpointer opaque);
 static void tcp_chr_telnet_init(Chardev *chr);
+static char *qemu_chr_compute_filename(SocketChardev *s);
 
 static void tcp_chr_change_state(SocketChardev *s, TCPChardevState state)
 {
@@ -384,8 +385,6 @@ static void tcp_chr_free_connection(Chardev *chr)
     s->sioc = NULL;
     object_unref(OBJECT(s->ioc));
     s->ioc = NULL;
-    g_free(chr->filename);
-    chr->filename = NULL;
     tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
 }
 
@@ -439,16 +438,17 @@ static char *qemu_chr_socket_address(SocketChardev *s, const char *prefix)
     }
 }
 
-static void update_disconnected_filename(SocketChardev *s)
+static char *tcp_chr_get_filename(Chardev *chr)
 {
-    Chardev *chr = CHARDEV(s);
+    SocketChardev *s = SOCKET_CHARDEV(chr);
 
-    g_free(chr->filename);
-    if (s->addr) {
-        chr->filename = qemu_chr_socket_address(s, "disconnected:");
-    } else {
-        chr->filename = g_strdup("disconnected:socket");
+    if (s->state == TCP_CHARDEV_STATE_CONNECTED) {
+        return qemu_chr_compute_filename(s);
+    } else if (s->addr) {
+        return qemu_chr_socket_address(s, "disconnected:");
     }
+
+    return g_strdup("disconnected:socket");
 }
 
 /* NB may be called even if tcp_chr_connect has not been
@@ -468,7 +468,6 @@ static void tcp_chr_disconnect_locked(Chardev *chr)
         qio_net_listener_set_client_func_full(s->listener, tcp_chr_accept,
                                               chr, NULL, chr->gcontext);
     }
-    update_disconnected_filename(s);
     if (emit_close) {
         qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
     }
@@ -639,9 +638,6 @@ static void tcp_chr_connect(void *opaque)
     Chardev *chr = CHARDEV(opaque);
     SocketChardev *s = SOCKET_CHARDEV(opaque);
 
-    g_free(chr->filename);
-    chr->filename = qemu_chr_compute_filename(s);
-
     tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTED);
     update_ioc_handlers(s);
     qemu_chr_be_event(chr, CHR_EVENT_OPENED);
@@ -1000,8 +996,8 @@ static void tcp_chr_accept_server_sync(Chardev *chr)
 {
     SocketChardev *s = SOCKET_CHARDEV(chr);
     QIOChannelSocket *sioc;
-    info_report("QEMU waiting for connection on: %s",
-                chr->filename);
+    g_autofree char *filename = qemu_chr_get_filename(chr);
+    info_report("QEMU waiting for connection on: %s", filename);
     tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTING);
     sioc = qio_net_listener_wait_client(s->listener);
     tcp_chr_set_client_ioc_name(chr, sioc);
@@ -1258,8 +1254,6 @@ static int qmp_chardev_open_socket_server(Chardev *chr,
     s->addr = qio_net_listener_get_local_address(s->listener, 0, errp);
 
 skip_listen:
-    update_disconnected_filename(s);
-
     if (is_waitconnect) {
         tcp_chr_accept_server_sync(chr);
     } else {
@@ -1436,8 +1430,6 @@ static bool tcp_chr_open(Chardev *chr, ChardevBackend *backend, Error **errp)
     }
     s->registered_yank = true;
 
-    update_disconnected_filename(s);
-
     if (s->is_listen) {
         if (qmp_chardev_open_socket_server(chr, is_telnet || is_tn3270,
                                            is_waitconnect, errp) < 0) {
@@ -1593,6 +1585,7 @@ static void char_socket_class_init(ObjectClass *oc, const void *data)
     cc->chr_add_watch = tcp_chr_add_watch;
     cc->chr_update_read_handler = tcp_chr_update_read_handler;
     cc->chr_listener_cleanup = tcp_chr_listener_cleanup;
+    cc->chr_get_filename = tcp_chr_get_filename;
 
     object_class_property_add(oc, "addr", "SocketAddress",
                               char_socket_get_addr, NULL,
diff --git a/chardev/char.c b/chardev/char.c
index 0dc792b88f..ef0cdffd3f 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -309,7 +309,6 @@ static void char_finalize(Object *obj)
     if (chr->fe) {
         chr->fe->chr = NULL;
     }
-    g_free(chr->filename);
     g_free(chr->label);
     if (chr->logfd != -1) {
         close(chr->logfd);
@@ -796,7 +795,7 @@ static int qmp_query_chardev_foreach(Object *obj, void *data)
     ChardevInfo *value = g_malloc0(sizeof(*value));
 
     value->label = g_strdup(chr->label);
-    value->filename = g_strdup(chr->filename);
+    value->filename = qemu_chr_get_filename(chr);
     value->frontend_open = chr->fe && chr->fe->fe_is_open;
 
     QAPI_LIST_PREPEND(*list, value);
@@ -1025,10 +1024,6 @@ static Chardev *chardev_new(const char *id, const char *typename,
         return NULL;
     }
 
-    if (!chr->filename) {
-        chr->filename = g_strdup(typename + 8);
-    }
-
     return chr;
 }
 
@@ -1110,6 +1105,21 @@ char *qemu_chr_get_pty_name(Chardev *chr)
     return NULL;
 }
 
+char *qemu_chr_get_filename(Chardev *chr)
+{
+    ChardevClass *cc = CHARDEV_GET_CLASS(chr);
+    const char *typename;
+
+    if (cc->chr_get_filename) {
+        return cc->chr_get_filename(chr);
+    }
+
+    typename = object_get_typename(OBJECT(chr));
+    assert(g_str_has_prefix(typename, "chardev-"));
+    return g_strdup(typename + 8);
+}
+
+
 ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
                                   Error **errp)
 {
diff --git a/hw/misc/ivshmem-pci.c b/hw/misc/ivshmem-pci.c
index b9162589a0..bf05688065 100644
--- a/hw/misc/ivshmem-pci.c
+++ b/hw/misc/ivshmem-pci.c
@@ -873,10 +873,10 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
         host_memory_backend_set_mapped(s->hostmem, true);
     } else {
         Chardev *chr = qemu_chr_fe_get_driver(&s->server_chr);
+        char *filename = qemu_chr_get_filename(chr);
         assert(chr);
 
-        IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n",
-                        chr->filename);
+        IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n", filename);
 
         /* we allocate enough space for 16 peers and grow as needed */
         resize_peers(s, 16);
diff --git a/include/chardev/char.h b/include/chardev/char.h
index ada5529fa6..81bc0cbdf2 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -62,7 +62,6 @@ struct Chardev {
     QemuMutex chr_write_lock;
     CharFrontend *fe;
     char *label;
-    char *filename;
     int logfd;
     int be_open;
     /* used to coordinate the chardev-change special-case: */
@@ -309,6 +308,9 @@ struct ChardevClass {
 
     /* return PTY name if available */
     char *(*chr_get_pty_name)(Chardev *s);
+
+    /* get filename for reporting */
+    char *(*chr_get_filename)(Chardev *s);
 };
 
 Chardev *qemu_chardev_new(const char *id, const char *typename,
@@ -324,5 +326,6 @@ void suspend_mux_open(void);
 void resume_mux_open(void);
 
 char *qemu_chr_get_pty_name(Chardev *chr);
+char *qemu_chr_get_filename(Chardev *chr);
 
 #endif
-- 
2.52.0