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

Vladimir Sementsov-Ogievskiy posted 10 patches 1 week, 2 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 v2 09/10] chardev: rework filename handling
Posted by Vladimir Sementsov-Ogievskiy 1 week, 2 days ago
We have the following flaws with it:

1. Layring violation by modifying generic state directly in backends

2. Tricky generic logic: we should check, did backend set the
generic state field, and fill it when not.

Let's fix them all by making filename a private field with getter
and setter. And move the "default logic" into getter.

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

diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index 047aade09e..f4294679be 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -336,6 +336,7 @@ static bool pty_chr_open(Chardev *chr, ChardevBackend *backend, Error **errp)
     int master_fd, slave_fd;
     char *name;
     char *path = backend->u.pty.data->path;
+    g_autofree char *filename = NULL;
 
     s = PTY_CHARDEV(chr);
 
@@ -351,7 +352,8 @@ static bool pty_chr_open(Chardev *chr, ChardevBackend *backend, Error **errp)
         return false;
     }
 
-    chr->filename = g_strdup_printf("pty:%s", s->pty_name);
+    filename = g_strdup_printf("pty:%s", s->pty_name);
+    qemu_chr_set_filename(chr, filename);
     qemu_printf("char device redirected to %s (label %s)\n",
                 s->pty_name, chr->label);
 
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 31c9acd164..9387760009 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -384,8 +384,7 @@ 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;
+    qemu_chr_set_filename(chr, NULL);
     tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
 }
 
@@ -443,11 +442,11 @@ static void update_disconnected_filename(SocketChardev *s)
 {
     Chardev *chr = CHARDEV(s);
 
-    g_free(chr->filename);
     if (s->addr) {
-        chr->filename = qemu_chr_socket_address(s, "disconnected:");
+        g_autofree char *filename = qemu_chr_socket_address(s, "disconnected:");
+        qemu_chr_set_filename(chr, filename);
     } else {
-        chr->filename = g_strdup("disconnected:socket");
+        qemu_chr_set_filename(chr, "disconnected:socket");
     }
 }
 
@@ -638,9 +637,9 @@ static void tcp_chr_connect(void *opaque)
 {
     Chardev *chr = CHARDEV(opaque);
     SocketChardev *s = SOCKET_CHARDEV(opaque);
+    g_autofree char *filename = qemu_chr_compute_filename(s);
 
-    g_free(chr->filename);
-    chr->filename = qemu_chr_compute_filename(s);
+    qemu_chr_set_filename(chr, filename);
 
     tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTED);
     update_ioc_handlers(s);
@@ -1000,8 +999,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);
diff --git a/chardev/char.c b/chardev/char.c
index 0dc792b88f..bdd907f015 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -309,7 +309,7 @@ static void char_finalize(Object *obj)
     if (chr->fe) {
         chr->fe->chr = NULL;
     }
-    g_free(chr->filename);
+    qemu_chr_set_filename(chr, NULL);
     g_free(chr->label);
     if (chr->logfd != -1) {
         close(chr->logfd);
@@ -796,7 +796,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 +1025,6 @@ static Chardev *chardev_new(const char *id, const char *typename,
         return NULL;
     }
 
-    if (!chr->filename) {
-        chr->filename = g_strdup(typename + 8);
-    }
-
     return chr;
 }
 
diff --git a/hw/misc/ivshmem-pci.c b/hw/misc/ivshmem-pci.c
index 636d0b83de..2c7b987241 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 d36e50b99e..ffeb4a4e3b 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -62,7 +62,7 @@ struct Chardev {
     QemuMutex chr_write_lock;
     CharFrontend *fe;
     char *label;
-    char *filename;
+    char *_filename;
     int logfd;
     int be_open;
     /* used to coordinate the chardev-change special-case: */
@@ -72,6 +72,25 @@ struct Chardev {
     DECLARE_BITMAP(features, QEMU_CHAR_FEATURE_LAST);
 };
 
+static inline char *qemu_chr_get_filename(Chardev *chr)
+{
+    const char *typename;
+
+    if (chr->_filename) {
+        return g_strdup(chr->_filename);
+    }
+
+    typename = object_get_typename(OBJECT(chr));
+    assert(g_str_has_prefix(typename, "chardev-"));
+    return g_strdup(typename + 8);
+}
+
+static inline void qemu_chr_set_filename(Chardev *chr, const char *filename)
+{
+    g_free(chr->_filename);
+    chr->_filename = g_strdup(filename);
+}
+
 /**
  * qemu_chr_new_from_opts:
  * @opts: see qemu-config.c for a list of valid options
-- 
2.48.1
Re: [PATCH v2 09/10] chardev: rework filename handling
Posted by Marc-André Lureau 1 week, 1 day ago
Hi

On Thu, Dec 4, 2025 at 7:42 PM Vladimir Sementsov-Ogievskiy <
vsementsov@yandex-team.ru> wrote:

> We have the following flaws with it:
>
> 1. Layring violation by modifying generic state directly in backends
>
>
Well, it's a parent field that can be modified, I am not sure it qualifies
as such


> 2. Tricky generic logic: we should check, did backend set the
> generic state field, and fill it when not.
>
> Let's fix them all by making filename a private field with getter
> and setter. And move the "default logic" into getter.
>
>
The tradeoff is that your implementation will do more allocation/free, but
I don't think we care much here.

I am not sure we gain much overall.


> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

---
>  chardev/char-pty.c     |  4 +++-
>  chardev/char-socket.c  | 17 ++++++++---------
>  chardev/char.c         |  8 ++------
>  hw/misc/ivshmem-pci.c  |  4 ++--
>  include/chardev/char.h | 21 ++++++++++++++++++++-
>  5 files changed, 35 insertions(+), 19 deletions(-)
>
> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> index 047aade09e..f4294679be 100644
> --- a/chardev/char-pty.c
> +++ b/chardev/char-pty.c
> @@ -336,6 +336,7 @@ static bool pty_chr_open(Chardev *chr, ChardevBackend
> *backend, Error **errp)
>      int master_fd, slave_fd;
>      char *name;
>      char *path = backend->u.pty.data->path;
> +    g_autofree char *filename = NULL;
>
>      s = PTY_CHARDEV(chr);
>
> @@ -351,7 +352,8 @@ static bool pty_chr_open(Chardev *chr, ChardevBackend
> *backend, Error **errp)
>          return false;
>      }
>
> -    chr->filename = g_strdup_printf("pty:%s", s->pty_name);
> +    filename = g_strdup_printf("pty:%s", s->pty_name);
> +    qemu_chr_set_filename(chr, filename);
>      qemu_printf("char device redirected to %s (label %s)\n",
>                  s->pty_name, chr->label);
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 31c9acd164..9387760009 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -384,8 +384,7 @@ 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;
> +    qemu_chr_set_filename(chr, NULL);
>      tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
>  }
>
> @@ -443,11 +442,11 @@ static void
> update_disconnected_filename(SocketChardev *s)
>  {
>      Chardev *chr = CHARDEV(s);
>
> -    g_free(chr->filename);
>      if (s->addr) {
> -        chr->filename = qemu_chr_socket_address(s, "disconnected:");
> +        g_autofree char *filename = qemu_chr_socket_address(s,
> "disconnected:");
> +        qemu_chr_set_filename(chr, filename);
>      } else {
> -        chr->filename = g_strdup("disconnected:socket");
> +        qemu_chr_set_filename(chr, "disconnected:socket");
>      }
>  }
>
> @@ -638,9 +637,9 @@ static void tcp_chr_connect(void *opaque)
>  {
>      Chardev *chr = CHARDEV(opaque);
>      SocketChardev *s = SOCKET_CHARDEV(opaque);
> +    g_autofree char *filename = qemu_chr_compute_filename(s);
>
> -    g_free(chr->filename);
> -    chr->filename = qemu_chr_compute_filename(s);
> +    qemu_chr_set_filename(chr, filename);
>
>      tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTED);
>      update_ioc_handlers(s);
> @@ -1000,8 +999,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);
> diff --git a/chardev/char.c b/chardev/char.c
> index 0dc792b88f..bdd907f015 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -309,7 +309,7 @@ static void char_finalize(Object *obj)
>      if (chr->fe) {
>          chr->fe->chr = NULL;
>      }
> -    g_free(chr->filename);
> +    qemu_chr_set_filename(chr, NULL);
>      g_free(chr->label);
>      if (chr->logfd != -1) {
>          close(chr->logfd);
> @@ -796,7 +796,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 +1025,6 @@ static Chardev *chardev_new(const char *id, const
> char *typename,
>          return NULL;
>      }
>
> -    if (!chr->filename) {
> -        chr->filename = g_strdup(typename + 8);
> -    }
> -
>      return chr;
>  }
>
> diff --git a/hw/misc/ivshmem-pci.c b/hw/misc/ivshmem-pci.c
> index 636d0b83de..2c7b987241 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);
>

Should be auto-free, since returned allocated


>          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 d36e50b99e..ffeb4a4e3b 100644
> --- a/include/chardev/char.h
> +++ b/include/chardev/char.h
> @@ -62,7 +62,7 @@ struct Chardev {
>      QemuMutex chr_write_lock;
>      CharFrontend *fe;
>      char *label;
> -    char *filename;
> +    char *_filename;
>

Why rename the field? we don't have a  convention to have "private" fields
with _ prefix afaik.


>      int logfd;
>      int be_open;
>      /* used to coordinate the chardev-change special-case: */
> @@ -72,6 +72,25 @@ struct Chardev {
>      DECLARE_BITMAP(features, QEMU_CHAR_FEATURE_LAST);
>  };
>
> +static inline char *qemu_chr_get_filename(Chardev *chr)
>

Let's avoid code in headers.


> +{
> +    const char *typename;
> +
> +    if (chr->_filename) {
> +        return g_strdup(chr->_filename);
> +    }
> +
> +    typename = object_get_typename(OBJECT(chr));
> +    assert(g_str_has_prefix(typename, "chardev-"));
> +    return g_strdup(typename + 8);
> +}
> +
> +static inline void qemu_chr_set_filename(Chardev *chr, const char
> *filename)
> +{
> +    g_free(chr->_filename);
> +    chr->_filename = g_strdup(filename);
> +}
> +
>  /**
>   * qemu_chr_new_from_opts:
>   * @opts: see qemu-config.c for a list of valid options
> --
> 2.48.1
>
>
Re: [PATCH v2 09/10] chardev: rework filename handling
Posted by Vladimir Sementsov-Ogievskiy 1 week, 1 day ago
On 05.12.25 18:33, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Dec 4, 2025 at 7:42 PM Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru <mailto:vsementsov@yandex-team.ru>> wrote:
> 
>     We have the following flaws with it:
> 
>     1. Layring violation by modifying generic state directly in backends
> 
> 
> Well, it's a parent field that can be modified, I am not sure it qualifies as such
> 
>     2. Tricky generic logic: we should check, did backend set the
>     generic state field, and fill it when not.
> 
>     Let's fix them all by making filename a private field with getter
>     and setter. And move the "default logic" into getter.
> 
> 
> The tradeoff is that your implementation will do more allocation/free, but I don't think we care much here.
> 
> I am not sure we gain much overall.


My actual goal is to "open the doors" for further movement of qemu_char_open() call
to the later point (I came to this idea instead of trying to split .open() into
.open() and .init())..

So I should do something with the logic in chardev_new(), done after qemu_char_open()
call. Alternative is to put it into qemu_char_open(). But I decided, that being here,
it's better to generalize it somehow.

Agree, that setter/getter for filename seems a bit cumbersome here.

Hmm. I think, I have a clearer idea:

add .chr_get_filename() handler and drop the .filename field and qemu_chr_set_filename()
function.

So, qemu_chr_get_filename() will realize the default logic or call .chr_get_filename() if
it exist.

We'll need implementations only for char-socket and char-pty.


> 
>     Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru <mailto:vsementsov@yandex-team.ru>> 
> 
>     ---
>       chardev/char-pty.c     |  4 +++-
>       chardev/char-socket.c  | 17 ++++++++---------
>       chardev/char.c         |  8 ++------
>       hw/misc/ivshmem-pci.c  |  4 ++--
>       include/chardev/char.h | 21 ++++++++++++++++++++-
>       5 files changed, 35 insertions(+), 19 deletions(-)
> 
>     diff --git a/chardev/char-pty.c b/chardev/char-pty.c
>     index 047aade09e..f4294679be 100644
>     --- a/chardev/char-pty.c
>     +++ b/chardev/char-pty.c
>     @@ -336,6 +336,7 @@ static bool pty_chr_open(Chardev *chr, ChardevBackend *backend, Error **errp)
>           int master_fd, slave_fd;
>           char *name;
>           char *path = backend->u.pty.data->path;
>     +    g_autofree char *filename = NULL;
> 
>           s = PTY_CHARDEV(chr);
> 
>     @@ -351,7 +352,8 @@ static bool pty_chr_open(Chardev *chr, ChardevBackend *backend, Error **errp)
>               return false;
>           }
> 
>     -    chr->filename = g_strdup_printf("pty:%s", s->pty_name);
>     +    filename = g_strdup_printf("pty:%s", s->pty_name);
>     +    qemu_chr_set_filename(chr, filename);
>           qemu_printf("char device redirected to %s (label %s)\n",
>                       s->pty_name, chr->label);
> 
>     diff --git a/chardev/char-socket.c b/chardev/char-socket.c
>     index 31c9acd164..9387760009 100644
>     --- a/chardev/char-socket.c
>     +++ b/chardev/char-socket.c
>     @@ -384,8 +384,7 @@ 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;
>     +    qemu_chr_set_filename(chr, NULL);
>           tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
>       }
> 
>     @@ -443,11 +442,11 @@ static void update_disconnected_filename(SocketChardev *s)
>       {
>           Chardev *chr = CHARDEV(s);
> 
>     -    g_free(chr->filename);
>           if (s->addr) {
>     -        chr->filename = qemu_chr_socket_address(s, "disconnected:");
>     +        g_autofree char *filename = qemu_chr_socket_address(s, "disconnected:");
>     +        qemu_chr_set_filename(chr, filename);
>           } else {
>     -        chr->filename = g_strdup("disconnected:socket");
>     +        qemu_chr_set_filename(chr, "disconnected:socket");
>           }
>       }
> 
>     @@ -638,9 +637,9 @@ static void tcp_chr_connect(void *opaque)
>       {
>           Chardev *chr = CHARDEV(opaque);
>           SocketChardev *s = SOCKET_CHARDEV(opaque);
>     +    g_autofree char *filename = qemu_chr_compute_filename(s);
> 
>     -    g_free(chr->filename);
>     -    chr->filename = qemu_chr_compute_filename(s);
>     +    qemu_chr_set_filename(chr, filename);
> 
>           tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTED);
>           update_ioc_handlers(s);
>     @@ -1000,8 +999,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);
>     diff --git a/chardev/char.c b/chardev/char.c
>     index 0dc792b88f..bdd907f015 100644
>     --- a/chardev/char.c
>     +++ b/chardev/char.c
>     @@ -309,7 +309,7 @@ static void char_finalize(Object *obj)
>           if (chr->fe) {
>               chr->fe->chr = NULL;
>           }
>     -    g_free(chr->filename);
>     +    qemu_chr_set_filename(chr, NULL);
>           g_free(chr->label);
>           if (chr->logfd != -1) {
>               close(chr->logfd);
>     @@ -796,7 +796,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 +1025,6 @@ static Chardev *chardev_new(const char *id, const char *typename,
>               return NULL;
>           }
> 
>     -    if (!chr->filename) {
>     -        chr->filename = g_strdup(typename + 8);
>     -    }
>     -
>           return chr;
>       }
> 
>     diff --git a/hw/misc/ivshmem-pci.c b/hw/misc/ivshmem-pci.c
>     index 636d0b83de..2c7b987241 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);
> 
> 
> Should be auto-free, since returned allocated
> 
>               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 d36e50b99e..ffeb4a4e3b 100644
>     --- a/include/chardev/char.h
>     +++ b/include/chardev/char.h
>     @@ -62,7 +62,7 @@ struct Chardev {
>           QemuMutex chr_write_lock;
>           CharFrontend *fe;
>           char *label;
>     -    char *filename;
>     +    char *_filename;
> 
> 
> Why rename the field? we don't have a  convention to have "private" fields with _ prefix afaik.
> 
>           int logfd;
>           int be_open;
>           /* used to coordinate the chardev-change special-case: */
>     @@ -72,6 +72,25 @@ struct Chardev {
>           DECLARE_BITMAP(features, QEMU_CHAR_FEATURE_LAST);
>       };
> 
>     +static inline char *qemu_chr_get_filename(Chardev *chr)
> 
> 
> Let's avoid code in headers.
> 
>     +{
>     +    const char *typename;
>     +
>     +    if (chr->_filename) {
>     +        return g_strdup(chr->_filename);
>     +    }
>     +
>     +    typename = object_get_typename(OBJECT(chr));
>     +    assert(g_str_has_prefix(typename, "chardev-"));
>     +    return g_strdup(typename + 8);
>     +}
>     +
>     +static inline void qemu_chr_set_filename(Chardev *chr, const char *filename)
>     +{
>     +    g_free(chr->_filename);
>     +    chr->_filename = g_strdup(filename);
>     +}
>     +
>       /**
>        * qemu_chr_new_from_opts:
>        * @opts: see qemu-config.c for a list of valid options
>     -- 
>     2.48.1
> 


-- 
Best regards,
Vladimir