On Wed, Jul 01, 2026 at 03:19:53PM +0200, Markus Armbruster wrote:
Daniel P. Berrangé <berrange@redhat.com> writes:
On Tue, Jun 30, 2026 at 01:55:52PM +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.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Tested-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- include/monitor/monitor.h | 11 ++++++++++- monitor/hmp.c | 29 +++++++++++++++++++++++++++-- monitor/monitor-internal.h | 18 ++++++++++++++++-- monitor/monitor.c | 18 ++++++++++++++++-- monitor/qmp-cmds.c | 15 ++++++++------- monitor/qmp.c | 29 +++++++++++++++++++++++++++-- 6 files changed, 104 insertions(+), 16 deletions(-)
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h index b9642b58ba..2e9f9e12e9 100644 --- a/include/monitor/monitor.h +++ b/include/monitor/monitor.h @@ -5,8 +5,17 @@ #include "qapi/qapi-types-misc.h" #include "qemu/readline.h" #include "exec/hwaddr.h" +#include "qom/object.h" + +#define TYPE_MONITOR "monitor" +OBJECT_DECLARE_TYPE(Monitor, MonitorClass, MONITOR); + +#define TYPE_MONITOR_HMP "monitor-hmp" +OBJECT_DECLARE_TYPE(MonitorHMP, MonitorHMPClass, MONITOR_HMP); + +#define TYPE_MONITOR_QMP "monitor-qmp" +OBJECT_DECLARE_TYPE(MonitorQMP, MonitorQMPClass, MONITOR_QMP);
-typedef struct MonitorHMP MonitorHMP; typedef struct MonitorOptions MonitorOptions;
#define QMP_REQ_QUEUE_LEN_MAX 8 diff --git a/monitor/hmp.c b/monitor/hmp.c index 4e4468424a..81047d2513 100644 --- a/monitor/hmp.c +++ b/monitor/hmp.c @@ -43,6 +43,20 @@ #include "system/block-backend.h" #include "trace.h"
+OBJECT_DEFINE_TYPE(MonitorHMP, monitor_hmp, MONITOR_HMP, MONITOR); + +static void monitor_hmp_finalize(Object *obj) +{ +} + +static void monitor_hmp_class_init(ObjectClass *cls, const void *data) +{ +} + +static void monitor_hmp_init(Object *obj) +{ +} + static void monitor_command_cb(void *opaque, const char *cmdline, void *readline_opaque) { @@ -1526,10 +1540,21 @@ static void monitor_readline_flush(void *opaque)
void monitor_new_hmp(Chardev *chr, bool use_readline, Error **errp) { - MonitorHMP *mon = g_new0(MonitorHMP, 1); + MonitorHMP *mon; + static int counter; + g_autofree char *id = g_strdup_printf("hmpcompat%d", counter++);
Hmm. The system picking IDs is problematic when they can clash with the user's IDs. If we had an ounce of common sense, we'd restrict both across the board so they cannot clash. But we don't.
We need an ID here, because we need to make the new object the child of something (actually: child of /objects/), which requires a child name.
Non-problem with -object / object-add, because @id is mandatory there.
Non-problem with -device / device_add, because we use separate parents for devices with and without @id (/machine/peripheral/ and /machine/peripheral-anon/, plus the /machine/unattached/ orphanage).
Example for an existing problem:
$ qemu-system-x86_64 -nodefaults -monitor stdio -chardev null,id=chr0 -mon id=compat_monitor0,chardev=chr0 qemu-system-x86_64: -mon id=compat_monitor0,chardev=chr0: Duplicate ID 'compat_monitor0' for mon
Example for a problem created by this series:
$ qemu-system-x86_64 -nodefaults -display none -S -monitor stdio -chardev null,id=chr0 -object monitor-hmp,id=hmpcompat0,chardev=chr0 qemu-system-x86_64: -monitor stdio: attempt to add duplicate property 'hmpcompat0' to object (type 'container')
I readily admit that these clashes are *unlikely*. Still, do we really want to define an interface that claims to let you pick any ID, then rejects some of them sometimes? Feels rather 1990s to me. At the very least, cover the wart in the commit message.
The way I looked at it was aything using -object with the new monitor-qmp/monitor-hmp types is new code. They can:
1. Trivially abide the warning about "hmpcompatNN" / "qmpcompatNN" being internal usage for compat syntax
A wart you can easily cope with is still a wart. Oh well, we'll live. However, the wart should be documented.
2. Not use both -monitor and -object on the same QEMU instance so not have a clash between the two to begin with
Management applications aware of the monitor QOM types should use -object / object-add exclusively.
Human users will use -monitor to save typing. If they then need to plug an additional one at run time, they have no choice but use object-add. Again, we'll live.
The remaining danger where is some existing code using -object with a *non-monitor* type, and calling it "hmpcompatNN" / "qmpcompatNN", which would be insanity. Never say never, but I think that's an acceptable risk.
The risk is acceptable. The documentation complexity that comes with the wart bothers me.
Could we use id_generate()?
Since we want to re-use the QemuOpts ID present in -monitor/-qmp, I'm going to stick with "compat_monitorNNN" and get rid of use of 'hmpcompatNN' and 'qmpcompatNN'. id_generate() does not help here because it forces a "#" prefix onto the ID which then (intentionally) violates id_wellformed() which object_new_xxx() validates. 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 :|