* [PATCH 01/17] monitor: replace 'common' with 'parent' in MonitorHMP
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
@ 2026-04-10 16:04 ` Daniel P. Berrangé
2026-04-10 21:05 ` Dr. David Alan Gilbert
2026-04-10 16:04 ` [PATCH 02/17] monitor: replace 'common' with 'parent' in MonitorQMP Daniel P. Berrangé
` (16 subsequent siblings)
17 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf,
Daniel P. Berrangé, qemu-block, Dr. David Alan Gilbert,
Eric Blake
The field name 'parent' is standard practice for QOM structs
so align the HMP monitor.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
monitor/hmp-cmds.c | 2 +-
monitor/hmp.c | 40 +++++++++++++++++++-------------------
monitor/monitor-internal.h | 2 +-
monitor/monitor.c | 6 +++---
monitor/qmp-cmds.c | 10 +++++-----
ui/ui-hmp-cmds.c | 2 +-
6 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index bc26b39d70..911d984cbe 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -236,7 +236,7 @@ void hmp_info_sync_profile(Monitor *mon, const QDict *qdict)
void hmp_info_history(Monitor *mon, const QDict *qdict)
{
- MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
+ MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, parent);
int i;
const char *str;
diff --git a/monitor/hmp.c b/monitor/hmp.c
index 0e5913fabb..af346d190b 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -47,9 +47,9 @@ static void monitor_command_cb(void *opaque, const char *cmdline,
{
MonitorHMP *mon = opaque;
- monitor_suspend(&mon->common);
+ monitor_suspend(&mon->parent);
handle_hmp_command(mon, cmdline);
- monitor_resume(&mon->common);
+ monitor_resume(&mon->parent);
}
void monitor_read_command(MonitorHMP *mon, int show_prompt)
@@ -72,7 +72,7 @@ int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func,
/* prompt is printed on return from the command handler */
return 0;
} else {
- monitor_printf(&mon->common,
+ monitor_printf(&mon->parent,
"terminal does not support password prompting\n");
return -ENOTTY;
}
@@ -689,7 +689,7 @@ static const HMPCommand *monitor_parse_command(MonitorHMP *hmp_mon,
const char **cmdp,
HMPCommand *table)
{
- Monitor *mon = &hmp_mon->common;
+ Monitor *mon = &hmp_mon->parent;
const char *p;
const HMPCommand *cmd;
char cmdname[256];
@@ -1182,35 +1182,35 @@ void handle_hmp_command(MonitorHMP *mon, const char *cmdline)
if (!cmd->cmd && !cmd->cmd_info_hrt) {
/* FIXME: is it useful to try autoload modules here ??? */
- monitor_printf(&mon->common, "Command \"%.*s\" is not available.\n",
+ monitor_printf(&mon->parent, "Command \"%.*s\" is not available.\n",
(int)(cmdline - cmd_start), cmd_start);
return;
}
- qdict = monitor_parse_arguments(&mon->common, &cmdline, cmd);
+ qdict = monitor_parse_arguments(&mon->parent, &cmdline, cmd);
if (!qdict) {
while (cmdline > cmd_start && qemu_isspace(cmdline[-1])) {
cmdline--;
}
- monitor_printf(&mon->common, "Try \"help %.*s\" for more information\n",
+ monitor_printf(&mon->parent, "Try \"help %.*s\" for more information\n",
(int)(cmdline - cmd_start), cmd_start);
return;
}
if (!cmd->coroutine) {
/* old_mon is non-NULL when called from qmp_human_monitor_command() */
- Monitor *old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common);
- handle_hmp_command_exec(&mon->common, cmd, qdict);
+ Monitor *old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->parent);
+ handle_hmp_command_exec(&mon->parent, cmd, qdict);
monitor_set_cur(qemu_coroutine_self(), old_mon);
} else {
HandleHmpCommandCo data = {
- .mon = &mon->common,
+ .mon = &mon->parent,
.cmd = cmd,
.qdict = qdict,
.done = false,
};
Coroutine *co = qemu_coroutine_create(handle_hmp_command_co, &data);
- monitor_set_cur(co, &mon->common);
+ monitor_set_cur(co, &mon->parent);
aio_co_enter(qemu_get_aio_context(), co);
AIO_WAIT_WHILE_UNLOCKED(NULL, !data.done);
}
@@ -1428,7 +1428,7 @@ cleanup:
static void monitor_read(void *opaque, const uint8_t *buf, int size)
{
- MonitorHMP *mon = container_of(opaque, MonitorHMP, common);
+ MonitorHMP *mon = container_of(opaque, MonitorHMP, parent);
int i;
if (mon->rs) {
@@ -1437,7 +1437,7 @@ static void monitor_read(void *opaque, const uint8_t *buf, int size)
}
} else {
if (size == 0 || buf[size - 1] != 0) {
- monitor_printf(&mon->common, "corrupted command\n");
+ monitor_printf(&mon->parent, "corrupted command\n");
} else {
handle_hmp_command(mon, (char *)buf);
}
@@ -1506,26 +1506,26 @@ static void G_GNUC_PRINTF(2, 3) monitor_readline_printf(void *opaque,
MonitorHMP *mon = opaque;
va_list ap;
va_start(ap, fmt);
- monitor_vprintf(&mon->common, fmt, ap);
+ monitor_vprintf(&mon->parent, fmt, ap);
va_end(ap);
}
static void monitor_readline_flush(void *opaque)
{
MonitorHMP *mon = opaque;
- monitor_flush(&mon->common);
+ monitor_flush(&mon->parent);
}
void monitor_init_hmp(Chardev *chr, bool use_readline, Error **errp)
{
MonitorHMP *mon = g_new0(MonitorHMP, 1);
- if (!qemu_chr_fe_init(&mon->common.chr, chr, errp)) {
+ if (!qemu_chr_fe_init(&mon->parent.chr, chr, errp)) {
g_free(mon);
return;
}
- monitor_data_init(&mon->common, false, false, false);
+ monitor_data_init(&mon->parent, false, false, false);
mon->use_readline = use_readline;
if (mon->use_readline) {
@@ -1536,9 +1536,9 @@ void monitor_init_hmp(Chardev *chr, bool use_readline, Error **errp)
monitor_read_command(mon, 0);
}
- qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read, monitor_read,
- monitor_event, NULL, &mon->common, NULL, true);
- monitor_list_append(&mon->common);
+ qemu_chr_fe_set_handlers(&mon->parent.chr, monitor_can_read, monitor_read,
+ monitor_event, NULL, &mon->parent, NULL, true);
+ monitor_list_append(&mon->parent);
}
/**
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index feca111ae3..ae0cf8c1da 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -119,7 +119,7 @@ struct Monitor {
};
struct MonitorHMP {
- Monitor common;
+ Monitor parent;
bool use_readline;
/*
* State used only in the thread "owning" the monitor.
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 00b93ed612..6532796edb 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -132,7 +132,7 @@ static inline bool monitor_is_hmp_non_interactive(const Monitor *mon)
return false;
}
- return !monitor_uses_readline(container_of(mon, MonitorHMP, common));
+ return !monitor_uses_readline(container_of(mon, MonitorHMP, parent));
}
static gboolean monitor_unblocked(void *do_not_use, GIOCondition cond,
@@ -542,7 +542,7 @@ static void monitor_accept_input(void *opaque)
qemu_mutex_lock(&mon->mon_lock);
if (!monitor_is_qmp(mon) && mon->reset_seen) {
- MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
+ MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, parent);
assert(hmp_mon->rs);
readline_restart(hmp_mon->rs);
qemu_mutex_unlock(&mon->mon_lock);
@@ -627,7 +627,7 @@ void monitor_data_destroy(Monitor *mon)
if (monitor_is_qmp(mon)) {
monitor_data_destroy_qmp(container_of(mon, MonitorQMP, common));
} else {
- readline_free(container_of(mon, MonitorHMP, common)->rs);
+ readline_free(container_of(mon, MonitorHMP, parent)->rs);
}
g_string_free(mon->outbuf, true);
qemu_mutex_destroy(&mon->mon_lock);
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 0c409c27dc..191eba1b3a 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -168,10 +168,10 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
char *output = NULL;
MonitorHMP hmp = {};
- monitor_data_init(&hmp.common, false, true, false);
+ monitor_data_init(&hmp.parent, false, true, false);
if (has_cpu_index) {
- int ret = monitor_set_cpu(&hmp.common, cpu_index);
+ int ret = monitor_set_cpu(&hmp.parent, cpu_index);
if (ret < 0) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "cpu-index",
"a CPU number");
@@ -181,12 +181,12 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
handle_hmp_command(&hmp, command_line);
- WITH_QEMU_LOCK_GUARD(&hmp.common.mon_lock) {
- output = g_strdup(hmp.common.outbuf->str);
+ WITH_QEMU_LOCK_GUARD(&hmp.parent.mon_lock) {
+ output = g_strdup(hmp.parent.outbuf->str);
}
out:
- monitor_data_destroy(&hmp.common);
+ monitor_data_destroy(&hmp.parent);
return output;
}
diff --git a/ui/ui-hmp-cmds.c b/ui/ui-hmp-cmds.c
index 6c93d452c9..fe89632474 100644
--- a/ui/ui-hmp-cmds.c
+++ b/ui/ui-hmp-cmds.c
@@ -341,7 +341,7 @@ void hmp_change_vnc(Monitor *mon, const char *device, const char *target,
return;
}
if (!arg) {
- MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
+ MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, parent);
monitor_read_password(hmp_mon, hmp_change_read_arg, NULL);
} else {
qmp_change_vnc_password(arg, errp);
--
2.53.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 01/17] monitor: replace 'common' with 'parent' in MonitorHMP
2026-04-10 16:04 ` [PATCH 01/17] monitor: replace 'common' with 'parent' in MonitorHMP Daniel P. Berrangé
@ 2026-04-10 21:05 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2026-04-10 21:05 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf, qemu-block,
Eric Blake
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> The field name 'parent' is standard practice for QOM structs
> so align the HMP monitor.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Just a rename, fine;
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
> ---
> monitor/hmp-cmds.c | 2 +-
> monitor/hmp.c | 40 +++++++++++++++++++-------------------
> monitor/monitor-internal.h | 2 +-
> monitor/monitor.c | 6 +++---
> monitor/qmp-cmds.c | 10 +++++-----
> ui/ui-hmp-cmds.c | 2 +-
> 6 files changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index bc26b39d70..911d984cbe 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -236,7 +236,7 @@ void hmp_info_sync_profile(Monitor *mon, const QDict *qdict)
>
> void hmp_info_history(Monitor *mon, const QDict *qdict)
> {
> - MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
> + MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, parent);
> int i;
> const char *str;
>
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index 0e5913fabb..af346d190b 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -47,9 +47,9 @@ static void monitor_command_cb(void *opaque, const char *cmdline,
> {
> MonitorHMP *mon = opaque;
>
> - monitor_suspend(&mon->common);
> + monitor_suspend(&mon->parent);
> handle_hmp_command(mon, cmdline);
> - monitor_resume(&mon->common);
> + monitor_resume(&mon->parent);
> }
>
> void monitor_read_command(MonitorHMP *mon, int show_prompt)
> @@ -72,7 +72,7 @@ int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func,
> /* prompt is printed on return from the command handler */
> return 0;
> } else {
> - monitor_printf(&mon->common,
> + monitor_printf(&mon->parent,
> "terminal does not support password prompting\n");
> return -ENOTTY;
> }
> @@ -689,7 +689,7 @@ static const HMPCommand *monitor_parse_command(MonitorHMP *hmp_mon,
> const char **cmdp,
> HMPCommand *table)
> {
> - Monitor *mon = &hmp_mon->common;
> + Monitor *mon = &hmp_mon->parent;
> const char *p;
> const HMPCommand *cmd;
> char cmdname[256];
> @@ -1182,35 +1182,35 @@ void handle_hmp_command(MonitorHMP *mon, const char *cmdline)
>
> if (!cmd->cmd && !cmd->cmd_info_hrt) {
> /* FIXME: is it useful to try autoload modules here ??? */
> - monitor_printf(&mon->common, "Command \"%.*s\" is not available.\n",
> + monitor_printf(&mon->parent, "Command \"%.*s\" is not available.\n",
> (int)(cmdline - cmd_start), cmd_start);
> return;
> }
>
> - qdict = monitor_parse_arguments(&mon->common, &cmdline, cmd);
> + qdict = monitor_parse_arguments(&mon->parent, &cmdline, cmd);
> if (!qdict) {
> while (cmdline > cmd_start && qemu_isspace(cmdline[-1])) {
> cmdline--;
> }
> - monitor_printf(&mon->common, "Try \"help %.*s\" for more information\n",
> + monitor_printf(&mon->parent, "Try \"help %.*s\" for more information\n",
> (int)(cmdline - cmd_start), cmd_start);
> return;
> }
>
> if (!cmd->coroutine) {
> /* old_mon is non-NULL when called from qmp_human_monitor_command() */
> - Monitor *old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common);
> - handle_hmp_command_exec(&mon->common, cmd, qdict);
> + Monitor *old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->parent);
> + handle_hmp_command_exec(&mon->parent, cmd, qdict);
> monitor_set_cur(qemu_coroutine_self(), old_mon);
> } else {
> HandleHmpCommandCo data = {
> - .mon = &mon->common,
> + .mon = &mon->parent,
> .cmd = cmd,
> .qdict = qdict,
> .done = false,
> };
> Coroutine *co = qemu_coroutine_create(handle_hmp_command_co, &data);
> - monitor_set_cur(co, &mon->common);
> + monitor_set_cur(co, &mon->parent);
> aio_co_enter(qemu_get_aio_context(), co);
> AIO_WAIT_WHILE_UNLOCKED(NULL, !data.done);
> }
> @@ -1428,7 +1428,7 @@ cleanup:
>
> static void monitor_read(void *opaque, const uint8_t *buf, int size)
> {
> - MonitorHMP *mon = container_of(opaque, MonitorHMP, common);
> + MonitorHMP *mon = container_of(opaque, MonitorHMP, parent);
> int i;
>
> if (mon->rs) {
> @@ -1437,7 +1437,7 @@ static void monitor_read(void *opaque, const uint8_t *buf, int size)
> }
> } else {
> if (size == 0 || buf[size - 1] != 0) {
> - monitor_printf(&mon->common, "corrupted command\n");
> + monitor_printf(&mon->parent, "corrupted command\n");
> } else {
> handle_hmp_command(mon, (char *)buf);
> }
> @@ -1506,26 +1506,26 @@ static void G_GNUC_PRINTF(2, 3) monitor_readline_printf(void *opaque,
> MonitorHMP *mon = opaque;
> va_list ap;
> va_start(ap, fmt);
> - monitor_vprintf(&mon->common, fmt, ap);
> + monitor_vprintf(&mon->parent, fmt, ap);
> va_end(ap);
> }
>
> static void monitor_readline_flush(void *opaque)
> {
> MonitorHMP *mon = opaque;
> - monitor_flush(&mon->common);
> + monitor_flush(&mon->parent);
> }
>
> void monitor_init_hmp(Chardev *chr, bool use_readline, Error **errp)
> {
> MonitorHMP *mon = g_new0(MonitorHMP, 1);
>
> - if (!qemu_chr_fe_init(&mon->common.chr, chr, errp)) {
> + if (!qemu_chr_fe_init(&mon->parent.chr, chr, errp)) {
> g_free(mon);
> return;
> }
>
> - monitor_data_init(&mon->common, false, false, false);
> + monitor_data_init(&mon->parent, false, false, false);
>
> mon->use_readline = use_readline;
> if (mon->use_readline) {
> @@ -1536,9 +1536,9 @@ void monitor_init_hmp(Chardev *chr, bool use_readline, Error **errp)
> monitor_read_command(mon, 0);
> }
>
> - qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read, monitor_read,
> - monitor_event, NULL, &mon->common, NULL, true);
> - monitor_list_append(&mon->common);
> + qemu_chr_fe_set_handlers(&mon->parent.chr, monitor_can_read, monitor_read,
> + monitor_event, NULL, &mon->parent, NULL, true);
> + monitor_list_append(&mon->parent);
> }
>
> /**
> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> index feca111ae3..ae0cf8c1da 100644
> --- a/monitor/monitor-internal.h
> +++ b/monitor/monitor-internal.h
> @@ -119,7 +119,7 @@ struct Monitor {
> };
>
> struct MonitorHMP {
> - Monitor common;
> + Monitor parent;
> bool use_readline;
> /*
> * State used only in the thread "owning" the monitor.
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 00b93ed612..6532796edb 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -132,7 +132,7 @@ static inline bool monitor_is_hmp_non_interactive(const Monitor *mon)
> return false;
> }
>
> - return !monitor_uses_readline(container_of(mon, MonitorHMP, common));
> + return !monitor_uses_readline(container_of(mon, MonitorHMP, parent));
> }
>
> static gboolean monitor_unblocked(void *do_not_use, GIOCondition cond,
> @@ -542,7 +542,7 @@ static void monitor_accept_input(void *opaque)
>
> qemu_mutex_lock(&mon->mon_lock);
> if (!monitor_is_qmp(mon) && mon->reset_seen) {
> - MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
> + MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, parent);
> assert(hmp_mon->rs);
> readline_restart(hmp_mon->rs);
> qemu_mutex_unlock(&mon->mon_lock);
> @@ -627,7 +627,7 @@ void monitor_data_destroy(Monitor *mon)
> if (monitor_is_qmp(mon)) {
> monitor_data_destroy_qmp(container_of(mon, MonitorQMP, common));
> } else {
> - readline_free(container_of(mon, MonitorHMP, common)->rs);
> + readline_free(container_of(mon, MonitorHMP, parent)->rs);
> }
> g_string_free(mon->outbuf, true);
> qemu_mutex_destroy(&mon->mon_lock);
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index 0c409c27dc..191eba1b3a 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -168,10 +168,10 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
> char *output = NULL;
> MonitorHMP hmp = {};
>
> - monitor_data_init(&hmp.common, false, true, false);
> + monitor_data_init(&hmp.parent, false, true, false);
>
> if (has_cpu_index) {
> - int ret = monitor_set_cpu(&hmp.common, cpu_index);
> + int ret = monitor_set_cpu(&hmp.parent, cpu_index);
> if (ret < 0) {
> error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "cpu-index",
> "a CPU number");
> @@ -181,12 +181,12 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
>
> handle_hmp_command(&hmp, command_line);
>
> - WITH_QEMU_LOCK_GUARD(&hmp.common.mon_lock) {
> - output = g_strdup(hmp.common.outbuf->str);
> + WITH_QEMU_LOCK_GUARD(&hmp.parent.mon_lock) {
> + output = g_strdup(hmp.parent.outbuf->str);
> }
>
> out:
> - monitor_data_destroy(&hmp.common);
> + monitor_data_destroy(&hmp.parent);
> return output;
> }
>
> diff --git a/ui/ui-hmp-cmds.c b/ui/ui-hmp-cmds.c
> index 6c93d452c9..fe89632474 100644
> --- a/ui/ui-hmp-cmds.c
> +++ b/ui/ui-hmp-cmds.c
> @@ -341,7 +341,7 @@ void hmp_change_vnc(Monitor *mon, const char *device, const char *target,
> return;
> }
> if (!arg) {
> - MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
> + MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, parent);
> monitor_read_password(hmp_mon, hmp_change_read_arg, NULL);
> } else {
> qmp_change_vnc_password(arg, errp);
> --
> 2.53.0
>
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 02/17] monitor: replace 'common' with 'parent' in MonitorQMP
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
2026-04-10 16:04 ` [PATCH 01/17] monitor: replace 'common' with 'parent' in MonitorHMP Daniel P. Berrangé
@ 2026-04-10 16:04 ` Daniel P. Berrangé
2026-04-14 15:15 ` Mark Cave-Ayland
2026-04-10 16:04 ` [PATCH 03/17] monitor: rename monitor_init* to monitor_new* Daniel P. Berrangé
` (15 subsequent siblings)
17 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf,
Daniel P. Berrangé, qemu-block, Dr. David Alan Gilbert,
Eric Blake
The field name 'parent' is standard practice for QOM structs
so align the QMP monitor.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
monitor/monitor-internal.h | 2 +-
monitor/monitor.c | 4 ++--
monitor/qmp-cmds-control.c | 4 ++--
monitor/qmp.c | 40 +++++++++++++++++++-------------------
4 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index ae0cf8c1da..0922588ad9 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -132,7 +132,7 @@ struct MonitorHMP {
};
typedef struct {
- Monitor common;
+ Monitor parent;
JSONMessageParser parser;
bool pretty;
/*
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 6532796edb..6b183edcf5 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -308,7 +308,7 @@ static void monitor_qapi_event_emit(QAPIEvent event, QDict *qdict)
continue;
}
- qmp_mon = container_of(mon, MonitorQMP, common);
+ qmp_mon = container_of(mon, MonitorQMP, parent);
{
QEMU_LOCK_GUARD(&mon->mon_lock);
if (qmp_mon->commands == &qmp_cap_negotiation_commands) {
@@ -625,7 +625,7 @@ void monitor_data_destroy(Monitor *mon)
g_free(mon->mon_cpu_path);
qemu_chr_fe_deinit(&mon->chr, false);
if (monitor_is_qmp(mon)) {
- monitor_data_destroy_qmp(container_of(mon, MonitorQMP, common));
+ monitor_data_destroy_qmp(container_of(mon, MonitorQMP, parent));
} else {
readline_free(container_of(mon, MonitorHMP, parent)->rs);
}
diff --git a/monitor/qmp-cmds-control.c b/monitor/qmp-cmds-control.c
index 150ca9f5cb..ca471bc1c7 100644
--- a/monitor/qmp-cmds-control.c
+++ b/monitor/qmp-cmds-control.c
@@ -76,7 +76,7 @@ void qmp_qmp_capabilities(bool has_enable, QMPCapabilityList *enable,
MonitorQMP *mon;
assert(monitor_is_qmp(cur_mon));
- mon = container_of(cur_mon, MonitorQMP, common);
+ mon = container_of(cur_mon, MonitorQMP, parent);
if (mon->commands == &qmp_commands) {
error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
@@ -126,7 +126,7 @@ CommandInfoList *qmp_query_commands(Error **errp)
MonitorQMP *mon;
assert(monitor_is_qmp(cur_mon));
- mon = container_of(cur_mon, MonitorQMP, common);
+ mon = container_of(cur_mon, MonitorQMP, parent);
qmp_for_each_command(mon->commands, query_commands_cb, &list);
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 687019811f..69d9e40e32 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -80,7 +80,7 @@ static void monitor_qmp_caps_reset(MonitorQMP *mon)
{
memset(mon->capab_offered, 0, sizeof(mon->capab_offered));
memset(mon->capab, 0, sizeof(mon->capab));
- mon->capab_offered[QMP_CAPABILITY_OOB] = mon->common.use_io_thread;
+ mon->capab_offered[QMP_CAPABILITY_OOB] = mon->parent.use_io_thread;
}
static void qmp_request_free(QMPRequest *req)
@@ -124,7 +124,7 @@ static void monitor_qmp_cleanup_queue_and_resume(MonitorQMP *mon)
* when we get here while the monitor is suspended. An
* unfortunately timed CHR_EVENT_CLOSED can do the trick.
*/
- monitor_resume(&mon->common);
+ monitor_resume(&mon->parent);
}
}
@@ -139,7 +139,7 @@ void qmp_send_response(MonitorQMP *mon, const QDict *rsp)
trace_monitor_qmp_respond(mon, json->str);
g_string_append_c(json, '\n');
- monitor_puts(&mon->common, json->str);
+ monitor_puts(&mon->parent, json->str);
g_string_free(json, true);
}
@@ -166,7 +166,7 @@ static void monitor_qmp_dispatch(MonitorQMP *mon, QObject *req)
QDict *error;
rsp = qmp_dispatch(mon->commands, req, qmp_oob_enabled(mon),
- &mon->common);
+ &mon->parent);
if (mon->commands == &qmp_cap_negotiation_commands) {
error = qdict_get_qdict(rsp, "error");
@@ -207,7 +207,7 @@ static QMPRequest *monitor_qmp_requests_pop_any_with_lock(void)
continue;
}
- qmp_mon = container_of(mon, MonitorQMP, common);
+ qmp_mon = container_of(mon, MonitorQMP, parent);
qemu_mutex_lock(&qmp_mon->qmp_queue_lock);
req_obj = g_queue_pop_head(qmp_mon->qmp_requests);
if (req_obj) {
@@ -302,7 +302,7 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
oob_enabled = qmp_oob_enabled(mon);
if (oob_enabled
&& mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) {
- monitor_resume(&mon->common);
+ monitor_resume(&mon->parent);
}
/*
@@ -343,7 +343,7 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
}
if (!oob_enabled) {
- monitor_resume(&mon->common);
+ monitor_resume(&mon->parent);
}
qmp_request_free(req_obj);
@@ -408,7 +408,7 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err)
*/
if (!qmp_oob_enabled(mon) ||
mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) {
- monitor_suspend(&mon->common);
+ monitor_suspend(&mon->parent);
}
/*
@@ -462,7 +462,7 @@ static void monitor_qmp_event(void *opaque, QEMUChrEvent event)
switch (event) {
case CHR_EVENT_OPENED:
- WITH_QEMU_LOCK_GUARD(&mon->common.mon_lock) {
+ WITH_QEMU_LOCK_GUARD(&mon->parent.mon_lock) {
mon->commands = &qmp_cap_negotiation_commands;
monitor_qmp_caps_reset(mon);
}
@@ -504,27 +504,27 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
MonitorQMP *mon = opaque;
GMainContext *context;
- assert(mon->common.use_io_thread);
+ assert(mon->parent.use_io_thread);
context = iothread_get_g_main_context(mon_iothread);
assert(context);
- qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read,
+ qemu_chr_fe_set_handlers(&mon->parent.chr, monitor_can_read,
monitor_qmp_read, monitor_qmp_event,
- NULL, &mon->common, context, true);
- monitor_list_append(&mon->common);
+ NULL, &mon->parent, context, true);
+ monitor_list_append(&mon->parent);
}
void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
{
MonitorQMP *mon = g_new0(MonitorQMP, 1);
- if (!qemu_chr_fe_init(&mon->common.chr, chr, errp)) {
+ if (!qemu_chr_fe_init(&mon->parent.chr, chr, errp)) {
g_free(mon);
return;
}
- qemu_chr_fe_set_echo(&mon->common.chr, true);
+ qemu_chr_fe_set_echo(&mon->parent.chr, true);
/* Note: we run QMP monitor in I/O thread when @chr supports that */
- monitor_data_init(&mon->common, true, false,
+ monitor_data_init(&mon->parent, true, false,
qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
mon->pretty = pretty;
@@ -533,7 +533,7 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
mon->qmp_requests = g_queue_new();
json_message_parser_init(&mon->parser, handle_qmp_command, mon, NULL);
- if (mon->common.use_io_thread) {
+ if (mon->parent.use_io_thread) {
/*
* Make sure the old iowatch is gone. It's possible when
* e.g. the chardev is in client mode, with wait=on.
@@ -553,9 +553,9 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
monitor_qmp_setup_handlers_bh, mon);
/* The bottom half will add @mon to @mon_list */
} else {
- qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read,
+ qemu_chr_fe_set_handlers(&mon->parent.chr, monitor_can_read,
monitor_qmp_read, monitor_qmp_event,
- NULL, &mon->common, NULL, true);
- monitor_list_append(&mon->common);
+ NULL, &mon->parent, NULL, true);
+ monitor_list_append(&mon->parent);
}
}
--
2.53.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 02/17] monitor: replace 'common' with 'parent' in MonitorQMP
2026-04-10 16:04 ` [PATCH 02/17] monitor: replace 'common' with 'parent' in MonitorQMP Daniel P. Berrangé
@ 2026-04-14 15:15 ` Mark Cave-Ayland
2026-04-14 15:25 ` Daniel P. Berrangé
0 siblings, 1 reply; 32+ messages in thread
From: Mark Cave-Ayland @ 2026-04-14 15:15 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf, qemu-block,
Dr. David Alan Gilbert, Eric Blake
On 10/04/2026 17:04, Daniel P. Berrangé wrote:
> The field name 'parent' is standard practice for QOM structs
> so align the QMP monitor.
This is not correct: the first field of a QOM object struct should be
called parent_obj, followed by an empty line as per our style guidelines
at
https://qemu.readthedocs.io/en/master/devel/style.html#qemu-object-model-declarations.
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> monitor/monitor-internal.h | 2 +-
> monitor/monitor.c | 4 ++--
> monitor/qmp-cmds-control.c | 4 ++--
> monitor/qmp.c | 40 +++++++++++++++++++-------------------
> 4 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> index ae0cf8c1da..0922588ad9 100644
> --- a/monitor/monitor-internal.h
> +++ b/monitor/monitor-internal.h
> @@ -132,7 +132,7 @@ struct MonitorHMP {
> };
>
> typedef struct {
> - Monitor common;
> + Monitor parent;
> JSONMessageParser parser;
> bool pretty;
> /*
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 6532796edb..6b183edcf5 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -308,7 +308,7 @@ static void monitor_qapi_event_emit(QAPIEvent event, QDict *qdict)
> continue;
> }
>
> - qmp_mon = container_of(mon, MonitorQMP, common);
> + qmp_mon = container_of(mon, MonitorQMP, parent);
> {
> QEMU_LOCK_GUARD(&mon->mon_lock);
> if (qmp_mon->commands == &qmp_cap_negotiation_commands) {
> @@ -625,7 +625,7 @@ void monitor_data_destroy(Monitor *mon)
> g_free(mon->mon_cpu_path);
> qemu_chr_fe_deinit(&mon->chr, false);
> if (monitor_is_qmp(mon)) {
> - monitor_data_destroy_qmp(container_of(mon, MonitorQMP, common));
> + monitor_data_destroy_qmp(container_of(mon, MonitorQMP, parent));
> } else {
> readline_free(container_of(mon, MonitorHMP, parent)->rs);
> }
> diff --git a/monitor/qmp-cmds-control.c b/monitor/qmp-cmds-control.c
> index 150ca9f5cb..ca471bc1c7 100644
> --- a/monitor/qmp-cmds-control.c
> +++ b/monitor/qmp-cmds-control.c
> @@ -76,7 +76,7 @@ void qmp_qmp_capabilities(bool has_enable, QMPCapabilityList *enable,
> MonitorQMP *mon;
>
> assert(monitor_is_qmp(cur_mon));
> - mon = container_of(cur_mon, MonitorQMP, common);
> + mon = container_of(cur_mon, MonitorQMP, parent);
>
> if (mon->commands == &qmp_commands) {
> error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
> @@ -126,7 +126,7 @@ CommandInfoList *qmp_query_commands(Error **errp)
> MonitorQMP *mon;
>
> assert(monitor_is_qmp(cur_mon));
> - mon = container_of(cur_mon, MonitorQMP, common);
> + mon = container_of(cur_mon, MonitorQMP, parent);
>
> qmp_for_each_command(mon->commands, query_commands_cb, &list);
>
> diff --git a/monitor/qmp.c b/monitor/qmp.c
> index 687019811f..69d9e40e32 100644
> --- a/monitor/qmp.c
> +++ b/monitor/qmp.c
> @@ -80,7 +80,7 @@ static void monitor_qmp_caps_reset(MonitorQMP *mon)
> {
> memset(mon->capab_offered, 0, sizeof(mon->capab_offered));
> memset(mon->capab, 0, sizeof(mon->capab));
> - mon->capab_offered[QMP_CAPABILITY_OOB] = mon->common.use_io_thread;
> + mon->capab_offered[QMP_CAPABILITY_OOB] = mon->parent.use_io_thread;
> }
>
> static void qmp_request_free(QMPRequest *req)
> @@ -124,7 +124,7 @@ static void monitor_qmp_cleanup_queue_and_resume(MonitorQMP *mon)
> * when we get here while the monitor is suspended. An
> * unfortunately timed CHR_EVENT_CLOSED can do the trick.
> */
> - monitor_resume(&mon->common);
> + monitor_resume(&mon->parent);
> }
>
> }
> @@ -139,7 +139,7 @@ void qmp_send_response(MonitorQMP *mon, const QDict *rsp)
> trace_monitor_qmp_respond(mon, json->str);
>
> g_string_append_c(json, '\n');
> - monitor_puts(&mon->common, json->str);
> + monitor_puts(&mon->parent, json->str);
>
> g_string_free(json, true);
> }
> @@ -166,7 +166,7 @@ static void monitor_qmp_dispatch(MonitorQMP *mon, QObject *req)
> QDict *error;
>
> rsp = qmp_dispatch(mon->commands, req, qmp_oob_enabled(mon),
> - &mon->common);
> + &mon->parent);
>
> if (mon->commands == &qmp_cap_negotiation_commands) {
> error = qdict_get_qdict(rsp, "error");
> @@ -207,7 +207,7 @@ static QMPRequest *monitor_qmp_requests_pop_any_with_lock(void)
> continue;
> }
>
> - qmp_mon = container_of(mon, MonitorQMP, common);
> + qmp_mon = container_of(mon, MonitorQMP, parent);
> qemu_mutex_lock(&qmp_mon->qmp_queue_lock);
> req_obj = g_queue_pop_head(qmp_mon->qmp_requests);
> if (req_obj) {
> @@ -302,7 +302,7 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
> oob_enabled = qmp_oob_enabled(mon);
> if (oob_enabled
> && mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) {
> - monitor_resume(&mon->common);
> + monitor_resume(&mon->parent);
> }
>
> /*
> @@ -343,7 +343,7 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
> }
>
> if (!oob_enabled) {
> - monitor_resume(&mon->common);
> + monitor_resume(&mon->parent);
> }
>
> qmp_request_free(req_obj);
> @@ -408,7 +408,7 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err)
> */
> if (!qmp_oob_enabled(mon) ||
> mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) {
> - monitor_suspend(&mon->common);
> + monitor_suspend(&mon->parent);
> }
>
> /*
> @@ -462,7 +462,7 @@ static void monitor_qmp_event(void *opaque, QEMUChrEvent event)
>
> switch (event) {
> case CHR_EVENT_OPENED:
> - WITH_QEMU_LOCK_GUARD(&mon->common.mon_lock) {
> + WITH_QEMU_LOCK_GUARD(&mon->parent.mon_lock) {
> mon->commands = &qmp_cap_negotiation_commands;
> monitor_qmp_caps_reset(mon);
> }
> @@ -504,27 +504,27 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
> MonitorQMP *mon = opaque;
> GMainContext *context;
>
> - assert(mon->common.use_io_thread);
> + assert(mon->parent.use_io_thread);
> context = iothread_get_g_main_context(mon_iothread);
> assert(context);
> - qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read,
> + qemu_chr_fe_set_handlers(&mon->parent.chr, monitor_can_read,
> monitor_qmp_read, monitor_qmp_event,
> - NULL, &mon->common, context, true);
> - monitor_list_append(&mon->common);
> + NULL, &mon->parent, context, true);
> + monitor_list_append(&mon->parent);
> }
>
> void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
> {
> MonitorQMP *mon = g_new0(MonitorQMP, 1);
>
> - if (!qemu_chr_fe_init(&mon->common.chr, chr, errp)) {
> + if (!qemu_chr_fe_init(&mon->parent.chr, chr, errp)) {
> g_free(mon);
> return;
> }
> - qemu_chr_fe_set_echo(&mon->common.chr, true);
> + qemu_chr_fe_set_echo(&mon->parent.chr, true);
>
> /* Note: we run QMP monitor in I/O thread when @chr supports that */
> - monitor_data_init(&mon->common, true, false,
> + monitor_data_init(&mon->parent, true, false,
> qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
>
> mon->pretty = pretty;
> @@ -533,7 +533,7 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
> mon->qmp_requests = g_queue_new();
>
> json_message_parser_init(&mon->parser, handle_qmp_command, mon, NULL);
> - if (mon->common.use_io_thread) {
> + if (mon->parent.use_io_thread) {
> /*
> * Make sure the old iowatch is gone. It's possible when
> * e.g. the chardev is in client mode, with wait=on.
> @@ -553,9 +553,9 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
> monitor_qmp_setup_handlers_bh, mon);
> /* The bottom half will add @mon to @mon_list */
> } else {
> - qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read,
> + qemu_chr_fe_set_handlers(&mon->parent.chr, monitor_can_read,
> monitor_qmp_read, monitor_qmp_event,
> - NULL, &mon->common, NULL, true);
> - monitor_list_append(&mon->common);
> + NULL, &mon->parent, NULL, true);
> + monitor_list_append(&mon->parent);
> }
> }
ATB,
Mark.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 02/17] monitor: replace 'common' with 'parent' in MonitorQMP
2026-04-14 15:15 ` Mark Cave-Ayland
@ 2026-04-14 15:25 ` Daniel P. Berrangé
2026-04-14 17:54 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-14 15:25 UTC (permalink / raw)
To: Mark Cave-Ayland
Cc: qemu-devel, Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf, qemu-block,
Dr. David Alan Gilbert, Eric Blake
On Tue, Apr 14, 2026 at 04:15:42PM +0100, Mark Cave-Ayland wrote:
> On 10/04/2026 17:04, Daniel P. Berrangé wrote:
>
> > The field name 'parent' is standard practice for QOM structs
> > so align the QMP monitor.
>
> This is not correct: the first field of a QOM object struct should be called
> parent_obj, followed by an empty line as per our style guidelines at https://qemu.readthedocs.io/en/master/devel/style.html#qemu-object-model-declarations.
Sigh, of course we're massively inconsistent in this :-(
I initially just checked 1st level objects, which favours 'parent'
$ git grep 'Object parent;' | wc -l
34
$ git grep 'Object parent_obj;' | wc -l
18
but I should have checked more deeply
$ git grep -E '(\w+) parent_obj;' | wc -l
859
$ git grep -E '(\w+) parent;' | wc -l
269
I'll switch to 'parent_obj' here.
>
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > monitor/monitor-internal.h | 2 +-
> > monitor/monitor.c | 4 ++--
> > monitor/qmp-cmds-control.c | 4 ++--
> > monitor/qmp.c | 40 +++++++++++++++++++-------------------
> > 4 files changed, 25 insertions(+), 25 deletions(-)
> >
> > diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> > index ae0cf8c1da..0922588ad9 100644
> > --- a/monitor/monitor-internal.h
> > +++ b/monitor/monitor-internal.h
> > @@ -132,7 +132,7 @@ struct MonitorHMP {
> > };
> > typedef struct {
> > - Monitor common;
> > + Monitor parent;
> > JSONMessageParser parser;
> > bool pretty;
> > /*
> > diff --git a/monitor/monitor.c b/monitor/monitor.c
> > index 6532796edb..6b183edcf5 100644
> > --- a/monitor/monitor.c
> > +++ b/monitor/monitor.c
> > @@ -308,7 +308,7 @@ static void monitor_qapi_event_emit(QAPIEvent event, QDict *qdict)
> > continue;
> > }
> > - qmp_mon = container_of(mon, MonitorQMP, common);
> > + qmp_mon = container_of(mon, MonitorQMP, parent);
> > {
> > QEMU_LOCK_GUARD(&mon->mon_lock);
> > if (qmp_mon->commands == &qmp_cap_negotiation_commands) {
> > @@ -625,7 +625,7 @@ void monitor_data_destroy(Monitor *mon)
> > g_free(mon->mon_cpu_path);
> > qemu_chr_fe_deinit(&mon->chr, false);
> > if (monitor_is_qmp(mon)) {
> > - monitor_data_destroy_qmp(container_of(mon, MonitorQMP, common));
> > + monitor_data_destroy_qmp(container_of(mon, MonitorQMP, parent));
> > } else {
> > readline_free(container_of(mon, MonitorHMP, parent)->rs);
> > }
> > diff --git a/monitor/qmp-cmds-control.c b/monitor/qmp-cmds-control.c
> > index 150ca9f5cb..ca471bc1c7 100644
> > --- a/monitor/qmp-cmds-control.c
> > +++ b/monitor/qmp-cmds-control.c
> > @@ -76,7 +76,7 @@ void qmp_qmp_capabilities(bool has_enable, QMPCapabilityList *enable,
> > MonitorQMP *mon;
> > assert(monitor_is_qmp(cur_mon));
> > - mon = container_of(cur_mon, MonitorQMP, common);
> > + mon = container_of(cur_mon, MonitorQMP, parent);
> > if (mon->commands == &qmp_commands) {
> > error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
> > @@ -126,7 +126,7 @@ CommandInfoList *qmp_query_commands(Error **errp)
> > MonitorQMP *mon;
> > assert(monitor_is_qmp(cur_mon));
> > - mon = container_of(cur_mon, MonitorQMP, common);
> > + mon = container_of(cur_mon, MonitorQMP, parent);
> > qmp_for_each_command(mon->commands, query_commands_cb, &list);
> > diff --git a/monitor/qmp.c b/monitor/qmp.c
> > index 687019811f..69d9e40e32 100644
> > --- a/monitor/qmp.c
> > +++ b/monitor/qmp.c
> > @@ -80,7 +80,7 @@ static void monitor_qmp_caps_reset(MonitorQMP *mon)
> > {
> > memset(mon->capab_offered, 0, sizeof(mon->capab_offered));
> > memset(mon->capab, 0, sizeof(mon->capab));
> > - mon->capab_offered[QMP_CAPABILITY_OOB] = mon->common.use_io_thread;
> > + mon->capab_offered[QMP_CAPABILITY_OOB] = mon->parent.use_io_thread;
> > }
> > static void qmp_request_free(QMPRequest *req)
> > @@ -124,7 +124,7 @@ static void monitor_qmp_cleanup_queue_and_resume(MonitorQMP *mon)
> > * when we get here while the monitor is suspended. An
> > * unfortunately timed CHR_EVENT_CLOSED can do the trick.
> > */
> > - monitor_resume(&mon->common);
> > + monitor_resume(&mon->parent);
> > }
> > }
> > @@ -139,7 +139,7 @@ void qmp_send_response(MonitorQMP *mon, const QDict *rsp)
> > trace_monitor_qmp_respond(mon, json->str);
> > g_string_append_c(json, '\n');
> > - monitor_puts(&mon->common, json->str);
> > + monitor_puts(&mon->parent, json->str);
> > g_string_free(json, true);
> > }
> > @@ -166,7 +166,7 @@ static void monitor_qmp_dispatch(MonitorQMP *mon, QObject *req)
> > QDict *error;
> > rsp = qmp_dispatch(mon->commands, req, qmp_oob_enabled(mon),
> > - &mon->common);
> > + &mon->parent);
> > if (mon->commands == &qmp_cap_negotiation_commands) {
> > error = qdict_get_qdict(rsp, "error");
> > @@ -207,7 +207,7 @@ static QMPRequest *monitor_qmp_requests_pop_any_with_lock(void)
> > continue;
> > }
> > - qmp_mon = container_of(mon, MonitorQMP, common);
> > + qmp_mon = container_of(mon, MonitorQMP, parent);
> > qemu_mutex_lock(&qmp_mon->qmp_queue_lock);
> > req_obj = g_queue_pop_head(qmp_mon->qmp_requests);
> > if (req_obj) {
> > @@ -302,7 +302,7 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
> > oob_enabled = qmp_oob_enabled(mon);
> > if (oob_enabled
> > && mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) {
> > - monitor_resume(&mon->common);
> > + monitor_resume(&mon->parent);
> > }
> > /*
> > @@ -343,7 +343,7 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
> > }
> > if (!oob_enabled) {
> > - monitor_resume(&mon->common);
> > + monitor_resume(&mon->parent);
> > }
> > qmp_request_free(req_obj);
> > @@ -408,7 +408,7 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err)
> > */
> > if (!qmp_oob_enabled(mon) ||
> > mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) {
> > - monitor_suspend(&mon->common);
> > + monitor_suspend(&mon->parent);
> > }
> > /*
> > @@ -462,7 +462,7 @@ static void monitor_qmp_event(void *opaque, QEMUChrEvent event)
> > switch (event) {
> > case CHR_EVENT_OPENED:
> > - WITH_QEMU_LOCK_GUARD(&mon->common.mon_lock) {
> > + WITH_QEMU_LOCK_GUARD(&mon->parent.mon_lock) {
> > mon->commands = &qmp_cap_negotiation_commands;
> > monitor_qmp_caps_reset(mon);
> > }
> > @@ -504,27 +504,27 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
> > MonitorQMP *mon = opaque;
> > GMainContext *context;
> > - assert(mon->common.use_io_thread);
> > + assert(mon->parent.use_io_thread);
> > context = iothread_get_g_main_context(mon_iothread);
> > assert(context);
> > - qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read,
> > + qemu_chr_fe_set_handlers(&mon->parent.chr, monitor_can_read,
> > monitor_qmp_read, monitor_qmp_event,
> > - NULL, &mon->common, context, true);
> > - monitor_list_append(&mon->common);
> > + NULL, &mon->parent, context, true);
> > + monitor_list_append(&mon->parent);
> > }
> > void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
> > {
> > MonitorQMP *mon = g_new0(MonitorQMP, 1);
> > - if (!qemu_chr_fe_init(&mon->common.chr, chr, errp)) {
> > + if (!qemu_chr_fe_init(&mon->parent.chr, chr, errp)) {
> > g_free(mon);
> > return;
> > }
> > - qemu_chr_fe_set_echo(&mon->common.chr, true);
> > + qemu_chr_fe_set_echo(&mon->parent.chr, true);
> > /* Note: we run QMP monitor in I/O thread when @chr supports that */
> > - monitor_data_init(&mon->common, true, false,
> > + monitor_data_init(&mon->parent, true, false,
> > qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
> > mon->pretty = pretty;
> > @@ -533,7 +533,7 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
> > mon->qmp_requests = g_queue_new();
> > json_message_parser_init(&mon->parser, handle_qmp_command, mon, NULL);
> > - if (mon->common.use_io_thread) {
> > + if (mon->parent.use_io_thread) {
> > /*
> > * Make sure the old iowatch is gone. It's possible when
> > * e.g. the chardev is in client mode, with wait=on.
> > @@ -553,9 +553,9 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
> > monitor_qmp_setup_handlers_bh, mon);
> > /* The bottom half will add @mon to @mon_list */
> > } else {
> > - qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read,
> > + qemu_chr_fe_set_handlers(&mon->parent.chr, monitor_can_read,
> > monitor_qmp_read, monitor_qmp_event,
> > - NULL, &mon->common, NULL, true);
> > - monitor_list_append(&mon->common);
> > + NULL, &mon->parent, NULL, true);
> > + monitor_list_append(&mon->parent);
> > }
> > }
>
>
> ATB,
>
> Mark.
>
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 :|
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 02/17] monitor: replace 'common' with 'parent' in MonitorQMP
2026-04-14 15:25 ` Daniel P. Berrangé
@ 2026-04-14 17:54 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2026-04-14 17:54 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Mark Cave-Ayland, qemu-devel, Paolo Bonzini, Markus Armbruster,
Christian Brauner, Alex Bennée, Philippe Mathieu-Daudé,
Fabiano Rosas, Marc-André Lureau, Peter Xu, Kevin Wolf,
qemu-block, Eric Blake
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Tue, Apr 14, 2026 at 04:15:42PM +0100, Mark Cave-Ayland wrote:
> > On 10/04/2026 17:04, Daniel P. Berrangé wrote:
> >
> > > The field name 'parent' is standard practice for QOM structs
> > > so align the QMP monitor.
> >
> > This is not correct: the first field of a QOM object struct should be called
> > parent_obj, followed by an empty line as per our style guidelines at https://qemu.readthedocs.io/en/master/devel/style.html#qemu-object-model-declarations.
>
> Sigh, of course we're massively inconsistent in this :-(
Because it's all manual!
It needs macro'ing, and linting.
Dave
> I initially just checked 1st level objects, which favours 'parent'
>
> $ git grep 'Object parent;' | wc -l
> 34
> $ git grep 'Object parent_obj;' | wc -l
> 18
>
> but I should have checked more deeply
>
> $ git grep -E '(\w+) parent_obj;' | wc -l
> 859
> $ git grep -E '(\w+) parent;' | wc -l
> 269
>
> I'll switch to 'parent_obj' here.
>
> >
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > > monitor/monitor-internal.h | 2 +-
> > > monitor/monitor.c | 4 ++--
> > > monitor/qmp-cmds-control.c | 4 ++--
> > > monitor/qmp.c | 40 +++++++++++++++++++-------------------
> > > 4 files changed, 25 insertions(+), 25 deletions(-)
> > >
> > > diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> > > index ae0cf8c1da..0922588ad9 100644
> > > --- a/monitor/monitor-internal.h
> > > +++ b/monitor/monitor-internal.h
> > > @@ -132,7 +132,7 @@ struct MonitorHMP {
> > > };
> > > typedef struct {
> > > - Monitor common;
> > > + Monitor parent;
> > > JSONMessageParser parser;
> > > bool pretty;
> > > /*
> > > diff --git a/monitor/monitor.c b/monitor/monitor.c
> > > index 6532796edb..6b183edcf5 100644
> > > --- a/monitor/monitor.c
> > > +++ b/monitor/monitor.c
> > > @@ -308,7 +308,7 @@ static void monitor_qapi_event_emit(QAPIEvent event, QDict *qdict)
> > > continue;
> > > }
> > > - qmp_mon = container_of(mon, MonitorQMP, common);
> > > + qmp_mon = container_of(mon, MonitorQMP, parent);
> > > {
> > > QEMU_LOCK_GUARD(&mon->mon_lock);
> > > if (qmp_mon->commands == &qmp_cap_negotiation_commands) {
> > > @@ -625,7 +625,7 @@ void monitor_data_destroy(Monitor *mon)
> > > g_free(mon->mon_cpu_path);
> > > qemu_chr_fe_deinit(&mon->chr, false);
> > > if (monitor_is_qmp(mon)) {
> > > - monitor_data_destroy_qmp(container_of(mon, MonitorQMP, common));
> > > + monitor_data_destroy_qmp(container_of(mon, MonitorQMP, parent));
> > > } else {
> > > readline_free(container_of(mon, MonitorHMP, parent)->rs);
> > > }
> > > diff --git a/monitor/qmp-cmds-control.c b/monitor/qmp-cmds-control.c
> > > index 150ca9f5cb..ca471bc1c7 100644
> > > --- a/monitor/qmp-cmds-control.c
> > > +++ b/monitor/qmp-cmds-control.c
> > > @@ -76,7 +76,7 @@ void qmp_qmp_capabilities(bool has_enable, QMPCapabilityList *enable,
> > > MonitorQMP *mon;
> > > assert(monitor_is_qmp(cur_mon));
> > > - mon = container_of(cur_mon, MonitorQMP, common);
> > > + mon = container_of(cur_mon, MonitorQMP, parent);
> > > if (mon->commands == &qmp_commands) {
> > > error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
> > > @@ -126,7 +126,7 @@ CommandInfoList *qmp_query_commands(Error **errp)
> > > MonitorQMP *mon;
> > > assert(monitor_is_qmp(cur_mon));
> > > - mon = container_of(cur_mon, MonitorQMP, common);
> > > + mon = container_of(cur_mon, MonitorQMP, parent);
> > > qmp_for_each_command(mon->commands, query_commands_cb, &list);
> > > diff --git a/monitor/qmp.c b/monitor/qmp.c
> > > index 687019811f..69d9e40e32 100644
> > > --- a/monitor/qmp.c
> > > +++ b/monitor/qmp.c
> > > @@ -80,7 +80,7 @@ static void monitor_qmp_caps_reset(MonitorQMP *mon)
> > > {
> > > memset(mon->capab_offered, 0, sizeof(mon->capab_offered));
> > > memset(mon->capab, 0, sizeof(mon->capab));
> > > - mon->capab_offered[QMP_CAPABILITY_OOB] = mon->common.use_io_thread;
> > > + mon->capab_offered[QMP_CAPABILITY_OOB] = mon->parent.use_io_thread;
> > > }
> > > static void qmp_request_free(QMPRequest *req)
> > > @@ -124,7 +124,7 @@ static void monitor_qmp_cleanup_queue_and_resume(MonitorQMP *mon)
> > > * when we get here while the monitor is suspended. An
> > > * unfortunately timed CHR_EVENT_CLOSED can do the trick.
> > > */
> > > - monitor_resume(&mon->common);
> > > + monitor_resume(&mon->parent);
> > > }
> > > }
> > > @@ -139,7 +139,7 @@ void qmp_send_response(MonitorQMP *mon, const QDict *rsp)
> > > trace_monitor_qmp_respond(mon, json->str);
> > > g_string_append_c(json, '\n');
> > > - monitor_puts(&mon->common, json->str);
> > > + monitor_puts(&mon->parent, json->str);
> > > g_string_free(json, true);
> > > }
> > > @@ -166,7 +166,7 @@ static void monitor_qmp_dispatch(MonitorQMP *mon, QObject *req)
> > > QDict *error;
> > > rsp = qmp_dispatch(mon->commands, req, qmp_oob_enabled(mon),
> > > - &mon->common);
> > > + &mon->parent);
> > > if (mon->commands == &qmp_cap_negotiation_commands) {
> > > error = qdict_get_qdict(rsp, "error");
> > > @@ -207,7 +207,7 @@ static QMPRequest *monitor_qmp_requests_pop_any_with_lock(void)
> > > continue;
> > > }
> > > - qmp_mon = container_of(mon, MonitorQMP, common);
> > > + qmp_mon = container_of(mon, MonitorQMP, parent);
> > > qemu_mutex_lock(&qmp_mon->qmp_queue_lock);
> > > req_obj = g_queue_pop_head(qmp_mon->qmp_requests);
> > > if (req_obj) {
> > > @@ -302,7 +302,7 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
> > > oob_enabled = qmp_oob_enabled(mon);
> > > if (oob_enabled
> > > && mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) {
> > > - monitor_resume(&mon->common);
> > > + monitor_resume(&mon->parent);
> > > }
> > > /*
> > > @@ -343,7 +343,7 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
> > > }
> > > if (!oob_enabled) {
> > > - monitor_resume(&mon->common);
> > > + monitor_resume(&mon->parent);
> > > }
> > > qmp_request_free(req_obj);
> > > @@ -408,7 +408,7 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err)
> > > */
> > > if (!qmp_oob_enabled(mon) ||
> > > mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) {
> > > - monitor_suspend(&mon->common);
> > > + monitor_suspend(&mon->parent);
> > > }
> > > /*
> > > @@ -462,7 +462,7 @@ static void monitor_qmp_event(void *opaque, QEMUChrEvent event)
> > > switch (event) {
> > > case CHR_EVENT_OPENED:
> > > - WITH_QEMU_LOCK_GUARD(&mon->common.mon_lock) {
> > > + WITH_QEMU_LOCK_GUARD(&mon->parent.mon_lock) {
> > > mon->commands = &qmp_cap_negotiation_commands;
> > > monitor_qmp_caps_reset(mon);
> > > }
> > > @@ -504,27 +504,27 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
> > > MonitorQMP *mon = opaque;
> > > GMainContext *context;
> > > - assert(mon->common.use_io_thread);
> > > + assert(mon->parent.use_io_thread);
> > > context = iothread_get_g_main_context(mon_iothread);
> > > assert(context);
> > > - qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read,
> > > + qemu_chr_fe_set_handlers(&mon->parent.chr, monitor_can_read,
> > > monitor_qmp_read, monitor_qmp_event,
> > > - NULL, &mon->common, context, true);
> > > - monitor_list_append(&mon->common);
> > > + NULL, &mon->parent, context, true);
> > > + monitor_list_append(&mon->parent);
> > > }
> > > void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
> > > {
> > > MonitorQMP *mon = g_new0(MonitorQMP, 1);
> > > - if (!qemu_chr_fe_init(&mon->common.chr, chr, errp)) {
> > > + if (!qemu_chr_fe_init(&mon->parent.chr, chr, errp)) {
> > > g_free(mon);
> > > return;
> > > }
> > > - qemu_chr_fe_set_echo(&mon->common.chr, true);
> > > + qemu_chr_fe_set_echo(&mon->parent.chr, true);
> > > /* Note: we run QMP monitor in I/O thread when @chr supports that */
> > > - monitor_data_init(&mon->common, true, false,
> > > + monitor_data_init(&mon->parent, true, false,
> > > qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
> > > mon->pretty = pretty;
> > > @@ -533,7 +533,7 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
> > > mon->qmp_requests = g_queue_new();
> > > json_message_parser_init(&mon->parser, handle_qmp_command, mon, NULL);
> > > - if (mon->common.use_io_thread) {
> > > + if (mon->parent.use_io_thread) {
> > > /*
> > > * Make sure the old iowatch is gone. It's possible when
> > > * e.g. the chardev is in client mode, with wait=on.
> > > @@ -553,9 +553,9 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
> > > monitor_qmp_setup_handlers_bh, mon);
> > > /* The bottom half will add @mon to @mon_list */
> > > } else {
> > > - qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read,
> > > + qemu_chr_fe_set_handlers(&mon->parent.chr, monitor_can_read,
> > > monitor_qmp_read, monitor_qmp_event,
> > > - NULL, &mon->common, NULL, true);
> > > - monitor_list_append(&mon->common);
> > > + NULL, &mon->parent, NULL, true);
> > > + monitor_list_append(&mon->parent);
> > > }
> > > }
> >
> >
> > ATB,
> >
> > Mark.
> >
>
> 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 :|
>
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 03/17] monitor: rename monitor_init* to monitor_new*
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
2026-04-10 16:04 ` [PATCH 01/17] monitor: replace 'common' with 'parent' in MonitorHMP Daniel P. Berrangé
2026-04-10 16:04 ` [PATCH 02/17] monitor: replace 'common' with 'parent' in MonitorQMP Daniel P. Berrangé
@ 2026-04-10 16:04 ` Daniel P. Berrangé
2026-04-10 22:57 ` Dr. David Alan Gilbert
2026-04-10 16:04 ` [PATCH 04/17] monitor: minimal conversion of monitors to QOM Daniel P. Berrangé
` (14 subsequent siblings)
17 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf,
Daniel P. Berrangé, qemu-block, Dr. David Alan Gilbert,
Eric Blake
The current "monitor_init" functions will clash with the methods of the
same name that are required by QOM. To ease the transition to QOM,
rename them out of the way.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
chardev/char.c | 2 +-
gdbstub/system.c | 2 +-
include/monitor/monitor.h | 8 ++++----
monitor/hmp.c | 2 +-
monitor/monitor.c | 10 +++++-----
monitor/qmp.c | 2 +-
storage-daemon/qemu-storage-daemon.c | 2 +-
stubs/monitor-internal.c | 2 +-
system/vl.c | 2 +-
9 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/chardev/char.c b/chardev/char.c
index 48b326d57b..b59972f325 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -786,7 +786,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_init_hmp(chr, true, &err);
+ monitor_new_hmp(chr, true, &err);
if (err) {
error_report_err(err);
object_unparent(OBJECT(chr));
diff --git a/gdbstub/system.c b/gdbstub/system.c
index e86c5870ab..20dcf7878d 100644
--- a/gdbstub/system.c
+++ b/gdbstub/system.c
@@ -388,7 +388,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_init_hmp(mon_chr, false, &error_abort);
+ monitor_new_hmp(mon_chr, 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 296690e1f1..bfbb440550 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -19,10 +19,10 @@ bool monitor_cur_is_qmp(void);
void monitor_init_globals(void);
void monitor_init_globals_core(void);
-void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp);
-void monitor_init_hmp(Chardev *chr, bool use_readline, Error **errp);
-int monitor_init(MonitorOptions *opts, bool allow_hmp, Error **errp);
-int monitor_init_opts(QemuOpts *opts, Error **errp);
+void monitor_new_qmp(Chardev *chr, bool pretty, Error **errp);
+void monitor_new_hmp(Chardev *chr, bool use_readline, Error **errp);
+int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp);
+int monitor_new_opts(QemuOpts *opts, Error **errp);
void monitor_cleanup(void);
int monitor_suspend(Monitor *mon);
diff --git a/monitor/hmp.c b/monitor/hmp.c
index af346d190b..6dfeca84b5 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -1516,7 +1516,7 @@ static void monitor_readline_flush(void *opaque)
monitor_flush(&mon->parent);
}
-void monitor_init_hmp(Chardev *chr, bool use_readline, Error **errp)
+void monitor_new_hmp(Chardev *chr, bool use_readline, Error **errp)
{
MonitorHMP *mon = g_new0(MonitorHMP, 1);
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 6b183edcf5..393fb7795e 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -715,7 +715,7 @@ void monitor_init_globals(void)
aio_co_schedule(iohandler_get_aio_context(), qmp_dispatcher_co);
}
-int monitor_init(MonitorOptions *opts, bool allow_hmp, Error **errp)
+int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp)
{
ERRP_GUARD();
Chardev *chr;
@@ -732,7 +732,7 @@ int monitor_init(MonitorOptions *opts, bool allow_hmp, Error **errp)
switch (opts->mode) {
case MONITOR_MODE_CONTROL:
- monitor_init_qmp(chr, opts->pretty, errp);
+ monitor_new_qmp(chr, opts->pretty, errp);
break;
case MONITOR_MODE_READLINE:
if (!allow_hmp) {
@@ -743,7 +743,7 @@ int monitor_init(MonitorOptions *opts, bool allow_hmp, Error **errp)
error_setg(errp, "'pretty' is not compatible with HMP monitors");
return -1;
}
- monitor_init_hmp(chr, true, errp);
+ monitor_new_hmp(chr, true, errp);
break;
default:
g_assert_not_reached();
@@ -752,7 +752,7 @@ int monitor_init(MonitorOptions *opts, bool allow_hmp, Error **errp)
return *errp ? -1 : 0;
}
-int monitor_init_opts(QemuOpts *opts, Error **errp)
+int monitor_new_opts(QemuOpts *opts, Error **errp)
{
Visitor *v;
MonitorOptions *options;
@@ -765,7 +765,7 @@ int monitor_init_opts(QemuOpts *opts, Error **errp)
return -1;
}
- ret = monitor_init(options, true, errp);
+ ret = monitor_new(options, true, errp);
qapi_free_MonitorOptions(options);
return ret;
}
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 69d9e40e32..9caee70624 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -513,7 +513,7 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
monitor_list_append(&mon->parent);
}
-void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
+void monitor_new_qmp(Chardev *chr, bool pretty, Error **errp)
{
MonitorQMP *mon = g_new0(MonitorQMP, 1);
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index eb72561358..50dbfbd97a 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -330,7 +330,7 @@ static void process_options(int argc, char *argv[], bool pre_init_pass)
visit_free(v);
/* TODO Catch duplicate monitor IDs */
- monitor_init(monitor, false, &error_fatal);
+ monitor_new(monitor, false, &error_fatal);
qapi_free_MonitorOptions(monitor);
break;
}
diff --git a/stubs/monitor-internal.c b/stubs/monitor-internal.c
index 4fece49d53..23d58da184 100644
--- a/stubs/monitor-internal.c
+++ b/stubs/monitor-internal.c
@@ -8,6 +8,6 @@ int monitor_get_fd(Monitor *mon, const char *name, Error **errp)
return -1;
}
-void monitor_init_hmp(Chardev *chr, bool use_readline, Error **errp)
+void monitor_new_hmp(Chardev *chr, bool use_readline, Error **errp)
{
}
diff --git a/system/vl.c b/system/vl.c
index 246623b319..2391811a46 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1244,7 +1244,7 @@ static int fsdev_init_func(void *opaque, QemuOpts *opts, Error **errp)
static int mon_init_func(void *opaque, QemuOpts *opts, Error **errp)
{
- return monitor_init_opts(opts, errp);
+ return monitor_new_opts(opts, errp);
}
static void monitor_parse(const char *str, const char *mode, bool pretty)
--
2.53.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 03/17] monitor: rename monitor_init* to monitor_new*
2026-04-10 16:04 ` [PATCH 03/17] monitor: rename monitor_init* to monitor_new* Daniel P. Berrangé
@ 2026-04-10 22:57 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2026-04-10 22:57 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf, qemu-block,
Eric Blake
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> The current "monitor_init" functions will clash with the methods of the
> same name that are required by QOM. To ease the transition to QOM,
> rename them out of the way.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
> ---
> chardev/char.c | 2 +-
> gdbstub/system.c | 2 +-
> include/monitor/monitor.h | 8 ++++----
> monitor/hmp.c | 2 +-
> monitor/monitor.c | 10 +++++-----
> monitor/qmp.c | 2 +-
> storage-daemon/qemu-storage-daemon.c | 2 +-
> stubs/monitor-internal.c | 2 +-
> system/vl.c | 2 +-
> 9 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index 48b326d57b..b59972f325 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -786,7 +786,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_init_hmp(chr, true, &err);
> + monitor_new_hmp(chr, true, &err);
> if (err) {
> error_report_err(err);
> object_unparent(OBJECT(chr));
> diff --git a/gdbstub/system.c b/gdbstub/system.c
> index e86c5870ab..20dcf7878d 100644
> --- a/gdbstub/system.c
> +++ b/gdbstub/system.c
> @@ -388,7 +388,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_init_hmp(mon_chr, false, &error_abort);
> + monitor_new_hmp(mon_chr, 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 296690e1f1..bfbb440550 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -19,10 +19,10 @@ bool monitor_cur_is_qmp(void);
>
> void monitor_init_globals(void);
> void monitor_init_globals_core(void);
> -void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp);
> -void monitor_init_hmp(Chardev *chr, bool use_readline, Error **errp);
> -int monitor_init(MonitorOptions *opts, bool allow_hmp, Error **errp);
> -int monitor_init_opts(QemuOpts *opts, Error **errp);
> +void monitor_new_qmp(Chardev *chr, bool pretty, Error **errp);
> +void monitor_new_hmp(Chardev *chr, bool use_readline, Error **errp);
> +int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp);
> +int monitor_new_opts(QemuOpts *opts, Error **errp);
> void monitor_cleanup(void);
>
> int monitor_suspend(Monitor *mon);
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index af346d190b..6dfeca84b5 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -1516,7 +1516,7 @@ static void monitor_readline_flush(void *opaque)
> monitor_flush(&mon->parent);
> }
>
> -void monitor_init_hmp(Chardev *chr, bool use_readline, Error **errp)
> +void monitor_new_hmp(Chardev *chr, bool use_readline, Error **errp)
> {
> MonitorHMP *mon = g_new0(MonitorHMP, 1);
>
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 6b183edcf5..393fb7795e 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -715,7 +715,7 @@ void monitor_init_globals(void)
> aio_co_schedule(iohandler_get_aio_context(), qmp_dispatcher_co);
> }
>
> -int monitor_init(MonitorOptions *opts, bool allow_hmp, Error **errp)
> +int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp)
> {
> ERRP_GUARD();
> Chardev *chr;
> @@ -732,7 +732,7 @@ int monitor_init(MonitorOptions *opts, bool allow_hmp, Error **errp)
>
> switch (opts->mode) {
> case MONITOR_MODE_CONTROL:
> - monitor_init_qmp(chr, opts->pretty, errp);
> + monitor_new_qmp(chr, opts->pretty, errp);
> break;
> case MONITOR_MODE_READLINE:
> if (!allow_hmp) {
> @@ -743,7 +743,7 @@ int monitor_init(MonitorOptions *opts, bool allow_hmp, Error **errp)
> error_setg(errp, "'pretty' is not compatible with HMP monitors");
> return -1;
> }
> - monitor_init_hmp(chr, true, errp);
> + monitor_new_hmp(chr, true, errp);
> break;
> default:
> g_assert_not_reached();
> @@ -752,7 +752,7 @@ int monitor_init(MonitorOptions *opts, bool allow_hmp, Error **errp)
> return *errp ? -1 : 0;
> }
>
> -int monitor_init_opts(QemuOpts *opts, Error **errp)
> +int monitor_new_opts(QemuOpts *opts, Error **errp)
> {
> Visitor *v;
> MonitorOptions *options;
> @@ -765,7 +765,7 @@ int monitor_init_opts(QemuOpts *opts, Error **errp)
> return -1;
> }
>
> - ret = monitor_init(options, true, errp);
> + ret = monitor_new(options, true, errp);
> qapi_free_MonitorOptions(options);
> return ret;
> }
> diff --git a/monitor/qmp.c b/monitor/qmp.c
> index 69d9e40e32..9caee70624 100644
> --- a/monitor/qmp.c
> +++ b/monitor/qmp.c
> @@ -513,7 +513,7 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
> monitor_list_append(&mon->parent);
> }
>
> -void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
> +void monitor_new_qmp(Chardev *chr, bool pretty, Error **errp)
> {
> MonitorQMP *mon = g_new0(MonitorQMP, 1);
>
> diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
> index eb72561358..50dbfbd97a 100644
> --- a/storage-daemon/qemu-storage-daemon.c
> +++ b/storage-daemon/qemu-storage-daemon.c
> @@ -330,7 +330,7 @@ static void process_options(int argc, char *argv[], bool pre_init_pass)
> visit_free(v);
>
> /* TODO Catch duplicate monitor IDs */
> - monitor_init(monitor, false, &error_fatal);
> + monitor_new(monitor, false, &error_fatal);
> qapi_free_MonitorOptions(monitor);
> break;
> }
> diff --git a/stubs/monitor-internal.c b/stubs/monitor-internal.c
> index 4fece49d53..23d58da184 100644
> --- a/stubs/monitor-internal.c
> +++ b/stubs/monitor-internal.c
> @@ -8,6 +8,6 @@ int monitor_get_fd(Monitor *mon, const char *name, Error **errp)
> return -1;
> }
>
> -void monitor_init_hmp(Chardev *chr, bool use_readline, Error **errp)
> +void monitor_new_hmp(Chardev *chr, bool use_readline, Error **errp)
> {
> }
> diff --git a/system/vl.c b/system/vl.c
> index 246623b319..2391811a46 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -1244,7 +1244,7 @@ static int fsdev_init_func(void *opaque, QemuOpts *opts, Error **errp)
>
> static int mon_init_func(void *opaque, QemuOpts *opts, Error **errp)
> {
> - return monitor_init_opts(opts, errp);
> + return monitor_new_opts(opts, errp);
> }
>
> static void monitor_parse(const char *str, const char *mode, bool pretty)
> --
> 2.53.0
>
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 04/17] monitor: minimal conversion of monitors to QOM
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
` (2 preceding siblings ...)
2026-04-10 16:04 ` [PATCH 03/17] monitor: rename monitor_init* to monitor_new* Daniel P. Berrangé
@ 2026-04-10 16:04 ` Daniel P. Berrangé
2026-04-10 23:32 ` Dr. David Alan Gilbert
2026-04-10 16:04 ` [PATCH 05/17] monitor: remove 'skip_flush' field Daniel P. Berrangé
` (13 subsequent siblings)
17 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf,
Daniel P. Berrangé, qemu-block, Dr. David Alan Gilbert,
Eric Blake
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.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
include/monitor/monitor.h | 11 ++++++++++-
monitor/hmp.c | 18 ++++++++++++++++--
monitor/monitor-internal.h | 18 ++++++++++++++++--
monitor/monitor.c | 18 ++++++++++++++++--
monitor/qmp-cmds.c | 15 ++++++++-------
monitor/qmp.c | 18 ++++++++++++++++--
6 files changed, 82 insertions(+), 16 deletions(-)
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index bfbb440550..b349b12c34 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 6dfeca84b5..614d9a3707 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -42,6 +42,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)
{
@@ -1518,10 +1532,10 @@ 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 = MONITOR_HMP(object_new(TYPE_MONITOR_HMP));
if (!qemu_chr_fe_init(&mon->parent.chr, chr, errp)) {
- g_free(mon);
+ object_unref(mon);
return;
}
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 0922588ad9..25320928a7 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -92,7 +92,13 @@ typedef struct HMPCommand {
void (*command_completion)(ReadLineState *rs, int nb_args, const char *str);
} HMPCommand;
+
+struct MonitorClass {
+ ObjectClass parent_class;
+};
+
struct Monitor {
+ Object parent;
CharFrontend chr;
int suspend_cnt; /* Needs to be accessed atomically */
bool is_qmp;
@@ -118,6 +124,10 @@ struct Monitor {
int reset_seen;
};
+struct MonitorHMPClass {
+ MonitorClass parent_class;
+};
+
struct MonitorHMP {
Monitor parent;
bool use_readline;
@@ -131,7 +141,11 @@ struct MonitorHMP {
ReadLineState *rs;
};
-typedef struct {
+struct MonitorQMPClass {
+ MonitorClass parent_class;
+};
+
+struct MonitorQMP {
Monitor parent;
JSONMessageParser parser;
bool pretty;
@@ -151,7 +165,7 @@ typedef struct {
QemuMutex qmp_queue_lock;
/* Input queue that holds all the parsed QMP requests */
GQueue *qmp_requests;
-} MonitorQMP;
+};
/**
* Is @mon a QMP monitor?
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 393fb7795e..7936e2ab22 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -73,6 +73,20 @@ static GHashTable *coroutine_mon; /* Maps Coroutine* to Monitor* */
MonitorList mon_list;
static bool monitor_destroyed;
+OBJECT_DEFINE_TYPE(Monitor, monitor, MONITOR, OBJECT);
+
+static void monitor_finalize(Object *obj)
+{
+}
+
+static void monitor_class_init(ObjectClass *cls, const void *data)
+{
+}
+
+static void monitor_init(Object *obj)
+{
+}
+
Monitor *monitor_cur(void)
{
Monitor *mon;
@@ -598,7 +612,7 @@ void monitor_list_append(Monitor *mon)
if (mon) {
monitor_data_destroy(mon);
- g_free(mon);
+ object_unref(mon);
}
}
@@ -680,7 +694,7 @@ void monitor_cleanup(void)
monitor_flush(mon);
monitor_data_destroy(mon);
qemu_mutex_lock(&monitor_lock);
- g_free(mon);
+ object_unref(mon);
}
qemu_mutex_unlock(&monitor_lock);
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 191eba1b3a..d0ed241d6a 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -166,12 +166,12 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
int64_t cpu_index, Error **errp)
{
char *output = NULL;
- MonitorHMP hmp = {};
+ MonitorHMP *hmp = MONITOR_HMP(object_new(TYPE_MONITOR_HMP));
- monitor_data_init(&hmp.parent, false, true, false);
+ monitor_data_init(&hmp->parent, false, true, false);
if (has_cpu_index) {
- int ret = monitor_set_cpu(&hmp.parent, cpu_index);
+ int ret = monitor_set_cpu(&hmp->parent, cpu_index);
if (ret < 0) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "cpu-index",
"a CPU number");
@@ -179,14 +179,15 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
}
}
- handle_hmp_command(&hmp, command_line);
+ handle_hmp_command(hmp, command_line);
- WITH_QEMU_LOCK_GUARD(&hmp.parent.mon_lock) {
- output = g_strdup(hmp.parent.outbuf->str);
+ WITH_QEMU_LOCK_GUARD(&hmp->parent.mon_lock) {
+ output = g_strdup(hmp->parent.outbuf->str);
}
out:
- monitor_data_destroy(&hmp.parent);
+ monitor_data_destroy(&hmp->parent);
+ object_unref(hmp);
return output;
}
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 9caee70624..36cb078f30 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -71,6 +71,20 @@ typedef struct QMPRequest QMPRequest;
QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
+OBJECT_DEFINE_TYPE(MonitorQMP, monitor_qmp, MONITOR_QMP, MONITOR);
+
+static void monitor_qmp_finalize(Object *obj)
+{
+}
+
+static void monitor_qmp_class_init(ObjectClass *cls, const void *data)
+{
+}
+
+static void monitor_qmp_init(Object *obj)
+{
+}
+
static bool qmp_oob_enabled(MonitorQMP *mon)
{
return mon->capab[QMP_CAPABILITY_OOB];
@@ -515,10 +529,10 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
void monitor_new_qmp(Chardev *chr, bool pretty, Error **errp)
{
- MonitorQMP *mon = g_new0(MonitorQMP, 1);
+ MonitorQMP *mon = MONITOR_QMP(object_new(TYPE_MONITOR_QMP));
if (!qemu_chr_fe_init(&mon->parent.chr, chr, errp)) {
- g_free(mon);
+ object_unref(mon);
return;
}
qemu_chr_fe_set_echo(&mon->parent.chr, true);
--
2.53.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 04/17] monitor: minimal conversion of monitors to QOM
2026-04-10 16:04 ` [PATCH 04/17] monitor: minimal conversion of monitors to QOM Daniel P. Berrangé
@ 2026-04-10 23:32 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2026-04-10 23:32 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf, qemu-block,
Eric Blake
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> 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.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Looks OK, but I'm not that familiar with QOM, so just:
Acked-by: Dr. David Alan Gilbert <dave@treblig.org>
> ---
> include/monitor/monitor.h | 11 ++++++++++-
> monitor/hmp.c | 18 ++++++++++++++++--
> monitor/monitor-internal.h | 18 ++++++++++++++++--
> monitor/monitor.c | 18 ++++++++++++++++--
> monitor/qmp-cmds.c | 15 ++++++++-------
> monitor/qmp.c | 18 ++++++++++++++++--
> 6 files changed, 82 insertions(+), 16 deletions(-)
>
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index bfbb440550..b349b12c34 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 6dfeca84b5..614d9a3707 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -42,6 +42,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)
> {
> @@ -1518,10 +1532,10 @@ 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 = MONITOR_HMP(object_new(TYPE_MONITOR_HMP));
>
> if (!qemu_chr_fe_init(&mon->parent.chr, chr, errp)) {
> - g_free(mon);
> + object_unref(mon);
> return;
> }
>
> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> index 0922588ad9..25320928a7 100644
> --- a/monitor/monitor-internal.h
> +++ b/monitor/monitor-internal.h
> @@ -92,7 +92,13 @@ typedef struct HMPCommand {
> void (*command_completion)(ReadLineState *rs, int nb_args, const char *str);
> } HMPCommand;
>
> +
> +struct MonitorClass {
> + ObjectClass parent_class;
> +};
> +
> struct Monitor {
> + Object parent;
> CharFrontend chr;
> int suspend_cnt; /* Needs to be accessed atomically */
> bool is_qmp;
> @@ -118,6 +124,10 @@ struct Monitor {
> int reset_seen;
> };
>
> +struct MonitorHMPClass {
> + MonitorClass parent_class;
> +};
> +
> struct MonitorHMP {
> Monitor parent;
> bool use_readline;
> @@ -131,7 +141,11 @@ struct MonitorHMP {
> ReadLineState *rs;
> };
>
> -typedef struct {
> +struct MonitorQMPClass {
> + MonitorClass parent_class;
> +};
> +
> +struct MonitorQMP {
> Monitor parent;
> JSONMessageParser parser;
> bool pretty;
> @@ -151,7 +165,7 @@ typedef struct {
> QemuMutex qmp_queue_lock;
> /* Input queue that holds all the parsed QMP requests */
> GQueue *qmp_requests;
> -} MonitorQMP;
> +};
>
> /**
> * Is @mon a QMP monitor?
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 393fb7795e..7936e2ab22 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -73,6 +73,20 @@ static GHashTable *coroutine_mon; /* Maps Coroutine* to Monitor* */
> MonitorList mon_list;
> static bool monitor_destroyed;
>
> +OBJECT_DEFINE_TYPE(Monitor, monitor, MONITOR, OBJECT);
> +
> +static void monitor_finalize(Object *obj)
> +{
> +}
> +
> +static void monitor_class_init(ObjectClass *cls, const void *data)
> +{
> +}
> +
> +static void monitor_init(Object *obj)
> +{
> +}
> +
> Monitor *monitor_cur(void)
> {
> Monitor *mon;
> @@ -598,7 +612,7 @@ void monitor_list_append(Monitor *mon)
>
> if (mon) {
> monitor_data_destroy(mon);
> - g_free(mon);
> + object_unref(mon);
> }
> }
>
> @@ -680,7 +694,7 @@ void monitor_cleanup(void)
> monitor_flush(mon);
> monitor_data_destroy(mon);
> qemu_mutex_lock(&monitor_lock);
> - g_free(mon);
> + object_unref(mon);
> }
> qemu_mutex_unlock(&monitor_lock);
>
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index 191eba1b3a..d0ed241d6a 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -166,12 +166,12 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
> int64_t cpu_index, Error **errp)
> {
> char *output = NULL;
> - MonitorHMP hmp = {};
> + MonitorHMP *hmp = MONITOR_HMP(object_new(TYPE_MONITOR_HMP));
>
> - monitor_data_init(&hmp.parent, false, true, false);
> + monitor_data_init(&hmp->parent, false, true, false);
>
> if (has_cpu_index) {
> - int ret = monitor_set_cpu(&hmp.parent, cpu_index);
> + int ret = monitor_set_cpu(&hmp->parent, cpu_index);
> if (ret < 0) {
> error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "cpu-index",
> "a CPU number");
> @@ -179,14 +179,15 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
> }
> }
>
> - handle_hmp_command(&hmp, command_line);
> + handle_hmp_command(hmp, command_line);
>
> - WITH_QEMU_LOCK_GUARD(&hmp.parent.mon_lock) {
> - output = g_strdup(hmp.parent.outbuf->str);
> + WITH_QEMU_LOCK_GUARD(&hmp->parent.mon_lock) {
> + output = g_strdup(hmp->parent.outbuf->str);
> }
>
> out:
> - monitor_data_destroy(&hmp.parent);
> + monitor_data_destroy(&hmp->parent);
> + object_unref(hmp);
> return output;
> }
>
> diff --git a/monitor/qmp.c b/monitor/qmp.c
> index 9caee70624..36cb078f30 100644
> --- a/monitor/qmp.c
> +++ b/monitor/qmp.c
> @@ -71,6 +71,20 @@ typedef struct QMPRequest QMPRequest;
>
> QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
>
> +OBJECT_DEFINE_TYPE(MonitorQMP, monitor_qmp, MONITOR_QMP, MONITOR);
> +
> +static void monitor_qmp_finalize(Object *obj)
> +{
> +}
> +
> +static void monitor_qmp_class_init(ObjectClass *cls, const void *data)
> +{
> +}
> +
> +static void monitor_qmp_init(Object *obj)
> +{
> +}
> +
> static bool qmp_oob_enabled(MonitorQMP *mon)
> {
> return mon->capab[QMP_CAPABILITY_OOB];
> @@ -515,10 +529,10 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
>
> void monitor_new_qmp(Chardev *chr, bool pretty, Error **errp)
> {
> - MonitorQMP *mon = g_new0(MonitorQMP, 1);
> + MonitorQMP *mon = MONITOR_QMP(object_new(TYPE_MONITOR_QMP));
>
> if (!qemu_chr_fe_init(&mon->parent.chr, chr, errp)) {
> - g_free(mon);
> + object_unref(mon);
> return;
> }
> qemu_chr_fe_set_echo(&mon->parent.chr, true);
> --
> 2.53.0
>
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 05/17] monitor: remove 'skip_flush' field
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
` (3 preceding siblings ...)
2026-04-10 16:04 ` [PATCH 04/17] monitor: minimal conversion of monitors to QOM Daniel P. Berrangé
@ 2026-04-10 16:04 ` Daniel P. Berrangé
2026-04-13 1:28 ` Dr. David Alan Gilbert
2026-04-10 16:04 ` [PATCH 06/17] monitor: move monitor_data_(init|destroy) into QOM init/finalize Daniel P. Berrangé
` (12 subsequent siblings)
17 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf,
Daniel P. Berrangé, qemu-block, Dr. David Alan Gilbert,
Eric Blake
The 'skip_flush' field is set on the dummy throwaway HMP monitor
object created by QMP's 'human-monitor-command', as an indication
not to try to write data to the chardev. Instead the QMP command
impl will grab the data straight out of the in-memory buffer.
The flag is redundant, however, as the monitor code could instead
simply check the 'fe_is_open' field on the CharFrontend, which
will be false in the same scenarios that 'skip_flush' is true.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
monitor/hmp.c | 2 +-
monitor/monitor-internal.h | 4 +---
monitor/monitor.c | 11 +++++++----
monitor/qmp-cmds.c | 2 +-
monitor/qmp.c | 2 +-
5 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/monitor/hmp.c b/monitor/hmp.c
index 614d9a3707..72f8303662 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -1539,7 +1539,7 @@ void monitor_new_hmp(Chardev *chr, bool use_readline, Error **errp)
return;
}
- monitor_data_init(&mon->parent, false, false, false);
+ monitor_data_init(&mon->parent, false, false);
mon->use_readline = use_readline;
if (mon->use_readline) {
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 25320928a7..84117805b7 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -102,7 +102,6 @@ struct Monitor {
CharFrontend chr;
int suspend_cnt; /* Needs to be accessed atomically */
bool is_qmp;
- bool skip_flush;
bool use_io_thread;
char *mon_cpu_path;
@@ -183,8 +182,7 @@ extern QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
extern QemuMutex monitor_lock;
extern MonitorList mon_list;
-void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
- bool use_io_thread);
+void monitor_data_init(Monitor *mon, bool is_qmp, bool use_io_thread);
void monitor_data_destroy(Monitor *mon);
int monitor_can_read(void *opaque);
void monitor_list_append(Monitor *mon);
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 7936e2ab22..f7e3708d2f 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -167,7 +167,12 @@ void monitor_flush_locked(Monitor *mon)
size_t len;
const char *buf;
- if (mon->skip_flush) {
+ /*
+ * When used by QMP human-monitor-command, no chardev
+ * will be connected, as we want to just collect the
+ * output in the buffer
+ */
+ if (!mon->chr.fe_is_open) {
return;
}
@@ -621,8 +626,7 @@ static void monitor_iothread_init(void)
mon_iothread = iothread_create("mon_iothread", &error_abort);
}
-void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
- bool use_io_thread)
+void monitor_data_init(Monitor *mon, bool is_qmp, bool use_io_thread)
{
if (use_io_thread && !mon_iothread) {
monitor_iothread_init();
@@ -630,7 +634,6 @@ void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
qemu_mutex_init(&mon->mon_lock);
mon->is_qmp = is_qmp;
mon->outbuf = g_string_new(NULL);
- mon->skip_flush = skip_flush;
mon->use_io_thread = use_io_thread;
}
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index d0ed241d6a..be2bd985b7 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -168,7 +168,7 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
char *output = NULL;
MonitorHMP *hmp = MONITOR_HMP(object_new(TYPE_MONITOR_HMP));
- monitor_data_init(&hmp->parent, false, true, false);
+ monitor_data_init(&hmp->parent, false, false);
if (has_cpu_index) {
int ret = monitor_set_cpu(&hmp->parent, cpu_index);
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 36cb078f30..fe2aec9ce9 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -538,7 +538,7 @@ void monitor_new_qmp(Chardev *chr, bool pretty, Error **errp)
qemu_chr_fe_set_echo(&mon->parent.chr, true);
/* Note: we run QMP monitor in I/O thread when @chr supports that */
- monitor_data_init(&mon->parent, true, false,
+ monitor_data_init(&mon->parent, true,
qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
mon->pretty = pretty;
--
2.53.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 05/17] monitor: remove 'skip_flush' field
2026-04-10 16:04 ` [PATCH 05/17] monitor: remove 'skip_flush' field Daniel P. Berrangé
@ 2026-04-13 1:28 ` Dr. David Alan Gilbert
2026-04-13 10:00 ` Daniel P. Berrangé
0 siblings, 1 reply; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2026-04-13 1:28 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf, qemu-block,
Eric Blake
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> The 'skip_flush' field is set on the dummy throwaway HMP monitor
> object created by QMP's 'human-monitor-command', as an indication
> not to try to write data to the chardev. Instead the QMP command
> impl will grab the data straight out of the in-memory buffer.
>
> The flag is redundant, however, as the monitor code could instead
> simply check the 'fe_is_open' field on the CharFrontend, which
> will be false in the same scenarios that 'skip_flush' is true.
For monitors on sockets where the other end isn't connected, does that
change the behaviour? Does that also have !fe_is_open and can that hit this
case?
Dave
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> monitor/hmp.c | 2 +-
> monitor/monitor-internal.h | 4 +---
> monitor/monitor.c | 11 +++++++----
> monitor/qmp-cmds.c | 2 +-
> monitor/qmp.c | 2 +-
> 5 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index 614d9a3707..72f8303662 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -1539,7 +1539,7 @@ void monitor_new_hmp(Chardev *chr, bool use_readline, Error **errp)
> return;
> }
>
> - monitor_data_init(&mon->parent, false, false, false);
> + monitor_data_init(&mon->parent, false, false);
>
> mon->use_readline = use_readline;
> if (mon->use_readline) {
> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> index 25320928a7..84117805b7 100644
> --- a/monitor/monitor-internal.h
> +++ b/monitor/monitor-internal.h
> @@ -102,7 +102,6 @@ struct Monitor {
> CharFrontend chr;
> int suspend_cnt; /* Needs to be accessed atomically */
> bool is_qmp;
> - bool skip_flush;
> bool use_io_thread;
>
> char *mon_cpu_path;
> @@ -183,8 +182,7 @@ extern QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
> extern QemuMutex monitor_lock;
> extern MonitorList mon_list;
>
> -void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
> - bool use_io_thread);
> +void monitor_data_init(Monitor *mon, bool is_qmp, bool use_io_thread);
> void monitor_data_destroy(Monitor *mon);
> int monitor_can_read(void *opaque);
> void monitor_list_append(Monitor *mon);
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 7936e2ab22..f7e3708d2f 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -167,7 +167,12 @@ void monitor_flush_locked(Monitor *mon)
> size_t len;
> const char *buf;
>
> - if (mon->skip_flush) {
> + /*
> + * When used by QMP human-monitor-command, no chardev
> + * will be connected, as we want to just collect the
> + * output in the buffer
> + */
> + if (!mon->chr.fe_is_open) {
> return;
> }
>
> @@ -621,8 +626,7 @@ static void monitor_iothread_init(void)
> mon_iothread = iothread_create("mon_iothread", &error_abort);
> }
>
> -void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
> - bool use_io_thread)
> +void monitor_data_init(Monitor *mon, bool is_qmp, bool use_io_thread)
> {
> if (use_io_thread && !mon_iothread) {
> monitor_iothread_init();
> @@ -630,7 +634,6 @@ void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
> qemu_mutex_init(&mon->mon_lock);
> mon->is_qmp = is_qmp;
> mon->outbuf = g_string_new(NULL);
> - mon->skip_flush = skip_flush;
> mon->use_io_thread = use_io_thread;
> }
>
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index d0ed241d6a..be2bd985b7 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -168,7 +168,7 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
> char *output = NULL;
> MonitorHMP *hmp = MONITOR_HMP(object_new(TYPE_MONITOR_HMP));
>
> - monitor_data_init(&hmp->parent, false, true, false);
> + monitor_data_init(&hmp->parent, false, false);
>
> if (has_cpu_index) {
> int ret = monitor_set_cpu(&hmp->parent, cpu_index);
> diff --git a/monitor/qmp.c b/monitor/qmp.c
> index 36cb078f30..fe2aec9ce9 100644
> --- a/monitor/qmp.c
> +++ b/monitor/qmp.c
> @@ -538,7 +538,7 @@ void monitor_new_qmp(Chardev *chr, bool pretty, Error **errp)
> qemu_chr_fe_set_echo(&mon->parent.chr, true);
>
> /* Note: we run QMP monitor in I/O thread when @chr supports that */
> - monitor_data_init(&mon->parent, true, false,
> + monitor_data_init(&mon->parent, true,
> qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
>
> mon->pretty = pretty;
> --
> 2.53.0
>
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 05/17] monitor: remove 'skip_flush' field
2026-04-13 1:28 ` Dr. David Alan Gilbert
@ 2026-04-13 10:00 ` Daniel P. Berrangé
2026-04-13 15:22 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-13 10:00 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: qemu-devel, Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf, qemu-block,
Eric Blake
On Mon, Apr 13, 2026 at 01:28:13AM +0000, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > The 'skip_flush' field is set on the dummy throwaway HMP monitor
> > object created by QMP's 'human-monitor-command', as an indication
> > not to try to write data to the chardev. Instead the QMP command
> > impl will grab the data straight out of the in-memory buffer.
> >
> > The flag is redundant, however, as the monitor code could instead
> > simply check the 'fe_is_open' field on the CharFrontend, which
> > will be false in the same scenarios that 'skip_flush' is true.
>
> For monitors on sockets where the other end isn't connected, does that
> change the behaviour? Does that also have !fe_is_open and can that hit this
> case?
The only place that sets fe_is_open is qemu_chr_fe_set_open,
which is called by qemu_chr_fe_set_handlers_full.
IOW, it merely reflects the fact that some frontend (the
monitor in this case) and configured the chardev for use,
and is separate from any notion of whether the chardev
backend is open/connected.
>
> Dave
>
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > monitor/hmp.c | 2 +-
> > monitor/monitor-internal.h | 4 +---
> > monitor/monitor.c | 11 +++++++----
> > monitor/qmp-cmds.c | 2 +-
> > monitor/qmp.c | 2 +-
> > 5 files changed, 11 insertions(+), 10 deletions(-)
> >
> > diff --git a/monitor/hmp.c b/monitor/hmp.c
> > index 614d9a3707..72f8303662 100644
> > --- a/monitor/hmp.c
> > +++ b/monitor/hmp.c
> > @@ -1539,7 +1539,7 @@ void monitor_new_hmp(Chardev *chr, bool use_readline, Error **errp)
> > return;
> > }
> >
> > - monitor_data_init(&mon->parent, false, false, false);
> > + monitor_data_init(&mon->parent, false, false);
> >
> > mon->use_readline = use_readline;
> > if (mon->use_readline) {
> > diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> > index 25320928a7..84117805b7 100644
> > --- a/monitor/monitor-internal.h
> > +++ b/monitor/monitor-internal.h
> > @@ -102,7 +102,6 @@ struct Monitor {
> > CharFrontend chr;
> > int suspend_cnt; /* Needs to be accessed atomically */
> > bool is_qmp;
> > - bool skip_flush;
> > bool use_io_thread;
> >
> > char *mon_cpu_path;
> > @@ -183,8 +182,7 @@ extern QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
> > extern QemuMutex monitor_lock;
> > extern MonitorList mon_list;
> >
> > -void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
> > - bool use_io_thread);
> > +void monitor_data_init(Monitor *mon, bool is_qmp, bool use_io_thread);
> > void monitor_data_destroy(Monitor *mon);
> > int monitor_can_read(void *opaque);
> > void monitor_list_append(Monitor *mon);
> > diff --git a/monitor/monitor.c b/monitor/monitor.c
> > index 7936e2ab22..f7e3708d2f 100644
> > --- a/monitor/monitor.c
> > +++ b/monitor/monitor.c
> > @@ -167,7 +167,12 @@ void monitor_flush_locked(Monitor *mon)
> > size_t len;
> > const char *buf;
> >
> > - if (mon->skip_flush) {
> > + /*
> > + * When used by QMP human-monitor-command, no chardev
> > + * will be connected, as we want to just collect the
> > + * output in the buffer
> > + */
> > + if (!mon->chr.fe_is_open) {
> > return;
> > }
> >
> > @@ -621,8 +626,7 @@ static void monitor_iothread_init(void)
> > mon_iothread = iothread_create("mon_iothread", &error_abort);
> > }
> >
> > -void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
> > - bool use_io_thread)
> > +void monitor_data_init(Monitor *mon, bool is_qmp, bool use_io_thread)
> > {
> > if (use_io_thread && !mon_iothread) {
> > monitor_iothread_init();
> > @@ -630,7 +634,6 @@ void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
> > qemu_mutex_init(&mon->mon_lock);
> > mon->is_qmp = is_qmp;
> > mon->outbuf = g_string_new(NULL);
> > - mon->skip_flush = skip_flush;
> > mon->use_io_thread = use_io_thread;
> > }
> >
> > diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> > index d0ed241d6a..be2bd985b7 100644
> > --- a/monitor/qmp-cmds.c
> > +++ b/monitor/qmp-cmds.c
> > @@ -168,7 +168,7 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
> > char *output = NULL;
> > MonitorHMP *hmp = MONITOR_HMP(object_new(TYPE_MONITOR_HMP));
> >
> > - monitor_data_init(&hmp->parent, false, true, false);
> > + monitor_data_init(&hmp->parent, false, false);
> >
> > if (has_cpu_index) {
> > int ret = monitor_set_cpu(&hmp->parent, cpu_index);
> > diff --git a/monitor/qmp.c b/monitor/qmp.c
> > index 36cb078f30..fe2aec9ce9 100644
> > --- a/monitor/qmp.c
> > +++ b/monitor/qmp.c
> > @@ -538,7 +538,7 @@ void monitor_new_qmp(Chardev *chr, bool pretty, Error **errp)
> > qemu_chr_fe_set_echo(&mon->parent.chr, true);
> >
> > /* Note: we run QMP monitor in I/O thread when @chr supports that */
> > - monitor_data_init(&mon->parent, true, false,
> > + monitor_data_init(&mon->parent, true,
> > qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
> >
> > mon->pretty = pretty;
> > --
> > 2.53.0
> >
> --
> -----Open up your eyes, open up your mind, open up your code -------
> / Dr. David Alan Gilbert | Running GNU/Linux | Happy \
> \ dave @ treblig.org | | In Hex /
> \ _________________________|_____ http://www.treblig.org |_______/
>
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 :|
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 05/17] monitor: remove 'skip_flush' field
2026-04-13 10:00 ` Daniel P. Berrangé
@ 2026-04-13 15:22 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2026-04-13 15:22 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf, qemu-block,
Eric Blake
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Mon, Apr 13, 2026 at 01:28:13AM +0000, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > > The 'skip_flush' field is set on the dummy throwaway HMP monitor
> > > object created by QMP's 'human-monitor-command', as an indication
> > > not to try to write data to the chardev. Instead the QMP command
> > > impl will grab the data straight out of the in-memory buffer.
> > >
> > > The flag is redundant, however, as the monitor code could instead
> > > simply check the 'fe_is_open' field on the CharFrontend, which
> > > will be false in the same scenarios that 'skip_flush' is true.
> >
> > For monitors on sockets where the other end isn't connected, does that
> > change the behaviour? Does that also have !fe_is_open and can that hit this
> > case?
>
> The only place that sets fe_is_open is qemu_chr_fe_set_open,
> which is called by qemu_chr_fe_set_handlers_full.
>
> IOW, it merely reflects the fact that some frontend (the
> monitor in this case) and configured the chardev for use,
> and is separate from any notion of whether the chardev
> backend is open/connected.
Ah ok, in which case,
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
>
>
>
> >
> > Dave
> >
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > > monitor/hmp.c | 2 +-
> > > monitor/monitor-internal.h | 4 +---
> > > monitor/monitor.c | 11 +++++++----
> > > monitor/qmp-cmds.c | 2 +-
> > > monitor/qmp.c | 2 +-
> > > 5 files changed, 11 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/monitor/hmp.c b/monitor/hmp.c
> > > index 614d9a3707..72f8303662 100644
> > > --- a/monitor/hmp.c
> > > +++ b/monitor/hmp.c
> > > @@ -1539,7 +1539,7 @@ void monitor_new_hmp(Chardev *chr, bool use_readline, Error **errp)
> > > return;
> > > }
> > >
> > > - monitor_data_init(&mon->parent, false, false, false);
> > > + monitor_data_init(&mon->parent, false, false);
> > >
> > > mon->use_readline = use_readline;
> > > if (mon->use_readline) {
> > > diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> > > index 25320928a7..84117805b7 100644
> > > --- a/monitor/monitor-internal.h
> > > +++ b/monitor/monitor-internal.h
> > > @@ -102,7 +102,6 @@ struct Monitor {
> > > CharFrontend chr;
> > > int suspend_cnt; /* Needs to be accessed atomically */
> > > bool is_qmp;
> > > - bool skip_flush;
> > > bool use_io_thread;
> > >
> > > char *mon_cpu_path;
> > > @@ -183,8 +182,7 @@ extern QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
> > > extern QemuMutex monitor_lock;
> > > extern MonitorList mon_list;
> > >
> > > -void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
> > > - bool use_io_thread);
> > > +void monitor_data_init(Monitor *mon, bool is_qmp, bool use_io_thread);
> > > void monitor_data_destroy(Monitor *mon);
> > > int monitor_can_read(void *opaque);
> > > void monitor_list_append(Monitor *mon);
> > > diff --git a/monitor/monitor.c b/monitor/monitor.c
> > > index 7936e2ab22..f7e3708d2f 100644
> > > --- a/monitor/monitor.c
> > > +++ b/monitor/monitor.c
> > > @@ -167,7 +167,12 @@ void monitor_flush_locked(Monitor *mon)
> > > size_t len;
> > > const char *buf;
> > >
> > > - if (mon->skip_flush) {
> > > + /*
> > > + * When used by QMP human-monitor-command, no chardev
> > > + * will be connected, as we want to just collect the
> > > + * output in the buffer
> > > + */
> > > + if (!mon->chr.fe_is_open) {
> > > return;
> > > }
> > >
> > > @@ -621,8 +626,7 @@ static void monitor_iothread_init(void)
> > > mon_iothread = iothread_create("mon_iothread", &error_abort);
> > > }
> > >
> > > -void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
> > > - bool use_io_thread)
> > > +void monitor_data_init(Monitor *mon, bool is_qmp, bool use_io_thread)
> > > {
> > > if (use_io_thread && !mon_iothread) {
> > > monitor_iothread_init();
> > > @@ -630,7 +634,6 @@ void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
> > > qemu_mutex_init(&mon->mon_lock);
> > > mon->is_qmp = is_qmp;
> > > mon->outbuf = g_string_new(NULL);
> > > - mon->skip_flush = skip_flush;
> > > mon->use_io_thread = use_io_thread;
> > > }
> > >
> > > diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> > > index d0ed241d6a..be2bd985b7 100644
> > > --- a/monitor/qmp-cmds.c
> > > +++ b/monitor/qmp-cmds.c
> > > @@ -168,7 +168,7 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
> > > char *output = NULL;
> > > MonitorHMP *hmp = MONITOR_HMP(object_new(TYPE_MONITOR_HMP));
> > >
> > > - monitor_data_init(&hmp->parent, false, true, false);
> > > + monitor_data_init(&hmp->parent, false, false);
> > >
> > > if (has_cpu_index) {
> > > int ret = monitor_set_cpu(&hmp->parent, cpu_index);
> > > diff --git a/monitor/qmp.c b/monitor/qmp.c
> > > index 36cb078f30..fe2aec9ce9 100644
> > > --- a/monitor/qmp.c
> > > +++ b/monitor/qmp.c
> > > @@ -538,7 +538,7 @@ void monitor_new_qmp(Chardev *chr, bool pretty, Error **errp)
> > > qemu_chr_fe_set_echo(&mon->parent.chr, true);
> > >
> > > /* Note: we run QMP monitor in I/O thread when @chr supports that */
> > > - monitor_data_init(&mon->parent, true, false,
> > > + monitor_data_init(&mon->parent, true,
> > > qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
> > >
> > > mon->pretty = pretty;
> > > --
> > > 2.53.0
> > >
> > --
> > -----Open up your eyes, open up your mind, open up your code -------
> > / Dr. David Alan Gilbert | Running GNU/Linux | Happy \
> > \ dave @ treblig.org | | In Hex /
> > \ _________________________|_____ http://www.treblig.org |_______/
> >
>
> 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 :|
>
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 06/17] monitor: move monitor_data_(init|destroy) into QOM init/finalize
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
` (4 preceding siblings ...)
2026-04-10 16:04 ` [PATCH 05/17] monitor: remove 'skip_flush' field Daniel P. Berrangé
@ 2026-04-10 16:04 ` Daniel P. Berrangé
2026-04-10 16:04 ` [PATCH 07/17] monitor: use class methods for monitor_vprintf Daniel P. Berrangé
` (11 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf,
Daniel P. Berrangé, qemu-block, Dr. David Alan Gilbert,
Eric Blake
Start to take advantage of QOM, by using object init and finalize
methods to replace monitor_data_init and monitor_data_destroy.
A standalone helper is provided to enable the I/O thread for QMP
where appropriate for the chardev backend.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
monitor/hmp.c | 4 ++--
monitor/monitor-internal.h | 3 +--
monitor/monitor.c | 48 ++++++++++++++------------------------
monitor/qmp-cmds.c | 3 ---
monitor/qmp.c | 21 +++++++++--------
5 files changed, 32 insertions(+), 47 deletions(-)
diff --git a/monitor/hmp.c b/monitor/hmp.c
index 72f8303662..833de0eee8 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -46,6 +46,8 @@ OBJECT_DEFINE_TYPE(MonitorHMP, monitor_hmp, MONITOR_HMP, MONITOR);
static void monitor_hmp_finalize(Object *obj)
{
+ MonitorHMP *mon = MONITOR_HMP(obj);
+ readline_free(mon->rs);
}
static void monitor_hmp_class_init(ObjectClass *cls, const void *data)
@@ -1539,8 +1541,6 @@ void monitor_new_hmp(Chardev *chr, bool use_readline, Error **errp)
return;
}
- monitor_data_init(&mon->parent, false, false);
-
mon->use_readline = use_readline;
if (mon->use_readline) {
mon->rs = readline_init(monitor_readline_printf,
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 84117805b7..5bfe3b7325 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -182,8 +182,7 @@ extern QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
extern QemuMutex monitor_lock;
extern MonitorList mon_list;
-void monitor_data_init(Monitor *mon, bool is_qmp, bool use_io_thread);
-void monitor_data_destroy(Monitor *mon);
+void monitor_iothread_init(Monitor *mon);
int monitor_can_read(void *opaque);
void monitor_list_append(Monitor *mon);
void monitor_fdsets_cleanup(void);
diff --git a/monitor/monitor.c b/monitor/monitor.c
index f7e3708d2f..f6c90145f6 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -77,6 +77,12 @@ OBJECT_DEFINE_TYPE(Monitor, monitor, MONITOR, OBJECT);
static void monitor_finalize(Object *obj)
{
+ Monitor *mon = MONITOR(obj);
+
+ g_free(mon->mon_cpu_path);
+ qemu_chr_fe_deinit(&mon->chr, false);
+ g_string_free(mon->outbuf, true);
+ qemu_mutex_destroy(&mon->mon_lock);
}
static void monitor_class_init(ObjectClass *cls, const void *data)
@@ -85,6 +91,11 @@ static void monitor_class_init(ObjectClass *cls, const void *data)
static void monitor_init(Object *obj)
{
+ Monitor *mon = MONITOR(obj);
+
+ qemu_mutex_init(&mon->mon_lock);
+ mon->is_qmp = !!object_dynamic_cast(obj, TYPE_MONITOR_QMP);
+ mon->outbuf = g_string_new(NULL);
}
Monitor *monitor_cur(void)
@@ -616,38 +627,16 @@ void monitor_list_append(Monitor *mon)
qemu_mutex_unlock(&monitor_lock);
if (mon) {
- monitor_data_destroy(mon);
object_unref(mon);
}
}
-static void monitor_iothread_init(void)
-{
- mon_iothread = iothread_create("mon_iothread", &error_abort);
-}
-
-void monitor_data_init(Monitor *mon, bool is_qmp, bool use_io_thread)
+void monitor_iothread_init(Monitor *mon)
{
- if (use_io_thread && !mon_iothread) {
- monitor_iothread_init();
+ if (!mon_iothread) {
+ mon_iothread = iothread_create("mon_iothread", &error_abort);
}
- qemu_mutex_init(&mon->mon_lock);
- mon->is_qmp = is_qmp;
- mon->outbuf = g_string_new(NULL);
- mon->use_io_thread = use_io_thread;
-}
-
-void monitor_data_destroy(Monitor *mon)
-{
- g_free(mon->mon_cpu_path);
- qemu_chr_fe_deinit(&mon->chr, false);
- if (monitor_is_qmp(mon)) {
- monitor_data_destroy_qmp(container_of(mon, MonitorQMP, parent));
- } else {
- readline_free(container_of(mon, MonitorHMP, parent)->rs);
- }
- g_string_free(mon->outbuf, true);
- qemu_mutex_destroy(&mon->mon_lock);
+ mon->use_io_thread = true;
}
void monitor_cleanup(void)
@@ -665,7 +654,7 @@ void monitor_cleanup(void)
* Letting the iothread continue while shutting down the dispatcher
* means that new requests may still be coming in. This is okay,
* we'll just leave them in the queue without sending a response
- * and monitor_data_destroy() will free them.
+ * and object finalization will free them.
*/
WITH_QEMU_LOCK_GUARD(&monitor_lock) {
qmp_dispatcher_co_shutdown = true;
@@ -679,8 +668,8 @@ void monitor_cleanup(void)
/*
* We need to explicitly stop the I/O thread (but not destroy it),
* clean up the monitor resources, then destroy the I/O thread since
- * we need to unregister from chardev below in
- * monitor_data_destroy(), and chardev is not thread-safe yet
+ * we need to unregister from chardev below in object
+ * finalization, and chardev is not thread-safe yet
*/
if (mon_iothread) {
iothread_stop(mon_iothread);
@@ -695,7 +684,6 @@ void monitor_cleanup(void)
/* Permit QAPI event emission from character frontend release */
qemu_mutex_unlock(&monitor_lock);
monitor_flush(mon);
- monitor_data_destroy(mon);
qemu_mutex_lock(&monitor_lock);
object_unref(mon);
}
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index be2bd985b7..5be93eff4d 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -168,8 +168,6 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
char *output = NULL;
MonitorHMP *hmp = MONITOR_HMP(object_new(TYPE_MONITOR_HMP));
- monitor_data_init(&hmp->parent, false, false);
-
if (has_cpu_index) {
int ret = monitor_set_cpu(&hmp->parent, cpu_index);
if (ret < 0) {
@@ -186,7 +184,6 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
}
out:
- monitor_data_destroy(&hmp->parent);
object_unref(hmp);
return output;
}
diff --git a/monitor/qmp.c b/monitor/qmp.c
index fe2aec9ce9..0de98b33fe 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -73,8 +73,16 @@ QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
OBJECT_DEFINE_TYPE(MonitorQMP, monitor_qmp, MONITOR_QMP, MONITOR);
+static void monitor_qmp_cleanup_req_queue_locked(MonitorQMP *mon);
+
static void monitor_qmp_finalize(Object *obj)
{
+ MonitorQMP *mon = MONITOR_QMP(obj);
+
+ json_message_parser_destroy(&mon->parser);
+ qemu_mutex_destroy(&mon->qmp_queue_lock);
+ monitor_qmp_cleanup_req_queue_locked(mon);
+ g_queue_free(mon->qmp_requests);
}
static void monitor_qmp_class_init(ObjectClass *cls, const void *data)
@@ -505,14 +513,6 @@ static void monitor_qmp_event(void *opaque, QEMUChrEvent event)
}
}
-void monitor_data_destroy_qmp(MonitorQMP *mon)
-{
- json_message_parser_destroy(&mon->parser);
- qemu_mutex_destroy(&mon->qmp_queue_lock);
- monitor_qmp_cleanup_req_queue_locked(mon);
- g_queue_free(mon->qmp_requests);
-}
-
static void monitor_qmp_setup_handlers_bh(void *opaque)
{
MonitorQMP *mon = opaque;
@@ -538,8 +538,9 @@ void monitor_new_qmp(Chardev *chr, bool pretty, Error **errp)
qemu_chr_fe_set_echo(&mon->parent.chr, true);
/* Note: we run QMP monitor in I/O thread when @chr supports that */
- monitor_data_init(&mon->parent, true,
- qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
+ if (qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT)) {
+ monitor_iothread_init(&mon->parent);
+ }
mon->pretty = pretty;
--
2.53.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 07/17] monitor: use class methods for monitor_vprintf
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
` (5 preceding siblings ...)
2026-04-10 16:04 ` [PATCH 06/17] monitor: move monitor_data_(init|destroy) into QOM init/finalize Daniel P. Berrangé
@ 2026-04-10 16:04 ` Daniel P. Berrangé
2026-04-13 1:32 ` Dr. David Alan Gilbert
2026-04-10 16:04 ` [PATCH 08/17] monitor: use class methods for monitor_qapi_event_emit Daniel P. Berrangé
` (10 subsequent siblings)
17 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf,
Daniel P. Berrangé, qemu-block, Dr. David Alan Gilbert,
Eric Blake
This removes the need for using monitor_is_qmp() to check the
subclass type, which is an anti-pattern.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
monitor/hmp.c | 12 ++++++++++++
monitor/monitor-internal.h | 3 +++
monitor/monitor.c | 11 ++++-------
3 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/monitor/hmp.c b/monitor/hmp.c
index 833de0eee8..3fe6f5715a 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -50,14 +50,26 @@ static void monitor_hmp_finalize(Object *obj)
readline_free(mon->rs);
}
+int monitor_hmp_vprintf(Monitor *mon, const char *fmt, va_list ap)
+ G_GNUC_PRINTF(2, 0);
+
static void monitor_hmp_class_init(ObjectClass *cls, const void *data)
{
+ MonitorClass *moncls = MONITOR_CLASS(cls);
+
+ moncls->vprintf = monitor_hmp_vprintf;
}
static void monitor_hmp_init(Object *obj)
{
}
+int monitor_hmp_vprintf(Monitor *mon, const char *fmt, va_list ap)
+{
+ g_autofree char *buf = g_strdup_vprintf(fmt, ap);
+ return monitor_puts(mon, buf);
+}
+
static void monitor_command_cb(void *opaque, const char *cmdline,
void *readline_opaque)
{
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 5bfe3b7325..3f05c46e88 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -95,6 +95,9 @@ typedef struct HMPCommand {
struct MonitorClass {
ObjectClass parent_class;
+
+ int (*vprintf)(Monitor *mon, const char *fmt, va_list ap)
+ G_GNUC_PRINTF(2, 0);
};
struct Monitor {
diff --git a/monitor/monitor.c b/monitor/monitor.c
index f6c90145f6..b51046c0c9 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -243,21 +243,18 @@ int monitor_puts(Monitor *mon, const char *str)
int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
{
- char *buf;
- int n;
+ MonitorClass *moncls;
if (!mon) {
return -1;
}
- if (monitor_is_qmp(mon)) {
+ moncls = MONITOR_GET_CLASS(mon);
+ if (!moncls->vprintf) {
return -1;
}
- buf = g_strdup_vprintf(fmt, ap);
- n = monitor_puts(mon, buf);
- g_free(buf);
- return n;
+ return moncls->vprintf(mon, fmt, ap);
}
int monitor_printf(Monitor *mon, const char *fmt, ...)
--
2.53.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 07/17] monitor: use class methods for monitor_vprintf
2026-04-10 16:04 ` [PATCH 07/17] monitor: use class methods for monitor_vprintf Daniel P. Berrangé
@ 2026-04-13 1:32 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2026-04-13 1:32 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf, qemu-block,
Eric Blake
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> This removes the need for using monitor_is_qmp() to check the
> subclass type, which is an anti-pattern.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
> ---
> monitor/hmp.c | 12 ++++++++++++
> monitor/monitor-internal.h | 3 +++
> monitor/monitor.c | 11 ++++-------
> 3 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index 833de0eee8..3fe6f5715a 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -50,14 +50,26 @@ static void monitor_hmp_finalize(Object *obj)
> readline_free(mon->rs);
> }
>
> +int monitor_hmp_vprintf(Monitor *mon, const char *fmt, va_list ap)
> + G_GNUC_PRINTF(2, 0);
> +
> static void monitor_hmp_class_init(ObjectClass *cls, const void *data)
> {
> + MonitorClass *moncls = MONITOR_CLASS(cls);
> +
> + moncls->vprintf = monitor_hmp_vprintf;
> }
>
> static void monitor_hmp_init(Object *obj)
> {
> }
>
> +int monitor_hmp_vprintf(Monitor *mon, const char *fmt, va_list ap)
> +{
> + g_autofree char *buf = g_strdup_vprintf(fmt, ap);
> + return monitor_puts(mon, buf);
> +}
> +
> static void monitor_command_cb(void *opaque, const char *cmdline,
> void *readline_opaque)
> {
> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> index 5bfe3b7325..3f05c46e88 100644
> --- a/monitor/monitor-internal.h
> +++ b/monitor/monitor-internal.h
> @@ -95,6 +95,9 @@ typedef struct HMPCommand {
>
> struct MonitorClass {
> ObjectClass parent_class;
> +
> + int (*vprintf)(Monitor *mon, const char *fmt, va_list ap)
> + G_GNUC_PRINTF(2, 0);
> };
>
> struct Monitor {
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index f6c90145f6..b51046c0c9 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -243,21 +243,18 @@ int monitor_puts(Monitor *mon, const char *str)
>
> int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
> {
> - char *buf;
> - int n;
> + MonitorClass *moncls;
>
> if (!mon) {
> return -1;
> }
>
> - if (monitor_is_qmp(mon)) {
> + moncls = MONITOR_GET_CLASS(mon);
> + if (!moncls->vprintf) {
> return -1;
> }
>
> - buf = g_strdup_vprintf(fmt, ap);
> - n = monitor_puts(mon, buf);
> - g_free(buf);
> - return n;
> + return moncls->vprintf(mon, fmt, ap);
> }
>
> int monitor_printf(Monitor *mon, const char *fmt, ...)
> --
> 2.53.0
>
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 08/17] monitor: use class methods for monitor_qapi_event_emit
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
` (6 preceding siblings ...)
2026-04-10 16:04 ` [PATCH 07/17] monitor: use class methods for monitor_vprintf Daniel P. Berrangé
@ 2026-04-10 16:04 ` Daniel P. Berrangé
2026-04-10 16:04 ` [PATCH 09/17] monitor: use class methods for monitor_accept_input Daniel P. Berrangé
` (9 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf,
Daniel P. Berrangé, qemu-block, Dr. David Alan Gilbert,
Eric Blake
This removes the need for using monitor_is_qmp() to check the
subclass type, which is an anti-pattern.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
include/monitor/monitor.h | 1 +
monitor/monitor-internal.h | 1 +
monitor/monitor.c | 15 +++------------
monitor/qmp.c | 19 +++++++++++++++++++
4 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index b349b12c34..7b0bda42c4 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -3,6 +3,7 @@
#include "block/block.h"
#include "qapi/qapi-types-misc.h"
+#include "qapi/qapi-emit-events.h"
#include "qemu/readline.h"
#include "exec/hwaddr.h"
#include "qom/object.h"
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 3f05c46e88..4b5e708f14 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -98,6 +98,7 @@ struct MonitorClass {
int (*vprintf)(Monitor *mon, const char *fmt, va_list ap)
G_GNUC_PRINTF(2, 0);
+ void (*emit_event)(Monitor *mon, QAPIEvent event, QDict *qdict);
};
struct Monitor {
diff --git a/monitor/monitor.c b/monitor/monitor.c
index b51046c0c9..5f55d33476 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -327,22 +327,13 @@ static inline QEMUClockType monitor_get_event_clock(void)
static void monitor_qapi_event_emit(QAPIEvent event, QDict *qdict)
{
Monitor *mon;
- MonitorQMP *qmp_mon;
trace_monitor_protocol_event_emit(event, qdict);
QTAILQ_FOREACH(mon, &mon_list, entry) {
- if (!monitor_is_qmp(mon)) {
- continue;
+ MonitorClass *cls = MONITOR_GET_CLASS(mon);
+ if (cls->emit_event) {
+ cls->emit_event(mon, event, qdict);
}
-
- qmp_mon = container_of(mon, MonitorQMP, parent);
- {
- QEMU_LOCK_GUARD(&mon->mon_lock);
- if (qmp_mon->commands == &qmp_cap_negotiation_commands) {
- continue;
- }
- }
- qmp_send_response(qmp_mon, qdict);
}
}
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 0de98b33fe..9c3e8e2678 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -85,14 +85,33 @@ static void monitor_qmp_finalize(Object *obj)
g_queue_free(mon->qmp_requests);
}
+static void monitor_qmp_emit_event(Monitor *mon, QAPIEvent event, QDict *qdict);
+
static void monitor_qmp_class_init(ObjectClass *cls, const void *data)
{
+ MonitorClass *moncls = MONITOR_CLASS(cls);
+
+ moncls->emit_event = monitor_qmp_emit_event;
}
static void monitor_qmp_init(Object *obj)
{
}
+static void monitor_qmp_emit_event(Monitor *mon, QAPIEvent event, QDict *qdict)
+{
+ MonitorQMP *qmp = MONITOR_QMP(mon);
+
+ WITH_QEMU_LOCK_GUARD(&mon->mon_lock) {
+ if (qmp->commands == &qmp_cap_negotiation_commands) {
+ return;
+ }
+ }
+
+ qmp_send_response(qmp, qdict);
+}
+
+
static bool qmp_oob_enabled(MonitorQMP *mon)
{
return mon->capab[QMP_CAPABILITY_OOB];
--
2.53.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 09/17] monitor: use class methods for monitor_accept_input
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
` (7 preceding siblings ...)
2026-04-10 16:04 ` [PATCH 08/17] monitor: use class methods for monitor_qapi_event_emit Daniel P. Berrangé
@ 2026-04-10 16:04 ` Daniel P. Berrangé
2026-04-13 17:43 ` Dr. David Alan Gilbert
2026-04-10 16:04 ` [PATCH 10/17] monitor: use dynamic cast in monitor_qmp_requests_pop_any_with_lock Daniel P. Berrangé
` (8 subsequent siblings)
17 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf,
Daniel P. Berrangé, qemu-block, Dr. David Alan Gilbert,
Eric Blake
This removes the need for using monitor_is_qmp() to check the
subclass type, which is an anti-pattern.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
monitor/hmp.c | 16 ++++++++++++++++
monitor/monitor-internal.h | 1 +
monitor/monitor.c | 12 +++---------
3 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/monitor/hmp.c b/monitor/hmp.c
index 3fe6f5715a..5f9373a87b 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -52,12 +52,14 @@ static void monitor_hmp_finalize(Object *obj)
int monitor_hmp_vprintf(Monitor *mon, const char *fmt, va_list ap)
G_GNUC_PRINTF(2, 0);
+static void monitor_hmp_accept_input(Monitor *mon);
static void monitor_hmp_class_init(ObjectClass *cls, const void *data)
{
MonitorClass *moncls = MONITOR_CLASS(cls);
moncls->vprintf = monitor_hmp_vprintf;
+ moncls->accept_input = monitor_hmp_accept_input;
}
static void monitor_hmp_init(Object *obj)
@@ -70,6 +72,20 @@ int monitor_hmp_vprintf(Monitor *mon, const char *fmt, va_list ap)
return monitor_puts(mon, buf);
}
+static void monitor_hmp_accept_input(Monitor *mon)
+{
+ qemu_mutex_lock(&mon->mon_lock);
+ if (mon->reset_seen) {
+ MonitorHMP *hmp = MONITOR_HMP(mon);
+ assert(hmp->rs);
+ readline_restart(hmp->rs);
+ qemu_mutex_unlock(&mon->mon_lock);
+ readline_show_prompt(hmp->rs);
+ } else {
+ qemu_mutex_unlock(&mon->mon_lock);
+ }
+}
+
static void monitor_command_cb(void *opaque, const char *cmdline,
void *readline_opaque)
{
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 4b5e708f14..7004f5d002 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -99,6 +99,7 @@ struct MonitorClass {
int (*vprintf)(Monitor *mon, const char *fmt, va_list ap)
G_GNUC_PRINTF(2, 0);
void (*emit_event)(Monitor *mon, QAPIEvent event, QDict *qdict);
+ void (*accept_input)(Monitor *mon);
};
struct Monitor {
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 5f55d33476..1326069b79 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -557,16 +557,10 @@ int monitor_suspend(Monitor *mon)
static void monitor_accept_input(void *opaque)
{
Monitor *mon = opaque;
+ MonitorClass *cls = MONITOR_GET_CLASS(mon);
- qemu_mutex_lock(&mon->mon_lock);
- if (!monitor_is_qmp(mon) && mon->reset_seen) {
- MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, parent);
- assert(hmp_mon->rs);
- readline_restart(hmp_mon->rs);
- qemu_mutex_unlock(&mon->mon_lock);
- readline_show_prompt(hmp_mon->rs);
- } else {
- qemu_mutex_unlock(&mon->mon_lock);
+ if (cls->accept_input) {
+ cls->accept_input(mon);
}
qemu_chr_fe_accept_input(&mon->chr);
--
2.53.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 09/17] monitor: use class methods for monitor_accept_input
2026-04-10 16:04 ` [PATCH 09/17] monitor: use class methods for monitor_accept_input Daniel P. Berrangé
@ 2026-04-13 17:43 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2026-04-13 17:43 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf, qemu-block,
Eric Blake
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> This removes the need for using monitor_is_qmp() to check the
> subclass type, which is an anti-pattern.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
> ---
> monitor/hmp.c | 16 ++++++++++++++++
> monitor/monitor-internal.h | 1 +
> monitor/monitor.c | 12 +++---------
> 3 files changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index 3fe6f5715a..5f9373a87b 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -52,12 +52,14 @@ static void monitor_hmp_finalize(Object *obj)
>
> int monitor_hmp_vprintf(Monitor *mon, const char *fmt, va_list ap)
> G_GNUC_PRINTF(2, 0);
> +static void monitor_hmp_accept_input(Monitor *mon);
>
> static void monitor_hmp_class_init(ObjectClass *cls, const void *data)
> {
> MonitorClass *moncls = MONITOR_CLASS(cls);
>
> moncls->vprintf = monitor_hmp_vprintf;
> + moncls->accept_input = monitor_hmp_accept_input;
> }
>
> static void monitor_hmp_init(Object *obj)
> @@ -70,6 +72,20 @@ int monitor_hmp_vprintf(Monitor *mon, const char *fmt, va_list ap)
> return monitor_puts(mon, buf);
> }
>
> +static void monitor_hmp_accept_input(Monitor *mon)
> +{
> + qemu_mutex_lock(&mon->mon_lock);
> + if (mon->reset_seen) {
> + MonitorHMP *hmp = MONITOR_HMP(mon);
> + assert(hmp->rs);
> + readline_restart(hmp->rs);
> + qemu_mutex_unlock(&mon->mon_lock);
> + readline_show_prompt(hmp->rs);
> + } else {
> + qemu_mutex_unlock(&mon->mon_lock);
> + }
> +}
> +
> static void monitor_command_cb(void *opaque, const char *cmdline,
> void *readline_opaque)
> {
> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> index 4b5e708f14..7004f5d002 100644
> --- a/monitor/monitor-internal.h
> +++ b/monitor/monitor-internal.h
> @@ -99,6 +99,7 @@ struct MonitorClass {
> int (*vprintf)(Monitor *mon, const char *fmt, va_list ap)
> G_GNUC_PRINTF(2, 0);
> void (*emit_event)(Monitor *mon, QAPIEvent event, QDict *qdict);
> + void (*accept_input)(Monitor *mon);
> };
>
> struct Monitor {
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 5f55d33476..1326069b79 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -557,16 +557,10 @@ int monitor_suspend(Monitor *mon)
> static void monitor_accept_input(void *opaque)
> {
> Monitor *mon = opaque;
> + MonitorClass *cls = MONITOR_GET_CLASS(mon);
>
> - qemu_mutex_lock(&mon->mon_lock);
> - if (!monitor_is_qmp(mon) && mon->reset_seen) {
> - MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, parent);
> - assert(hmp_mon->rs);
> - readline_restart(hmp_mon->rs);
> - qemu_mutex_unlock(&mon->mon_lock);
> - readline_show_prompt(hmp_mon->rs);
> - } else {
> - qemu_mutex_unlock(&mon->mon_lock);
> + if (cls->accept_input) {
> + cls->accept_input(mon);
> }
>
> qemu_chr_fe_accept_input(&mon->chr);
> --
> 2.53.0
>
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 10/17] monitor: use dynamic cast in monitor_qmp_requests_pop_any_with_lock
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
` (8 preceding siblings ...)
2026-04-10 16:04 ` [PATCH 09/17] monitor: use class methods for monitor_accept_input Daniel P. Berrangé
@ 2026-04-10 16:04 ` Daniel P. Berrangé
2026-04-10 16:04 ` [PATCH 11/17] util: use dynamic cast in error vreport Daniel P. Berrangé
` (7 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf,
Daniel P. Berrangé, qemu-block, Dr. David Alan Gilbert,
Eric Blake
This eliminates a use of monitor_is_qmp() from the QMP coroutine
dispatch path.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
monitor/qmp.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 9c3e8e2678..2ec9cca3a6 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -244,11 +244,12 @@ static QMPRequest *monitor_qmp_requests_pop_any_with_lock(void)
MonitorQMP *qmp_mon;
QTAILQ_FOREACH(mon, &mon_list, entry) {
- if (!monitor_is_qmp(mon)) {
+ qmp_mon = MONITOR_QMP(
+ object_dynamic_cast(OBJECT(mon), TYPE_MONITOR_QMP));
+ if (!qmp_mon) {
continue;
}
- qmp_mon = container_of(mon, MonitorQMP, parent);
qemu_mutex_lock(&qmp_mon->qmp_queue_lock);
req_obj = g_queue_pop_head(qmp_mon->qmp_requests);
if (req_obj) {
--
2.53.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 11/17] util: use dynamic cast in error vreport
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
` (9 preceding siblings ...)
2026-04-10 16:04 ` [PATCH 10/17] monitor: use dynamic cast in monitor_qmp_requests_pop_any_with_lock Daniel P. Berrangé
@ 2026-04-10 16:04 ` Daniel P. Berrangé
2026-04-10 16:04 ` [PATCH 12/17] monitor: drop unused monitor_cur_is_qmp Daniel P. Berrangé
` (6 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf,
Daniel P. Berrangé, qemu-block, Dr. David Alan Gilbert,
Eric Blake
This eliminates a use of monitor_is_qmp() from the error reporting
path.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
util/error-report.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/util/error-report.c b/util/error-report.c
index bbea49a55d..b0e062da9f 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -223,16 +223,15 @@ char *real_time_iso8601(void)
G_GNUC_PRINTF(2, 0)
static void vreport(report_type type, const char *fmt, va_list ap)
{
- Monitor *cur = monitor_cur();
- gchar *timestr;
-
/*
* When current monitor is QMP, messages must go to stderr
- * and have prefixes added
+ * and have prefixes added, so we cast to HMP, leaving 'cur'
+ * as NULL in QMP case
*/
- if (monitor_cur_is_qmp()) {
- cur = NULL;
- }
+ Monitor *cur = MONITOR(
+ object_dynamic_cast(OBJECT(monitor_cur()), TYPE_MONITOR_HMP));
+ gchar *timestr;
+
if (!cur) {
qemu_flockfile(stderr);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 12/17] monitor: drop unused monitor_cur_is_qmp
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
` (10 preceding siblings ...)
2026-04-10 16:04 ` [PATCH 11/17] util: use dynamic cast in error vreport Daniel P. Berrangé
@ 2026-04-10 16:04 ` Daniel P. Berrangé
2026-04-10 16:04 ` [PATCH 13/17] monitor: use dynamic cast in QMP commands Daniel P. Berrangé
` (5 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf,
Daniel P. Berrangé, qemu-block, Dr. David Alan Gilbert,
Eric Blake
The previous patch dropped the only remaining use of
monitor_cur_is_qmp.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
include/monitor/monitor.h | 1 -
monitor/monitor.c | 10 ----------
stubs/monitor-core.c | 5 -----
tests/unit/test-util-sockets.c | 1 -
4 files changed, 17 deletions(-)
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 7b0bda42c4..f66e465775 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -25,7 +25,6 @@ extern QemuOptsList qemu_mon_opts;
Monitor *monitor_cur(void);
Monitor *monitor_set_cur(Coroutine *co, Monitor *mon);
-bool monitor_cur_is_qmp(void);
void monitor_init_globals(void);
void monitor_init_globals_core(void);
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 1326069b79..a3e69b3cec 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -131,16 +131,6 @@ Monitor *monitor_set_cur(Coroutine *co, Monitor *mon)
return old_monitor;
}
-/**
- * Is the current monitor, if any, a QMP monitor?
- */
-bool monitor_cur_is_qmp(void)
-{
- Monitor *cur_mon = monitor_cur();
-
- return cur_mon && monitor_is_qmp(cur_mon);
-}
-
/**
* Is @mon is using readline?
* Note: not all HMP monitors use readline, e.g., gdbserver has a
diff --git a/stubs/monitor-core.c b/stubs/monitor-core.c
index 078a5012e9..a7c32297c9 100644
--- a/stubs/monitor-core.c
+++ b/stubs/monitor-core.c
@@ -7,11 +7,6 @@ Monitor *monitor_cur(void)
return NULL;
}
-bool monitor_cur_is_qmp(void)
-{
- return false;
-}
-
Monitor *monitor_set_cur(Coroutine *co, Monitor *mon)
{
return NULL;
diff --git a/tests/unit/test-util-sockets.c b/tests/unit/test-util-sockets.c
index b9f2453e29..ee66d727c3 100644
--- a/tests/unit/test-util-sockets.c
+++ b/tests/unit/test-util-sockets.c
@@ -74,7 +74,6 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
Monitor *monitor_cur(void) { return cur_mon; }
Monitor *monitor_set_cur(Coroutine *co, Monitor *mon) { abort(); }
int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) { abort(); }
-bool monitor_cur_is_qmp(void) { abort(); };
#ifndef _WIN32
static void test_socket_fd_pass_name_good(void)
--
2.53.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 13/17] monitor: use dynamic cast in QMP commands
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
` (11 preceding siblings ...)
2026-04-10 16:04 ` [PATCH 12/17] monitor: drop unused monitor_cur_is_qmp Daniel P. Berrangé
@ 2026-04-10 16:04 ` Daniel P. Berrangé
2026-04-10 16:04 ` [PATCH 14/17] monitor: use dynamic cast in monitor_is_hmp_non_interactive Daniel P. Berrangé
` (4 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf,
Daniel P. Berrangé, qemu-block, Dr. David Alan Gilbert,
Eric Blake
Rather than asserting monitor_is_qmp(), use a QOM cast via
MONITOR_QMP which performs an assert already.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
monitor/qmp-cmds-control.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/monitor/qmp-cmds-control.c b/monitor/qmp-cmds-control.c
index ca471bc1c7..8fe0876a3d 100644
--- a/monitor/qmp-cmds-control.c
+++ b/monitor/qmp-cmds-control.c
@@ -72,11 +72,7 @@ static bool qmp_caps_accept(MonitorQMP *mon, QMPCapabilityList *list,
void qmp_qmp_capabilities(bool has_enable, QMPCapabilityList *enable,
Error **errp)
{
- Monitor *cur_mon = monitor_cur();
- MonitorQMP *mon;
-
- assert(monitor_is_qmp(cur_mon));
- mon = container_of(cur_mon, MonitorQMP, parent);
+ MonitorQMP *mon = MONITOR_QMP(monitor_cur());
if (mon->commands == &qmp_commands) {
error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
@@ -122,11 +118,7 @@ static void query_commands_cb(const QmpCommand *cmd, void *opaque)
CommandInfoList *qmp_query_commands(Error **errp)
{
CommandInfoList *list = NULL;
- Monitor *cur_mon = monitor_cur();
- MonitorQMP *mon;
-
- assert(monitor_is_qmp(cur_mon));
- mon = container_of(cur_mon, MonitorQMP, parent);
+ MonitorQMP *mon = MONITOR_QMP(monitor_cur());
qmp_for_each_command(mon->commands, query_commands_cb, &list);
--
2.53.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 14/17] monitor: use dynamic cast in monitor_is_hmp_non_interactive
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
` (12 preceding siblings ...)
2026-04-10 16:04 ` [PATCH 13/17] monitor: use dynamic cast in QMP commands Daniel P. Berrangé
@ 2026-04-10 16:04 ` Daniel P. Berrangé
2026-04-10 16:04 ` [PATCH 15/17] monitor: drop unused monitor_is_qmp method Daniel P. Berrangé
` (3 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf,
Daniel P. Berrangé, qemu-block, Dr. David Alan Gilbert,
Eric Blake
Rather than checking !monitor_is_qmp(), use a dynamic cast to
check for HMP.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
monitor/monitor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/monitor/monitor.c b/monitor/monitor.c
index a3e69b3cec..c08b2c2ee1 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -143,7 +143,7 @@ static inline bool monitor_uses_readline(const MonitorHMP *mon)
static inline bool monitor_is_hmp_non_interactive(const Monitor *mon)
{
- if (monitor_is_qmp(mon)) {
+ if (!object_dynamic_cast(OBJECT(mon), TYPE_MONITOR_HMP)) {
return false;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 15/17] monitor: drop unused monitor_is_qmp method
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
` (13 preceding siblings ...)
2026-04-10 16:04 ` [PATCH 14/17] monitor: use dynamic cast in monitor_is_hmp_non_interactive Daniel P. Berrangé
@ 2026-04-10 16:04 ` Daniel P. Berrangé
2026-04-10 16:04 ` [PATCH 16/17] monitor: eliminate monitor_is_hmp_non_interactive method Daniel P. Berrangé
` (2 subsequent siblings)
17 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf,
Daniel P. Berrangé, qemu-block, Dr. David Alan Gilbert,
Eric Blake
The previous patch dropped the only remaining use of
monitor_is_qmp.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
monitor/monitor-internal.h | 9 ---------
monitor/monitor.c | 1 -
2 files changed, 10 deletions(-)
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 7004f5d002..d40fdde793 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -106,7 +106,6 @@ struct Monitor {
Object parent;
CharFrontend chr;
int suspend_cnt; /* Needs to be accessed atomically */
- bool is_qmp;
bool use_io_thread;
char *mon_cpu_path;
@@ -171,14 +170,6 @@ struct MonitorQMP {
GQueue *qmp_requests;
};
-/**
- * Is @mon a QMP monitor?
- */
-static inline bool monitor_is_qmp(const Monitor *mon)
-{
- return mon->is_qmp;
-}
-
typedef QTAILQ_HEAD(MonitorList, Monitor) MonitorList;
extern IOThread *mon_iothread;
extern Coroutine *qmp_dispatcher_co;
diff --git a/monitor/monitor.c b/monitor/monitor.c
index c08b2c2ee1..f48603a7e8 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -94,7 +94,6 @@ static void monitor_init(Object *obj)
Monitor *mon = MONITOR(obj);
qemu_mutex_init(&mon->mon_lock);
- mon->is_qmp = !!object_dynamic_cast(obj, TYPE_MONITOR_QMP);
mon->outbuf = g_string_new(NULL);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 16/17] monitor: eliminate monitor_is_hmp_non_interactive method
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
` (14 preceding siblings ...)
2026-04-10 16:04 ` [PATCH 15/17] monitor: drop unused monitor_is_qmp method Daniel P. Berrangé
@ 2026-04-10 16:04 ` Daniel P. Berrangé
2026-04-13 22:47 ` Dr. David Alan Gilbert
2026-04-10 16:04 ` [PATCH 17/17] FIXME: monitor: implement "user creatable" interface Daniel P. Berrangé
2026-04-27 6:35 ` [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Markus Armbruster
17 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf,
Daniel P. Berrangé, qemu-block, Dr. David Alan Gilbert,
Eric Blake
The monitor_is_hmp_non_interactive method is used by
monitor_suspend and monitor_resume, to make them a no-op
if the HMP does not use readline.
There are only a handful of callers of suspend/resume and
they can be made to skip the call with readline is not
present.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
include/monitor/monitor.h | 2 +-
migration/migration-hmp-cmds.c | 5 ++++-
monitor/hmp-cmds.c | 5 ++++-
monitor/hmp.c | 14 +++++++++-----
monitor/monitor-internal.h | 1 -
monitor/monitor.c | 30 +-----------------------------
6 files changed, 19 insertions(+), 38 deletions(-)
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index f66e465775..52df053c6d 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -34,7 +34,7 @@ int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp);
int monitor_new_opts(QemuOpts *opts, Error **errp);
void monitor_cleanup(void);
-int monitor_suspend(Monitor *mon);
+void monitor_suspend(Monitor *mon);
void monitor_resume(Monitor *mon);
int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp);
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 0a193b8f54..e9bb970b20 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -18,6 +18,7 @@
#include "migration/snapshot.h"
#include "monitor/hmp.h"
#include "monitor/monitor.h"
+#include "monitor/monitor-internal.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-migration.h"
#include "qapi/qapi-visit-migration.h"
@@ -836,12 +837,14 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
if (!detach) {
HMPMigrationStatus *status;
+ MonitorHMP *hmp = MONITOR_HMP(mon);
- if (monitor_suspend(mon) < 0) {
+ if (!hmp->rs) {
monitor_printf(mon, "terminal does not allow synchronous "
"migration, continuing detached\n");
return;
}
+ monitor_suspend(mon);
status = g_malloc0(sizeof(*status));
status->mon = mon;
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 911d984cbe..a6314c1159 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -90,7 +90,10 @@ void hmp_info_version(Monitor *mon, const QDict *qdict)
void hmp_quit(Monitor *mon, const QDict *qdict)
{
- monitor_suspend(mon);
+ MonitorHMP *hmp = MONITOR_HMP(mon);
+ if (hmp->rs) {
+ monitor_suspend(mon);
+ }
qmp_quit(NULL);
}
diff --git a/monitor/hmp.c b/monitor/hmp.c
index 5f9373a87b..387dab2567 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -1491,13 +1491,16 @@ static void monitor_read(void *opaque, const uint8_t *buf, int size)
static void monitor_event(void *opaque, QEMUChrEvent event)
{
Monitor *mon = opaque;
+ MonitorHMP *hmp = MONITOR_HMP(mon);
switch (event) {
case CHR_EVENT_MUX_IN:
qemu_mutex_lock(&mon->mon_lock);
if (mon->mux_out) {
mon->mux_out = 0;
- monitor_resume(mon);
+ if (hmp->rs) {
+ monitor_resume(mon);
+ }
}
qemu_mutex_unlock(&mon->mon_lock);
break;
@@ -1510,7 +1513,9 @@ static void monitor_event(void *opaque, QEMUChrEvent event)
} else {
monitor_flush_locked(mon);
}
- monitor_suspend(mon);
+ if (hmp->rs) {
+ monitor_suspend(mon);
+ }
mon->mux_out = 1;
}
qemu_mutex_unlock(&mon->mon_lock);
@@ -1521,7 +1526,7 @@ static void monitor_event(void *opaque, QEMUChrEvent event)
"information\n", QEMU_VERSION);
qemu_mutex_lock(&mon->mon_lock);
mon->reset_seen = 1;
- if (!mon->mux_out) {
+ if (!mon->mux_out && hmp->rs) {
/* Suspend-resume forces the prompt to be printed. */
monitor_suspend(mon);
monitor_resume(mon);
@@ -1569,8 +1574,7 @@ void monitor_new_hmp(Chardev *chr, bool use_readline, Error **errp)
return;
}
- mon->use_readline = use_readline;
- if (mon->use_readline) {
+ if (use_readline) {
mon->rs = readline_init(monitor_readline_printf,
monitor_readline_flush,
mon,
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index d40fdde793..7557deb978 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -133,7 +133,6 @@ struct MonitorHMPClass {
struct MonitorHMP {
Monitor parent;
- bool use_readline;
/*
* State used only in the thread "owning" the monitor.
* If @use_io_thread, this is @mon_iothread. (This does not actually happen
diff --git a/monitor/monitor.c b/monitor/monitor.c
index f48603a7e8..3860486a32 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -130,25 +130,6 @@ Monitor *monitor_set_cur(Coroutine *co, Monitor *mon)
return old_monitor;
}
-/**
- * Is @mon is using readline?
- * Note: not all HMP monitors use readline, e.g., gdbserver has a
- * non-interactive HMP monitor, so readline is not used there.
- */
-static inline bool monitor_uses_readline(const MonitorHMP *mon)
-{
- return mon->use_readline;
-}
-
-static inline bool monitor_is_hmp_non_interactive(const Monitor *mon)
-{
- if (!object_dynamic_cast(OBJECT(mon), TYPE_MONITOR_HMP)) {
- return false;
- }
-
- return !monitor_uses_readline(container_of(mon, MonitorHMP, parent));
-}
-
static gboolean monitor_unblocked(void *do_not_use, GIOCondition cond,
void *opaque)
{
@@ -523,12 +504,8 @@ static gboolean qapi_event_throttle_equal(const void *a, const void *b)
return TRUE;
}
-int monitor_suspend(Monitor *mon)
+void monitor_suspend(Monitor *mon)
{
- if (monitor_is_hmp_non_interactive(mon)) {
- return -ENOTTY;
- }
-
qatomic_inc(&mon->suspend_cnt);
if (mon->use_io_thread) {
@@ -540,7 +517,6 @@ int monitor_suspend(Monitor *mon)
}
trace_monitor_suspend(mon, 1);
- return 0;
}
static void monitor_accept_input(void *opaque)
@@ -557,10 +533,6 @@ static void monitor_accept_input(void *opaque)
void monitor_resume(Monitor *mon)
{
- if (monitor_is_hmp_non_interactive(mon)) {
- return;
- }
-
if (qatomic_dec_fetch(&mon->suspend_cnt) == 0) {
AioContext *ctx;
--
2.53.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 16/17] monitor: eliminate monitor_is_hmp_non_interactive method
2026-04-10 16:04 ` [PATCH 16/17] monitor: eliminate monitor_is_hmp_non_interactive method Daniel P. Berrangé
@ 2026-04-13 22:47 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2026-04-13 22:47 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf, qemu-block,
Eric Blake
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> The monitor_is_hmp_non_interactive method is used by
> monitor_suspend and monitor_resume, to make them a no-op
> if the HMP does not use readline.
>
> There are only a handful of callers of suspend/resume and
> they can be made to skip the call with readline is not
> present.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
> ---
> include/monitor/monitor.h | 2 +-
> migration/migration-hmp-cmds.c | 5 ++++-
> monitor/hmp-cmds.c | 5 ++++-
> monitor/hmp.c | 14 +++++++++-----
> monitor/monitor-internal.h | 1 -
> monitor/monitor.c | 30 +-----------------------------
> 6 files changed, 19 insertions(+), 38 deletions(-)
>
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index f66e465775..52df053c6d 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -34,7 +34,7 @@ int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp);
> int monitor_new_opts(QemuOpts *opts, Error **errp);
> void monitor_cleanup(void);
>
> -int monitor_suspend(Monitor *mon);
> +void monitor_suspend(Monitor *mon);
> void monitor_resume(Monitor *mon);
>
> int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp);
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 0a193b8f54..e9bb970b20 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -18,6 +18,7 @@
> #include "migration/snapshot.h"
> #include "monitor/hmp.h"
> #include "monitor/monitor.h"
> +#include "monitor/monitor-internal.h"
> #include "qapi/error.h"
> #include "qapi/qapi-commands-migration.h"
> #include "qapi/qapi-visit-migration.h"
> @@ -836,12 +837,14 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
>
> if (!detach) {
> HMPMigrationStatus *status;
> + MonitorHMP *hmp = MONITOR_HMP(mon);
>
> - if (monitor_suspend(mon) < 0) {
> + if (!hmp->rs) {
> monitor_printf(mon, "terminal does not allow synchronous "
> "migration, continuing detached\n");
> return;
> }
> + monitor_suspend(mon);
>
> status = g_malloc0(sizeof(*status));
> status->mon = mon;
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 911d984cbe..a6314c1159 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -90,7 +90,10 @@ void hmp_info_version(Monitor *mon, const QDict *qdict)
>
> void hmp_quit(Monitor *mon, const QDict *qdict)
> {
> - monitor_suspend(mon);
> + MonitorHMP *hmp = MONITOR_HMP(mon);
> + if (hmp->rs) {
> + monitor_suspend(mon);
> + }
> qmp_quit(NULL);
> }
>
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index 5f9373a87b..387dab2567 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -1491,13 +1491,16 @@ static void monitor_read(void *opaque, const uint8_t *buf, int size)
> static void monitor_event(void *opaque, QEMUChrEvent event)
> {
> Monitor *mon = opaque;
> + MonitorHMP *hmp = MONITOR_HMP(mon);
>
> switch (event) {
> case CHR_EVENT_MUX_IN:
> qemu_mutex_lock(&mon->mon_lock);
> if (mon->mux_out) {
> mon->mux_out = 0;
> - monitor_resume(mon);
> + if (hmp->rs) {
> + monitor_resume(mon);
> + }
> }
> qemu_mutex_unlock(&mon->mon_lock);
> break;
> @@ -1510,7 +1513,9 @@ static void monitor_event(void *opaque, QEMUChrEvent event)
> } else {
> monitor_flush_locked(mon);
> }
> - monitor_suspend(mon);
> + if (hmp->rs) {
> + monitor_suspend(mon);
> + }
> mon->mux_out = 1;
> }
> qemu_mutex_unlock(&mon->mon_lock);
> @@ -1521,7 +1526,7 @@ static void monitor_event(void *opaque, QEMUChrEvent event)
> "information\n", QEMU_VERSION);
> qemu_mutex_lock(&mon->mon_lock);
> mon->reset_seen = 1;
> - if (!mon->mux_out) {
> + if (!mon->mux_out && hmp->rs) {
> /* Suspend-resume forces the prompt to be printed. */
> monitor_suspend(mon);
> monitor_resume(mon);
> @@ -1569,8 +1574,7 @@ void monitor_new_hmp(Chardev *chr, bool use_readline, Error **errp)
> return;
> }
>
> - mon->use_readline = use_readline;
> - if (mon->use_readline) {
> + if (use_readline) {
> mon->rs = readline_init(monitor_readline_printf,
> monitor_readline_flush,
> mon,
> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> index d40fdde793..7557deb978 100644
> --- a/monitor/monitor-internal.h
> +++ b/monitor/monitor-internal.h
> @@ -133,7 +133,6 @@ struct MonitorHMPClass {
>
> struct MonitorHMP {
> Monitor parent;
> - bool use_readline;
> /*
> * State used only in the thread "owning" the monitor.
> * If @use_io_thread, this is @mon_iothread. (This does not actually happen
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index f48603a7e8..3860486a32 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -130,25 +130,6 @@ Monitor *monitor_set_cur(Coroutine *co, Monitor *mon)
> return old_monitor;
> }
>
> -/**
> - * Is @mon is using readline?
> - * Note: not all HMP monitors use readline, e.g., gdbserver has a
> - * non-interactive HMP monitor, so readline is not used there.
> - */
> -static inline bool monitor_uses_readline(const MonitorHMP *mon)
> -{
> - return mon->use_readline;
> -}
> -
> -static inline bool monitor_is_hmp_non_interactive(const Monitor *mon)
> -{
> - if (!object_dynamic_cast(OBJECT(mon), TYPE_MONITOR_HMP)) {
> - return false;
> - }
> -
> - return !monitor_uses_readline(container_of(mon, MonitorHMP, parent));
> -}
> -
> static gboolean monitor_unblocked(void *do_not_use, GIOCondition cond,
> void *opaque)
> {
> @@ -523,12 +504,8 @@ static gboolean qapi_event_throttle_equal(const void *a, const void *b)
> return TRUE;
> }
>
> -int monitor_suspend(Monitor *mon)
> +void monitor_suspend(Monitor *mon)
> {
> - if (monitor_is_hmp_non_interactive(mon)) {
> - return -ENOTTY;
> - }
> -
> qatomic_inc(&mon->suspend_cnt);
>
> if (mon->use_io_thread) {
> @@ -540,7 +517,6 @@ int monitor_suspend(Monitor *mon)
> }
>
> trace_monitor_suspend(mon, 1);
> - return 0;
> }
>
> static void monitor_accept_input(void *opaque)
> @@ -557,10 +533,6 @@ static void monitor_accept_input(void *opaque)
>
> void monitor_resume(Monitor *mon)
> {
> - if (monitor_is_hmp_non_interactive(mon)) {
> - return;
> - }
> -
> if (qatomic_dec_fetch(&mon->suspend_cnt) == 0) {
> AioContext *ctx;
>
> --
> 2.53.0
>
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 17/17] FIXME: monitor: implement "user creatable" interface
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
` (15 preceding siblings ...)
2026-04-10 16:04 ` [PATCH 16/17] monitor: eliminate monitor_is_hmp_non_interactive method Daniel P. Berrangé
@ 2026-04-10 16:04 ` Daniel P. Berrangé
2026-04-27 6:35 ` [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Markus Armbruster
17 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Christian Brauner,
Alex Bennée, Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf,
Daniel P. Berrangé, qemu-block, Dr. David Alan Gilbert,
Eric Blake
Implement the user creatable QOM interface and define the monitor-qmp
and monitor-hmp types in QAPI. This unlocks the ability to create them
on the command line with -object or in HMP/QMP with object_add.
For example:
$QEMU -chardev stdio,id=monchr0 -object monitor-hmp,id=mon0,chrdev=monchr0
FIXME: object_del is NOT safe on monitors yet. Resolve against
Christian's patches for monitor deletion & synchronization.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
chardev/char.c | 10 +++++-
gdbstub/system.c | 10 ++++--
include/monitor/monitor.h | 2 --
monitor/hmp.c | 73 ++++++++++++++++++++++++++++++--------
monitor/monitor-internal.h | 2 ++
monitor/monitor.c | 70 +++++++++++++++++++++++++++++-------
monitor/qmp.c | 47 ++++++++++++++++++------
qapi/qom.json | 43 ++++++++++++++++++++++
stubs/monitor-internal.c | 4 ---
system/vl.c | 8 ++++-
10 files changed, 222 insertions(+), 47 deletions(-)
diff --git a/chardev/char.c b/chardev/char.c
index b59972f325..42eb799d78 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -785,8 +785,16 @@ static Chardev *qemu_chr_new_from_name(const char *label, const char *filename,
}
if (qemu_opt_get_bool(opts, "mux", 0)) {
+ const char *cdevid = qemu_opts_id(opts);
+ const char *monid = g_strdup_printf("mon%s", cdevid);
assert(permit_mux_mon);
- monitor_new_hmp(chr, true, &err);
+ object_new_with_props(TYPE_MONITOR_HMP,
+ object_get_objects_root(),
+ monid,
+ &err,
+ "chrdev", cdevid,
+ "readline", "yes",
+ NULL);
if (err) {
error_report_err(err);
object_unparent(OBJECT(chr));
diff --git a/gdbstub/system.c b/gdbstub/system.c
index 20dcf7878d..03b7bbeffc 100644
--- a/gdbstub/system.c
+++ b/gdbstub/system.c
@@ -386,9 +386,15 @@ bool gdbserver_start(const char *device, Error **errp)
qemu_add_vm_change_state_handler(gdb_vm_state_change, NULL);
/* Initialize a monitor terminal for gdb */
- mon_chr = qemu_chardev_new(NULL, TYPE_CHARDEV_GDB,
+ mon_chr = qemu_chardev_new("gdbchrdev0", TYPE_CHARDEV_GDB,
NULL, NULL, &error_abort);
- monitor_new_hmp(mon_chr, false, &error_abort);
+ object_new_with_props(TYPE_MONITOR_HMP,
+ object_get_objects_root(),
+ "gdbmon0",
+ &error_abort,
+ "chrdev", "gdbchrdev0",
+ "readline", "no",
+ NULL);
} 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 52df053c6d..dc7f0d0b48 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -28,8 +28,6 @@ Monitor *monitor_set_cur(Coroutine *co, Monitor *mon);
void monitor_init_globals(void);
void monitor_init_globals_core(void);
-void monitor_new_qmp(Chardev *chr, bool pretty, Error **errp);
-void monitor_new_hmp(Chardev *chr, bool use_readline, Error **errp);
int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp);
int monitor_new_opts(QemuOpts *opts, Error **errp);
void monitor_cleanup(void);
diff --git a/monitor/hmp.c b/monitor/hmp.c
index 387dab2567..02d3ebf7b4 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -38,6 +38,8 @@
#include "qemu/option.h"
#include "qemu/target-info.h"
#include "qemu/units.h"
+#include "qapi/error.h"
+#include "qom/object_interfaces.h"
#include "exec/gdbstub.h"
#include "system/block-backend.h"
#include "trace.h"
@@ -54,16 +56,47 @@ int monitor_hmp_vprintf(Monitor *mon, const char *fmt, va_list ap)
G_GNUC_PRINTF(2, 0);
static void monitor_hmp_accept_input(Monitor *mon);
+static bool monitor_hmp_get_readline(Object *obj, Error **errp)
+{
+ MonitorHMP *mon = MONITOR_HMP(obj);
+
+ return mon->readline;
+}
+
+static void monitor_hmp_set_readline(Object *obj, bool val, Error **errp)
+{
+ MonitorHMP *mon = MONITOR_HMP(obj);
+
+ mon->readline = val;
+}
+
+static void monitor_hmp_complete(UserCreatable *uc, Error **errp);
+
static void monitor_hmp_class_init(ObjectClass *cls, const void *data)
{
MonitorClass *moncls = MONITOR_CLASS(cls);
+ UserCreatableClass *ucc = USER_CREATABLE_CLASS(cls);
+
+ object_class_property_add_bool(cls, "readline",
+ monitor_hmp_get_readline,
+ monitor_hmp_set_readline);
moncls->vprintf = monitor_hmp_vprintf;
moncls->accept_input = monitor_hmp_accept_input;
+
+ ucc->complete = monitor_hmp_complete;
}
static void monitor_hmp_init(Object *obj)
{
+ MonitorHMP *hmp = MONITOR_HMP(obj);
+
+ /*
+ * Default to common case for external HMP use,
+ * as opposed to non-interactive internal use
+ * from gdbstub
+ */
+ hmp->readline = true;
}
int monitor_hmp_vprintf(Monitor *mon, const char *fmt, va_list ap)
@@ -1565,26 +1598,36 @@ static void monitor_readline_flush(void *opaque)
monitor_flush(&mon->parent);
}
-void monitor_new_hmp(Chardev *chr, bool use_readline, Error **errp)
+static void monitor_hmp_complete(UserCreatable *uc, Error **errp)
{
- MonitorHMP *mon = MONITOR_HMP(object_new(TYPE_MONITOR_HMP));
-
- if (!qemu_chr_fe_init(&mon->parent.chr, chr, errp)) {
- object_unref(mon);
+ MonitorHMP *mon = MONITOR_HMP(uc);
+ UserCreatableClass *ucc_parent =
+ USER_CREATABLE_CLASS(
+ object_class_get_parent(
+ OBJECT_CLASS(MONITOR_HMP_GET_CLASS(mon))));
+ ERRP_GUARD();
+
+ ucc_parent->complete(uc, errp);
+ if (*errp) {
return;
}
- if (use_readline) {
- mon->rs = readline_init(monitor_readline_printf,
- monitor_readline_flush,
- mon,
- monitor_find_completion);
- monitor_read_command(mon, 0);
- }
+ if (mon->parent.chrdev_id) {
+ if (mon->readline) {
+ mon->rs = readline_init(monitor_readline_printf,
+ monitor_readline_flush,
+ mon,
+ monitor_find_completion);
+ monitor_read_command(mon, 0);
+ }
- qemu_chr_fe_set_handlers(&mon->parent.chr, monitor_can_read, monitor_read,
- monitor_event, NULL, &mon->parent, NULL, true);
- monitor_list_append(&mon->parent);
+ qemu_chr_fe_set_handlers(&mon->parent.chr,
+ monitor_can_read,
+ monitor_read,
+ monitor_event, NULL,
+ &mon->parent, NULL, true);
+ monitor_list_append(&mon->parent);
+ }
}
/**
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 7557deb978..200f3ac284 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -104,6 +104,7 @@ struct MonitorClass {
struct Monitor {
Object parent;
+ char *chrdev_id;
CharFrontend chr;
int suspend_cnt; /* Needs to be accessed atomically */
bool use_io_thread;
@@ -133,6 +134,7 @@ struct MonitorHMPClass {
struct MonitorHMP {
Monitor parent;
+ bool readline;
/*
* State used only in the thread "owning" the monitor.
* If @use_io_thread, this is @mon_iothread. (This does not actually happen
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 3860486a32..bbc234d683 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -29,6 +29,7 @@
#include "qapi/qapi-emit-events.h"
#include "qapi/qapi-visit-control.h"
#include "qobject/qdict.h"
+#include "qom/object_interfaces.h"
#include "qemu/error-report.h"
#include "qemu/option.h"
#include "system/qtest.h"
@@ -73,7 +74,8 @@ static GHashTable *coroutine_mon; /* Maps Coroutine* to Monitor* */
MonitorList mon_list;
static bool monitor_destroyed;
-OBJECT_DEFINE_TYPE(Monitor, monitor, MONITOR, OBJECT);
+OBJECT_DEFINE_TYPE_EXTENDED(Monitor, monitor, MONITOR, OBJECT, true,
+ { TYPE_USER_CREATABLE }, {});
static void monitor_finalize(Object *obj)
{
@@ -85,8 +87,45 @@ static void monitor_finalize(Object *obj)
qemu_mutex_destroy(&mon->mon_lock);
}
+static void monitor_complete(UserCreatable *uc, Error **errp)
+{
+ Monitor *mon = MONITOR(uc);
+
+ if (mon->chrdev_id) {
+ Chardev *chr = qemu_chr_find(mon->chrdev_id);
+ if (chr == NULL) {
+ error_setg(errp, "chardev \"%s\" not found", mon->chrdev_id);
+ return;
+ }
+
+ if (!qemu_chr_fe_init(&mon->chr, chr, errp)) {
+ return;
+ }
+ }
+}
+
+static char *monitor_get_chrdev_id(Object *obj, Error **errp)
+{
+ Monitor *mon = MONITOR(obj);
+
+ return g_strdup(mon->chrdev_id);
+}
+
+static void monitor_set_chrdev_id(Object *obj, const char *str, Error **errp)
+{
+ Monitor *mon = MONITOR(obj);
+
+ mon->chrdev_id = g_strdup(str);
+}
+
static void monitor_class_init(ObjectClass *cls, const void *data)
{
+ UserCreatableClass *ucc = USER_CREATABLE_CLASS(cls);
+
+ object_class_property_add_str(cls, "chrdev",
+ monitor_get_chrdev_id, monitor_set_chrdev_id);
+
+ ucc->complete = monitor_complete;
}
static void monitor_init(Object *obj)
@@ -570,7 +609,7 @@ void monitor_list_append(Monitor *mon)
qemu_mutex_unlock(&monitor_lock);
if (mon) {
- object_unref(mon);
+ object_unparent(OBJECT(mon));
}
}
@@ -628,7 +667,7 @@ void monitor_cleanup(void)
qemu_mutex_unlock(&monitor_lock);
monitor_flush(mon);
qemu_mutex_lock(&monitor_lock);
- object_unref(mon);
+ object_unparent(OBJECT(mon));
}
qemu_mutex_unlock(&monitor_lock);
@@ -666,13 +705,8 @@ void monitor_init_globals(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;
- }
+ static int counter;
+ g_autofree char *id = g_strdup_printf("mon%d", counter++);
if (!opts->has_mode) {
opts->mode = allow_hmp ? MONITOR_MODE_READLINE : MONITOR_MODE_CONTROL;
@@ -680,7 +714,13 @@ int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp)
switch (opts->mode) {
case MONITOR_MODE_CONTROL:
- monitor_new_qmp(chr, opts->pretty, errp);
+ object_new_with_props(TYPE_MONITOR_QMP,
+ object_get_objects_root(),
+ id,
+ errp,
+ "chrdev", opts->chardev,
+ "pretty", opts->pretty ? "yes" : "no",
+ NULL);
break;
case MONITOR_MODE_READLINE:
if (!allow_hmp) {
@@ -691,7 +731,13 @@ 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(chr, true, errp);
+ object_new_with_props(TYPE_MONITOR_HMP,
+ object_get_objects_root(),
+ id,
+ errp,
+ "chrdev", opts->chardev,
+ "readline", "yes",
+ NULL);
break;
default:
g_assert_not_reached();
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 2ec9cca3a6..8fbc55757c 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -31,6 +31,7 @@
#include "qobject/qdict.h"
#include "qobject/qjson.h"
#include "qobject/qlist.h"
+#include "qom/object_interfaces.h"
#include "trace.h"
/*
@@ -85,13 +86,35 @@ static void monitor_qmp_finalize(Object *obj)
g_queue_free(mon->qmp_requests);
}
+static bool monitor_qmp_get_pretty(Object *obj, Error **errp)
+{
+ MonitorQMP *mon = MONITOR_QMP(obj);
+
+ return mon->pretty;
+}
+
+static void monitor_qmp_set_pretty(Object *obj, bool val, Error **errp)
+{
+ MonitorQMP *mon = MONITOR_QMP(obj);
+
+ mon->pretty = val;
+}
+
static void monitor_qmp_emit_event(Monitor *mon, QAPIEvent event, QDict *qdict);
+static void monitor_qmp_complete(UserCreatable *uc, Error **errp);
static void monitor_qmp_class_init(ObjectClass *cls, const void *data)
{
MonitorClass *moncls = MONITOR_CLASS(cls);
+ UserCreatableClass *ucc = USER_CREATABLE_CLASS(cls);
+
+ object_class_property_add_bool(cls, "pretty",
+ monitor_qmp_get_pretty,
+ monitor_qmp_set_pretty);
moncls->emit_event = monitor_qmp_emit_event;
+
+ ucc->complete = monitor_qmp_complete;
}
static void monitor_qmp_init(Object *obj)
@@ -547,23 +570,27 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
monitor_list_append(&mon->parent);
}
-void monitor_new_qmp(Chardev *chr, bool pretty, Error **errp)
+static void monitor_qmp_complete(UserCreatable *uc, Error **errp)
{
- MonitorQMP *mon = MONITOR_QMP(object_new(TYPE_MONITOR_QMP));
-
- if (!qemu_chr_fe_init(&mon->parent.chr, chr, errp)) {
- object_unref(mon);
+ MonitorQMP *mon = MONITOR_QMP(uc);
+ UserCreatableClass *ucc_parent =
+ USER_CREATABLE_CLASS(
+ object_class_get_parent(
+ OBJECT_CLASS(MONITOR_QMP_GET_CLASS(mon))));
+ ERRP_GUARD();
+
+ ucc_parent->complete(uc, errp);
+ if (*errp) {
return;
}
+
qemu_chr_fe_set_echo(&mon->parent.chr, true);
/* Note: we run QMP monitor in I/O thread when @chr supports that */
- if (qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT)) {
+ if (qemu_chr_has_feature(mon->parent.chr.chr, QEMU_CHAR_FEATURE_GCONTEXT)) {
monitor_iothread_init(&mon->parent);
}
- mon->pretty = pretty;
-
qemu_mutex_init(&mon->qmp_queue_lock);
mon->qmp_requests = g_queue_new();
@@ -573,12 +600,12 @@ void monitor_new_qmp(Chardev *chr, bool pretty, Error **errp)
* Make sure the old iowatch is gone. It's possible when
* e.g. the chardev is in client mode, with wait=on.
*/
- remove_fd_in_watch(chr);
+ remove_fd_in_watch(mon->parent.chr.chr);
/*
* Clean up listener IO sources early to prevent racy fd
* handling between the main thread and the I/O thread.
*/
- remove_listener_fd_in_watch(chr);
+ remove_listener_fd_in_watch(mon->parent.chr.chr);
/*
* We can't call qemu_chr_fe_set_handlers() directly here
* since chardev might be running in the monitor I/O
diff --git a/qapi/qom.json b/qapi/qom.json
index c653248f85..53264bcd7b 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -1179,6 +1179,45 @@
'data': { '*cpu-affinity': ['uint16'],
'*node-affinity': ['uint16'] } }
+##
+# @MonitorProperties:
+#
+# Properties for all monitors
+#
+# @chrdev: ID of the character device providing the monitor transport
+#
+# Since: 11.1
+##
+{ 'struct': 'MonitorProperties',
+ 'data': { 'chrdev': 'str' }}
+
+##
+# @MonitorHMPProperties:
+#
+# Properties for the HMP monitor
+#
+# @readline: whether to enable readline for interactive command
+# usage (default: enabled)
+#
+# Since: 11.1
+##
+{ 'struct': 'MonitorHMPProperties',
+ 'base': 'MonitorProperties',
+ 'data': { '*readline': 'bool' } }
+
+##
+# @MonitorQMPProperties:
+#
+# Properties for the QMP monitor
+#
+# @pretty: whether to pretty print JSON responses (default: disabled)
+#
+# Since: 11.1
+##
+{ 'struct': 'MonitorQMPProperties',
+ 'base': 'MonitorProperties',
+ 'data': { '*pretty': 'bool' } }
+
##
# @ObjectType:
#
@@ -1229,6 +1268,8 @@
'memory-backend-ram',
{ 'name': 'memory-backend-shm',
'if': 'CONFIG_POSIX' },
+ 'monitor-hmp',
+ 'monitor-qmp',
'pef-guest',
{ 'name': 'pr-manager-helper',
'if': 'CONFIG_LINUX' },
@@ -1307,6 +1348,8 @@
'memory-backend-ram': 'MemoryBackendProperties',
'memory-backend-shm': { 'type': 'MemoryBackendShmProperties',
'if': 'CONFIG_POSIX' },
+ 'monitor-hmp': {'type': 'MonitorHMPProperties' },
+ 'monitor-qmp': {'type': 'MonitorQMPProperties' },
'pr-manager-helper': { 'type': 'PrManagerHelperProperties',
'if': 'CONFIG_LINUX' },
'qtest': 'QtestProperties',
diff --git a/stubs/monitor-internal.c b/stubs/monitor-internal.c
index 23d58da184..cbec2f35e0 100644
--- a/stubs/monitor-internal.c
+++ b/stubs/monitor-internal.c
@@ -7,7 +7,3 @@ int monitor_get_fd(Monitor *mon, const char *name, Error **errp)
error_setg(errp, "only QEMU supports file descriptor passing");
return -1;
}
-
-void monitor_new_hmp(Chardev *chr, bool use_readline, Error **errp)
-{
-}
diff --git a/system/vl.c b/system/vl.c
index 2391811a46..e772e1401f 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1823,6 +1823,10 @@ static void object_option_add_visitor(Visitor *v)
{
ObjectOption *opt = g_new0(ObjectOption, 1);
visit_type_ObjectOptions(v, NULL, &opt->opts, &error_fatal);
+ if (opt->opts->qom_type == OBJECT_TYPE_MONITOR_HMP ||
+ opt->opts->qom_type == OBJECT_TYPE_MONITOR_QMP) {
+ default_monitor = 0;
+ }
QTAILQ_INSERT_TAIL(&object_opts, opt, next);
}
@@ -1964,7 +1968,9 @@ static bool object_create_early(const char *type)
/* Reason: property "chardev" */
if (g_str_equal(type, "rng-egd") ||
- g_str_equal(type, "qtest")) {
+ g_str_equal(type, "qtest") ||
+ g_str_equal(type, "monitor-hmp") ||
+ g_str_equal(type, "monitor-qmp")) {
return false;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects
2026-04-10 16:04 [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Daniel P. Berrangé
` (16 preceding siblings ...)
2026-04-10 16:04 ` [PATCH 17/17] FIXME: monitor: implement "user creatable" interface Daniel P. Berrangé
@ 2026-04-27 6:35 ` Markus Armbruster
2026-04-27 7:04 ` Daniel P. Berrangé
17 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2026-04-27 6:35 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Paolo Bonzini, Christian Brauner, Alex Bennée,
Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf, qemu-block,
Dr. David Alan Gilbert, Eric Blake
Daniel P. Berrangé <berrange@redhat.com> writes:
> Conceptually -object and object_add/object_del should be sufficient
> for essentially all QEMU configuration....if only we ported all our
> internal custom backends/devices/etc to QOM. That is of course a big
> job which is why it hasn't happened.
In addition, we'd need a more unified object life cycle. See "Problem
5: QOM lacks a clear life cycle" in
Subject: Dynamic & heterogeneous machines, initial configuration: problems
Date: Wed, 31 Jan 2024 21:14:21 +0100
Message-ID: <87o7d1i7ky.fsf@pond.sub.org>
https://lore.kernel.org/qemu-devel/87o7d1i7ky.fsf@pond.sub.org/
> This series started with the premise that the monitor is one of the
> easier areas to convert since we have no more than three classes,
> a common base, and QMP and HMP subclasses. So why not give it a go
> and thus unlock the ability to dynamically create/delete monitors
> in QMP/HMP.
Makes sense.
> This series does the conversion in as many small steps as I could
> achieve. At the end it is possible to create HMP and QMP monitors
> with -object and object_add.
I'll try to squeeze in review.
> In theory object_del is possible but is unsafe since there's no
> synchronization wrt "in use" monitors. Christian Brauner sent
> a proposal for monitor_add/monitor_del commands that address the
> safety/synchronization issues, but I've not yet integrated the
> relevent bits of that series with this proposal:
>
> https://lists.nongnu.org/archive/html/qemu-devel/2026-04/msg01349.html
>
> My overall feeling is that we're better off turning monitors
> into QOM classes, rather than inventing more type specific
> QMP commands.
I agree!
> What's missing:
>
> - Integration of code for safe deletion of monitors
> - The existing -mon/-qmp/-monitor args should be turned
> into syntax sugar around -object & -chardev
Yes. A step we sometimes neglect.
> - The new approach must be documented
> - Functional tests are needed for the hot add/remove of
> monitors and new -object syntax. (The pre-existing
> test suites cover the static config with the traditional
> args).
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects
2026-04-27 6:35 ` [PATCH RFC 00/17] monitor: turn QMP and HMP into QOM objects Markus Armbruster
@ 2026-04-27 7:04 ` Daniel P. Berrangé
0 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrangé @ 2026-04-27 7:04 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Paolo Bonzini, Christian Brauner, Alex Bennée,
Philippe Mathieu-Daudé, Fabiano Rosas,
Marc-André Lureau, Peter Xu, Kevin Wolf, qemu-block,
Dr. David Alan Gilbert, Eric Blake
On Mon, Apr 27, 2026 at 08:35:41AM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
> > Conceptually -object and object_add/object_del should be sufficient
> > for essentially all QEMU configuration....if only we ported all our
> > internal custom backends/devices/etc to QOM. That is of course a big
> > job which is why it hasn't happened.
>
> In addition, we'd need a more unified object life cycle. See "Problem
> 5: QOM lacks a clear life cycle" in
>
> Subject: Dynamic & heterogeneous machines, initial configuration: problems
> Date: Wed, 31 Jan 2024 21:14:21 +0100
> Message-ID: <87o7d1i7ky.fsf@pond.sub.org>
> https://lore.kernel.org/qemu-devel/87o7d1i7ky.fsf@pond.sub.org/
>
> > This series started with the premise that the monitor is one of the
> > easier areas to convert since we have no more than three classes,
> > a common base, and QMP and HMP subclasses. So why not give it a go
> > and thus unlock the ability to dynamically create/delete monitors
> > in QMP/HMP.
>
> Makes sense.
>
> > This series does the conversion in as many small steps as I could
> > achieve. At the end it is possible to create HMP and QMP monitors
> > with -object and object_add.
>
> I'll try to squeeze in review.
Don't review this v1 yet. I've got a v2 I'm finishing off to send
real soon.
^ permalink raw reply [flat|nested] 32+ messages in thread