[PATCH v7 06/35] monitor: pass chardev ID into monitor constructor instead of object

Daniel P. Berrangé via Devel posted 35 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v7 06/35] monitor: pass chardev ID into monitor constructor instead of object
Posted by Daniel P. Berrangé 1 month, 1 week ago
Current the monitor_new_hmp/monitor_new_qmp constructors accept
a Chardev object pointer. To facilitate the next commit which will
introduce a QOM property for the character device ID, switch to
accepting an chardev ID in the constructor.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 chardev/char.c            |  3 ++-
 gdbstub/system.c          |  2 +-
 include/monitor/monitor.h |  4 ++--
 monitor/hmp.c             | 22 ++++++++++++++++------
 monitor/monitor.c         | 11 ++---------
 monitor/qmp.c             | 21 +++++++++++++++------
 stubs/monitor-internal.c  |  2 +-
 7 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/chardev/char.c b/chardev/char.c
index c71ffa7963..22e5bae388 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -804,8 +804,9 @@ static Chardev *qemu_chr_new_from_name(const char *label, const char *filename,
     }
 
     if (qemu_opt_get_bool(opts, "mux", 0)) {
+        const char *chardev_id = qemu_opts_id(opts);
         assert(permit_mux_mon);
-        monitor_new_hmp(NULL, chr, true, &err);
+        monitor_new_hmp(NULL, chardev_id, true, &err);
         if (err) {
             error_report_err(err);
             object_unparent(OBJECT(chr));
diff --git a/gdbstub/system.c b/gdbstub/system.c
index fda8ef9352..fd754e5664 100644
--- a/gdbstub/system.c
+++ b/gdbstub/system.c
@@ -389,7 +389,7 @@ bool gdbserver_start(const char *device, Error **errp)
         /* Initialize a monitor terminal for gdb */
         mon_chr = qemu_chardev_new(NULL, TYPE_CHARDEV_GDB,
                                    NULL, NULL, &error_abort);
-        monitor_new_hmp(NULL, mon_chr, false, &error_abort);
+        monitor_new_hmp(NULL, mon_chr->label, false, &error_abort);
     } else {
         qemu_chr_fe_deinit(&gdbserver_system_state.chr, true);
         mon_chr = gdbserver_system_state.mon_chr;
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 83723f705a..bfbeedec9b 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -29,9 +29,9 @@ bool monitor_cur_is_qmp(void);
 void monitor_init_globals(void);
 void monitor_init_globals_core(void);
 char *monitor_compat_id(void);
-void monitor_new_qmp(const char *id, Chardev *chr,
+void monitor_new_qmp(const char *id, const char *chardev_id,
                      bool pretty, Error **errp);
-void monitor_new_hmp(const char *id, Chardev *chr,
+void monitor_new_hmp(const char *id, const char *chardev_id,
                      bool use_readline, Error **errp);
 int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp);
 int monitor_new_opts(QemuOpts *opts, Error **errp);
diff --git a/monitor/hmp.c b/monitor/hmp.c
index da11e56854..1704166326 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -39,6 +39,7 @@
 #include "qemu/base-arch-defs.h"
 #include "qemu/target-info.h"
 #include "qemu/units.h"
+#include "qapi/error.h"
 #include "exec/gdbstub.h"
 #include "system/block-backend.h"
 #include "trace.h"
@@ -1538,16 +1539,25 @@ static void monitor_readline_flush(void *opaque)
     monitor_flush(&mon->parent_obj);
 }
 
-void monitor_new_hmp(const char *id, Chardev *chr,
+void monitor_new_hmp(const char *id, const char *chardev_id,
                      bool use_readline, Error **errp)
 {
     MonitorHMP *mon;
     g_autofree char *autoid = id ? NULL : monitor_compat_id();
-    Object *obj = object_new_with_props(TYPE_MONITOR_HMP,
-                                        object_get_objects_root(),
-                                        id ? id : autoid,
-                                        errp,
-                                        NULL);
+    Chardev *chr;
+    Object *obj;
+
+    chr = qemu_chr_find(chardev_id);
+    if (chr == NULL) {
+        error_setg(errp, "chardev \"%s\" not found", chardev_id);
+        return;
+    }
+
+    obj = object_new_with_props(TYPE_MONITOR_HMP,
+                                object_get_objects_root(),
+                                id ? id : autoid,
+                                errp,
+                                NULL);
     if (!obj) {
         return;
     }
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 18a1d8ddde..cb0299a2f7 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -741,13 +741,6 @@ char *monitor_compat_id(void)
 int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp)
 {
     ERRP_GUARD();
-    Chardev *chr;
-
-    chr = qemu_chr_find(opts->chardev);
-    if (chr == NULL) {
-        error_setg(errp, "chardev \"%s\" not found", opts->chardev);
-        return -1;
-    }
 
     if (!opts->has_mode) {
         opts->mode = allow_hmp ? MONITOR_MODE_READLINE : MONITOR_MODE_CONTROL;
@@ -755,7 +748,7 @@ int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp)
 
     switch (opts->mode) {
     case MONITOR_MODE_CONTROL:
-        monitor_new_qmp(opts->id, chr, opts->pretty, errp);
+        monitor_new_qmp(opts->id, opts->chardev, opts->pretty, errp);
         break;
     case MONITOR_MODE_READLINE:
         if (!allow_hmp) {
@@ -766,7 +759,7 @@ int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp)
             error_setg(errp, "'pretty' is not compatible with HMP monitors");
             return -1;
         }
-        monitor_new_hmp(opts->id, chr, true, errp);
+        monitor_new_hmp(opts->id, opts->chardev, true, errp);
         break;
     default:
         g_assert_not_reached();
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 1ef09352fc..e2f841212b 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -527,16 +527,25 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
     monitor_list_append(&mon->parent_obj);
 }
 
-void monitor_new_qmp(const char *id, Chardev *chr,
+void monitor_new_qmp(const char *id, const char *chardev_id,
                      bool pretty, Error **errp)
 {
     MonitorQMP *mon;
     g_autofree char *autoid = id ? NULL : monitor_compat_id();
-    Object *obj = object_new_with_props(TYPE_MONITOR_QMP,
-                                        object_get_objects_root(),
-                                        id ? id : autoid,
-                                        errp,
-                                        NULL);
+    Chardev *chr;
+    Object *obj;
+
+    chr = qemu_chr_find(chardev_id);
+    if (chr == NULL) {
+        error_setg(errp, "chardev \"%s\" not found", chardev_id);
+        return;
+    }
+
+    obj = object_new_with_props(TYPE_MONITOR_QMP,
+                                object_get_objects_root(),
+                                id ? id : autoid,
+                                errp,
+                                NULL);
     if (!obj) {
         return;
     }
diff --git a/stubs/monitor-internal.c b/stubs/monitor-internal.c
index 20367b7e9a..f94b5e5c21 100644
--- a/stubs/monitor-internal.c
+++ b/stubs/monitor-internal.c
@@ -8,7 +8,7 @@ int monitor_get_fd(Monitor *mon, const char *name, Error **errp)
     return -1;
 }
 
-void monitor_new_hmp(const char *id, Chardev *chr,
+void monitor_new_hmp(const char *id, const char *chardev_id,
                      bool use_readline, Error **errp)
 {
 }
-- 
2.55.0


Re: [PATCH v7 06/35] monitor: pass chardev ID into monitor constructor instead of object
Posted by Marc-André Lureau 1 month, 1 week ago
On Mon, Jul 6, 2026 at 5:58 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> Current the monitor_new_hmp/monitor_new_qmp constructors accept
> a Chardev object pointer. To facilitate the next commit which will
> introduce a QOM property for the character device ID, switch to
> accepting an chardev ID in the constructor.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  chardev/char.c            |  3 ++-
>  gdbstub/system.c          |  2 +-
>  include/monitor/monitor.h |  4 ++--
>  monitor/hmp.c             | 22 ++++++++++++++++------
>  monitor/monitor.c         | 11 ++---------
>  monitor/qmp.c             | 21 +++++++++++++++------
>  stubs/monitor-internal.c  |  2 +-
>  7 files changed, 39 insertions(+), 26 deletions(-)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index c71ffa7963..22e5bae388 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -804,8 +804,9 @@ static Chardev *qemu_chr_new_from_name(const char *label, const char *filename,
>      }
>
>      if (qemu_opt_get_bool(opts, "mux", 0)) {
> +        const char *chardev_id = qemu_opts_id(opts);
>          assert(permit_mux_mon);
> -        monitor_new_hmp(NULL, chr, true, &err);
> +        monitor_new_hmp(NULL, chardev_id, true, &err);
>          if (err) {
>              error_report_err(err);
>              object_unparent(OBJECT(chr));
> diff --git a/gdbstub/system.c b/gdbstub/system.c
> index fda8ef9352..fd754e5664 100644
> --- a/gdbstub/system.c
> +++ b/gdbstub/system.c
> @@ -389,7 +389,7 @@ bool gdbserver_start(const char *device, Error **errp)
>          /* Initialize a monitor terminal for gdb */
>          mon_chr = qemu_chardev_new(NULL, TYPE_CHARDEV_GDB,
>                                     NULL, NULL, &error_abort);
> -        monitor_new_hmp(NULL, mon_chr, false, &error_abort);
> +        monitor_new_hmp(NULL, mon_chr->label, false, &error_abort);
>      } else {
>          qemu_chr_fe_deinit(&gdbserver_system_state.chr, true);
>          mon_chr = gdbserver_system_state.mon_chr;
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index 83723f705a..bfbeedec9b 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -29,9 +29,9 @@ bool monitor_cur_is_qmp(void);
>  void monitor_init_globals(void);
>  void monitor_init_globals_core(void);
>  char *monitor_compat_id(void);
> -void monitor_new_qmp(const char *id, Chardev *chr,
> +void monitor_new_qmp(const char *id, const char *chardev_id,
>                       bool pretty, Error **errp);
> -void monitor_new_hmp(const char *id, Chardev *chr,
> +void monitor_new_hmp(const char *id, const char *chardev_id,
>                       bool use_readline, Error **errp);
>  int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp);
>  int monitor_new_opts(QemuOpts *opts, Error **errp);
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index da11e56854..1704166326 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -39,6 +39,7 @@
>  #include "qemu/base-arch-defs.h"
>  #include "qemu/target-info.h"
>  #include "qemu/units.h"
> +#include "qapi/error.h"
>  #include "exec/gdbstub.h"
>  #include "system/block-backend.h"
>  #include "trace.h"
> @@ -1538,16 +1539,25 @@ static void monitor_readline_flush(void *opaque)
>      monitor_flush(&mon->parent_obj);
>  }
>
> -void monitor_new_hmp(const char *id, Chardev *chr,
> +void monitor_new_hmp(const char *id, const char *chardev_id,
>                       bool use_readline, Error **errp)
>  {
>      MonitorHMP *mon;
>      g_autofree char *autoid = id ? NULL : monitor_compat_id();
> -    Object *obj = object_new_with_props(TYPE_MONITOR_HMP,
> -                                        object_get_objects_root(),
> -                                        id ? id : autoid,
> -                                        errp,
> -                                        NULL);
> +    Chardev *chr;
> +    Object *obj;
> +
> +    chr = qemu_chr_find(chardev_id);
> +    if (chr == NULL) {
> +        error_setg(errp, "chardev \"%s\" not found", chardev_id);
> +        return;
> +    }
> +
> +    obj = object_new_with_props(TYPE_MONITOR_HMP,
> +                                object_get_objects_root(),
> +                                id ? id : autoid,
> +                                errp,
> +                                NULL);
>      if (!obj) {
>          return;
>      }
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 18a1d8ddde..cb0299a2f7 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -741,13 +741,6 @@ char *monitor_compat_id(void)
>  int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp)
>  {
>      ERRP_GUARD();
> -    Chardev *chr;
> -
> -    chr = qemu_chr_find(opts->chardev);
> -    if (chr == NULL) {
> -        error_setg(errp, "chardev \"%s\" not found", opts->chardev);
> -        return -1;
> -    }
>
>      if (!opts->has_mode) {
>          opts->mode = allow_hmp ? MONITOR_MODE_READLINE : MONITOR_MODE_CONTROL;
> @@ -755,7 +748,7 @@ int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp)
>
>      switch (opts->mode) {
>      case MONITOR_MODE_CONTROL:
> -        monitor_new_qmp(opts->id, chr, opts->pretty, errp);
> +        monitor_new_qmp(opts->id, opts->chardev, opts->pretty, errp);
>          break;
>      case MONITOR_MODE_READLINE:
>          if (!allow_hmp) {
> @@ -766,7 +759,7 @@ int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp)
>              error_setg(errp, "'pretty' is not compatible with HMP monitors");
>              return -1;
>          }
> -        monitor_new_hmp(opts->id, chr, true, errp);
> +        monitor_new_hmp(opts->id, opts->chardev, true, errp);
>          break;
>      default:
>          g_assert_not_reached();
> diff --git a/monitor/qmp.c b/monitor/qmp.c
> index 1ef09352fc..e2f841212b 100644
> --- a/monitor/qmp.c
> +++ b/monitor/qmp.c
> @@ -527,16 +527,25 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
>      monitor_list_append(&mon->parent_obj);
>  }
>
> -void monitor_new_qmp(const char *id, Chardev *chr,
> +void monitor_new_qmp(const char *id, const char *chardev_id,
>                       bool pretty, Error **errp)
>  {
>      MonitorQMP *mon;
>      g_autofree char *autoid = id ? NULL : monitor_compat_id();
> -    Object *obj = object_new_with_props(TYPE_MONITOR_QMP,
> -                                        object_get_objects_root(),
> -                                        id ? id : autoid,
> -                                        errp,
> -                                        NULL);
> +    Chardev *chr;
> +    Object *obj;
> +
> +    chr = qemu_chr_find(chardev_id);
> +    if (chr == NULL) {
> +        error_setg(errp, "chardev \"%s\" not found", chardev_id);
> +        return;
> +    }
> +
> +    obj = object_new_with_props(TYPE_MONITOR_QMP,
> +                                object_get_objects_root(),
> +                                id ? id : autoid,
> +                                errp,
> +                                NULL);
>      if (!obj) {
>          return;
>      }
> diff --git a/stubs/monitor-internal.c b/stubs/monitor-internal.c
> index 20367b7e9a..f94b5e5c21 100644
> --- a/stubs/monitor-internal.c
> +++ b/stubs/monitor-internal.c
> @@ -8,7 +8,7 @@ int monitor_get_fd(Monitor *mon, const char *name, Error **errp)
>      return -1;
>  }
>
> -void monitor_new_hmp(const char *id, Chardev *chr,
> +void monitor_new_hmp(const char *id, const char *chardev_id,
>                       bool use_readline, Error **errp)
>  {
>  }
> --
> 2.55.0
>
Re: [PATCH v7 06/35] monitor: pass chardev ID into monitor constructor instead of object
Posted by Markus Armbruster 1 month, 1 week ago
Daniel P. Berrangé <berrange@redhat.com> writes:

> Current the monitor_new_hmp/monitor_new_qmp constructors accept
> a Chardev object pointer. To facilitate the next commit which will
> introduce a QOM property for the character device ID, switch to
> accepting an chardev ID in the constructor.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Split off a patch that got Marc-André's R-by.  Marc-André, care to give
it again?