On Fri, Jul 03, 2026 at 02:48:14PM +0200, Markus Armbruster wrote:
Daniel P. Berrangé <berrange@redhat.com> writes:
This introduces a Monitor QOM object, with MonitorHMP and MonitorQMP subclasses. This is the bare minimum conversion of just the type declarations and replacing g_new/g_free with object_new/object_unref. The Monitor base class is abstract since only the HMP/QMP variants should ever be created.
When created through the existing QemuOpts interfaces, the new internal QOM object will get assigned a dynamic ID with the format "compat_monitorNNN" which is the historical QemuOpts ID naming pattern.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- chardev/char.c | 2 +- gdbstub/system.c | 2 +- include/monitor/monitor.h | 18 +++++++++++++++--- monitor/hmp.c | 31 ++++++++++++++++++++++++++++--- monitor/monitor-internal.h | 18 ++++++++++++++++-- monitor/monitor.c | 31 +++++++++++++++++++++++++++---- monitor/qmp-cmds.c | 15 ++++++++------- monitor/qmp.c | 31 ++++++++++++++++++++++++++++--- stubs/monitor-internal.c | 3 ++- system/vl.c | 7 +++---- 10 files changed, 129 insertions(+), 29 deletions(-)
diff --git a/chardev/char.c b/chardev/char.c index 813c04d953..c71ffa7963 100644 --- a/chardev/char.c +++ b/chardev/char.c @@ -805,7 +805,7 @@ static Chardev *qemu_chr_new_from_name(const char *label, const char *filename,
if (qemu_opt_get_bool(opts, "mux", 0)) { assert(permit_mux_mon); - monitor_new_hmp(chr, true, &err); + monitor_new_hmp(NULL, chr, true, &err);
Yet another way to create a monitor, not covered by the commit message.
We get here for legacy chardev syntax "mon:...", I think. Where exactly in the external interface this is accepted I'm not sure. It's documented for -serial. See qemu-options.hx.
Yes, I confirmed this codepath hits for -serial (which means it will apply to -parallel too, but no one cares about that )
There is no "mon" QemuOpts.
You pass NULL to monitor_new_hmp(), which makes it create the object as /objects/compat_monitorNNN.
Yes.
diff --git a/gdbstub/system.c b/gdbstub/system.c index 18f6a62b2e..fda8ef9352 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(mon_chr, false, &error_abort); + monitor_new_hmp(NULL, mon_chr, false, &error_abort);
And yet another, also no "mon" QemuOpts, and also /objects/compat_monitorNNN.
Yep, triggered with -gdbstub arg
diff --git a/system/vl.c b/system/vl.c index 6643242729..4737c03872 100644 --- a/system/vl.c +++ b/system/vl.c @@ -1252,7 +1252,6 @@ static int mon_init_func(void *opaque, QemuOpts *opts, Error **errp)
static void monitor_parse(const char *str, const char *mode, bool pretty) { - static int monitor_device_index = 0; QemuOpts *opts; const char *p; char label[32]; @@ -1260,8 +1259,9 @@ static void monitor_parse(const char *str, const char *mode, bool pretty) if (strstart(str, "chardev:", &p)) { snprintf(label, sizeof(label), "%s", p); } else { - snprintf(label, sizeof(label), "compat_monitor%d", - monitor_device_index); + g_autofree char *id = monitor_compat_id(); + assert(strlen(id) < sizeof(label)); + memcpy(label, id, strlen(id) + 1); opts = qemu_chr_parse_compat(label, str, true); if (!opts) { error_report("parse error: %s", str); @@ -1277,7 +1277,6 @@ static void monitor_parse(const char *str, const char *mode, bool pretty) } else { assert(pretty == false); } - monitor_device_index++;
Before the patch, we increment the NNN in "compat_monitorNNN" exactly here.
Afterwards, we increment it in monitor_compat_id(), which gets called from here and several additional places.
If any of the additional places execute before an execution of the original place, NNN change.
Feels tolerable, but needs a mention in the commit message.
Yes, will mention the change in NNN incrementing. With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|