Hi
On Thu, Dec 4, 2025 at 7:42 PM Vladimir Sementsov-Ogievskiy <
vsementsov@yandex-team.ru> wrote:
> Currently we do two wrong things:
>
> 1. Abuse s->filename to get pty_name from it
>
> 2. Violate layering with help of CHARDEV_IS_PTY()
>
> Let's get rid of both, and introduce correct way to get pty name in
> generic code, if available.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> chardev/char-pty.c | 7 +++++++
> chardev/char.c | 19 +++++++++++++------
> hw/char/xen_console.c | 7 ++++---
> include/chardev/char.h | 7 +++++--
> 4 files changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> index a582aa7bc7..047aade09e 100644
> --- a/chardev/char-pty.c
> +++ b/chardev/char-pty.c
> @@ -387,6 +387,12 @@ static void pty_chr_parse(QemuOpts *opts,
> ChardevBackend *backend, Error **errp)
> pty->path = g_strdup(path);
> }
>
> +static char *pty_chr_get_pty_name(Chardev *chr)
> +{
> + PtyChardev *s = PTY_CHARDEV(chr);
> + return g_strdup(s->pty_name);
> +}
> +
> static void char_pty_class_init(ObjectClass *oc, const void *data)
> {
> ChardevClass *cc = CHARDEV_CLASS(oc);
> @@ -396,6 +402,7 @@ static void char_pty_class_init(ObjectClass *oc, const
> void *data)
> cc->chr_write = pty_chr_write;
> 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;
> }
>
> static const TypeInfo char_pty_type_info = {
> diff --git a/chardev/char.c b/chardev/char.c
> index 44bfed3627..0dc792b88f 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -1090,9 +1090,7 @@ ChardevReturn *qmp_chardev_add(const char *id,
> ChardevBackend *backend,
> }
>
> ret = g_new0(ChardevReturn, 1);
> - if (CHARDEV_IS_PTY(chr)) {
> - ret->pty = g_strdup(chr->filename + 4);
> - }
> + ret->pty = qemu_chr_get_pty_name(chr);
>
> return ret;
>
> @@ -1101,6 +1099,17 @@ err:
> return NULL;
> }
>
> +char *qemu_chr_get_pty_name(Chardev *chr)
> +{
> + ChardevClass *cc = CHARDEV_GET_CLASS(chr);
> +
> + if (cc->chr_get_pty_name) {
> + return cc->chr_get_pty_name(chr);
> + }
> +
> + return NULL;
> +}
> +
> ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
> Error **errp)
> {
> @@ -1192,9 +1201,7 @@ ChardevReturn *qmp_chardev_change(const char *id,
> ChardevBackend *backend,
> object_unref(OBJECT(chr_new));
>
> ret = g_new0(ChardevReturn, 1);
> - if (CHARDEV_IS_PTY(chr_new)) {
> - ret->pty = g_strdup(chr_new->filename + 4);
> - }
> + ret->pty = qemu_chr_get_pty_name(chr_new);
>
> return ret;
> }
> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> index a639fb0b11..7502de46e4 100644
> --- a/hw/char/xen_console.c
> +++ b/hw/char/xen_console.c
> @@ -418,6 +418,7 @@ static void xen_console_realize(XenDevice *xendev,
> Error **errp)
> XenConsole *con = XEN_CONSOLE_DEVICE(xendev);
> Chardev *cs = qemu_chr_fe_get_driver(&con->chr);
> unsigned int u;
> + g_autofree char *pty_name = NULL;
>
> if (!cs) {
> error_setg(errp, "no backing character device");
> @@ -450,9 +451,9 @@ static void xen_console_realize(XenDevice *xendev,
> Error **errp)
>
> trace_xen_console_realize(con->dev, object_get_typename(OBJECT(cs)));
>
> - if (CHARDEV_IS_PTY(cs)) {
> - /* Strip the leading 'pty:' */
> - xen_device_frontend_printf(xendev, "tty", "%s", cs->filename + 4);
> + pty_name = qemu_chr_get_pty_name(cs);
> + if (pty_name) {
> + xen_device_frontend_printf(xendev, "tty", "%s", pty_name);
> }
>
> /* No normal PV driver initialization for the primary console under
> Xen */
> diff --git a/include/chardev/char.h b/include/chardev/char.h
> index 23a227dca9..d36e50b99e 100644
> --- a/include/chardev/char.h
> +++ b/include/chardev/char.h
> @@ -247,8 +247,6 @@ OBJECT_DECLARE_TYPE(Chardev, ChardevClass, CHARDEV)
>
> #define CHARDEV_IS_RINGBUF(chr) \
> object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_RINGBUF)
> -#define CHARDEV_IS_PTY(chr) \
> - object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_PTY)
>
> struct ChardevClass {
> ObjectClass parent_class;
> @@ -306,6 +304,9 @@ struct ChardevClass {
>
> /* handle various events */
> void (*chr_be_event)(Chardev *s, QEMUChrEvent event);
> +
> + /* return PTY name if available */
> + char *(*chr_get_pty_name)(Chardev *s);
> };
>
> Chardev *qemu_chardev_new(const char *id, const char *typename,
> @@ -320,4 +321,6 @@ GSource *qemu_chr_timeout_add_ms(Chardev *chr, guint
> ms,
> void suspend_mux_open(void);
> void resume_mux_open(void);
>
> +char *qemu_chr_get_pty_name(Chardev *chr);
> +
> #endif
> --
> 2.48.1
>
>