Add 'id', 'dynamic', and 'dead' fields to struct Monitor. The id field
stores the monitor identifier from MonitorOptions which was previously
parsed but discarded. The dynamic flag marks monitors created at runtime
via the upcoming monitor-add command, and the dead flag will be used for
deferred destruction during monitor-remove.
Extend monitor_init_qmp() to accept id and dynamic parameters so these
are set before the monitor is added to mon_list.
For iothread monitors, move monitor_list_append() from the setup BH to
the caller so monitor_find_by_id() can detect duplicates immediately.
Without this, two concurrent monitor-add calls could both pass the
duplicate check before either BH runs. This means the monitor is now
visible in mon_list before its chardev handlers are set up, which was
not the case before. This is safe because the request queue is still
empty (no chardev handlers means no monitor_qmp_read(), so the
dispatcher finds nothing to dispatch) and event broadcast is handled
below.
This requires initializing mon->commands = &qmp_cap_negotiation_commands
before monitor_list_append(). Without it, commands is NULL (from
g_new0) and monitor_qapi_event_emit() would not skip the monitor during
event broadcast -- its check is specifically for the
qmp_cap_negotiation_commands pointer, so a NULL falls through to
qmp_send_response() on an uninitialized monitor. CHR_EVENT_OPENED sets
commands to the same value again later.
Add monitor_find_by_id() to look up monitors by identifier. The lookup
takes monitor_lock to serialize with the I/O thread BH that modifies
mon_list, but releases it before returning. The caller must hold the
BQL to ensure the returned pointer remains valid since only BQL holders
can destroy monitors.
Free the id string in monitor_data_destroy().
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
include/monitor/monitor.h | 3 ++-
monitor/monitor-internal.h | 5 +++++
monitor/monitor.c | 21 ++++++++++++++++++++-
monitor/qmp.c | 11 ++++++++---
4 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 296690e1f1..7a2bb603e4 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -19,7 +19,8 @@ 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_qmp(Chardev *chr, bool pretty, const char *id,
+ bool dynamic, 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);
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index feca111ae3..4896812d4e 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -98,7 +98,10 @@ struct Monitor {
bool is_qmp;
bool skip_flush;
bool use_io_thread;
+ bool dynamic; /* true if created via monitor-add */
+ bool dead; /* awaiting drain after monitor-remove */
+ char *id; /* NULL for unnamed CLI monitors */
char *mon_cpu_path;
QTAILQ_ENTRY(Monitor) entry;
@@ -181,6 +184,8 @@ void monitor_data_destroy_qmp(MonitorQMP *mon);
void coroutine_fn monitor_qmp_dispatcher_co(void *data);
void qmp_dispatcher_co_wake(void);
+Monitor *monitor_find_by_id(const char *id);
+
int get_monitor_def(Monitor *mon, int64_t *pval, const char *name);
void handle_hmp_command(MonitorHMP *mon, const char *cmdline);
int hmp_compare_cmd(const char *name, const char *list);
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 00b93ed612..7144255e12 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -622,6 +622,7 @@ void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
void monitor_data_destroy(Monitor *mon)
{
+ g_free(mon->id);
g_free(mon->mon_cpu_path);
qemu_chr_fe_deinit(&mon->chr, false);
if (monitor_is_qmp(mon)) {
@@ -633,6 +634,24 @@ void monitor_data_destroy(Monitor *mon)
qemu_mutex_destroy(&mon->mon_lock);
}
+/*
+ * Look up a monitor by its id. The monitor_lock is released before
+ * returning, so the caller must hold the BQL to ensure the returned
+ * pointer remains valid (only BQL holders can destroy monitors).
+ */
+Monitor *monitor_find_by_id(const char *id)
+{
+ Monitor *mon;
+
+ QEMU_LOCK_GUARD(&monitor_lock);
+ QTAILQ_FOREACH(mon, &mon_list, entry) {
+ if (mon->id && strcmp(mon->id, id) == 0) {
+ return mon;
+ }
+ }
+ return NULL;
+}
+
void monitor_cleanup(void)
{
/*
@@ -732,7 +751,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_init_qmp(chr, opts->pretty, opts->id, false, errp);
break;
case MONITOR_MODE_READLINE:
if (!allow_hmp) {
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 687019811f..afbe2283d6 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -510,10 +510,10 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read,
monitor_qmp_read, monitor_qmp_event,
NULL, &mon->common, context, true);
- monitor_list_append(&mon->common);
}
-void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
+void monitor_init_qmp(Chardev *chr, bool pretty, const char *id,
+ bool dynamic, Error **errp)
{
MonitorQMP *mon = g_new0(MonitorQMP, 1);
@@ -527,12 +527,16 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
monitor_data_init(&mon->common, true, false,
qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
+ mon->common.id = g_strdup(id);
+ mon->common.dynamic = dynamic;
mon->pretty = pretty;
qemu_mutex_init(&mon->qmp_queue_lock);
mon->qmp_requests = g_queue_new();
json_message_parser_init(&mon->parser, handle_qmp_command, mon, NULL);
+ /* Prevent event broadcast to an uninitialized monitor. */
+ mon->commands = &qmp_cap_negotiation_commands;
if (mon->common.use_io_thread) {
/*
* Make sure the old iowatch is gone. It's possible when
@@ -551,7 +555,8 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
*/
aio_bh_schedule_oneshot(iothread_get_aio_context(mon_iothread),
monitor_qmp_setup_handlers_bh, mon);
- /* The bottom half will add @mon to @mon_list */
+ /* Synchronous insert for immediate duplicate detection. */
+ monitor_list_append(&mon->common);
} else {
qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read,
monitor_qmp_read, monitor_qmp_event,
--
2.47.3On Tue, Apr 07, 2026 at 09:32:45AM +0200, Christian Brauner wrote:
> Add 'id', 'dynamic', and 'dead' fields to struct Monitor. The id field
> stores the monitor identifier from MonitorOptions which was previously
> parsed but discarded. The dynamic flag marks monitors created at runtime
> via the upcoming monitor-add command, and the dead flag will be used for
> deferred destruction during monitor-remove.
>
> Extend monitor_init_qmp() to accept id and dynamic parameters so these
> are set before the monitor is added to mon_list.
>
> For iothread monitors, move monitor_list_append() from the setup BH to
> the caller so monitor_find_by_id() can detect duplicates immediately.
> Without this, two concurrent monitor-add calls could both pass the
> duplicate check before either BH runs. This means the monitor is now
> visible in mon_list before its chardev handlers are set up, which was
> not the case before. This is safe because the request queue is still
> empty (no chardev handlers means no monitor_qmp_read(), so the
> dispatcher finds nothing to dispatch) and event broadcast is handled
> below.
>
> This requires initializing mon->commands = &qmp_cap_negotiation_commands
> before monitor_list_append(). Without it, commands is NULL (from
> g_new0) and monitor_qapi_event_emit() would not skip the monitor during
> event broadcast -- its check is specifically for the
> qmp_cap_negotiation_commands pointer, so a NULL falls through to
> qmp_send_response() on an uninitialized monitor. CHR_EVENT_OPENED sets
> commands to the same value again later.
>
> Add monitor_find_by_id() to look up monitors by identifier. The lookup
> takes monitor_lock to serialize with the I/O thread BH that modifies
> mon_list, but releases it before returning. The caller must hold the
> BQL to ensure the returned pointer remains valid since only BQL holders
> can destroy monitors.
>
> Free the id string in monitor_data_destroy().
>
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> ---
> include/monitor/monitor.h | 3 ++-
> monitor/monitor-internal.h | 5 +++++
> monitor/monitor.c | 21 ++++++++++++++++++++-
> monitor/qmp.c | 11 ++++++++---
> 4 files changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index 296690e1f1..7a2bb603e4 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -19,7 +19,8 @@ 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_qmp(Chardev *chr, bool pretty, const char *id,
> + bool dynamic, 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);
> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> index feca111ae3..4896812d4e 100644
> --- a/monitor/monitor-internal.h
> +++ b/monitor/monitor-internal.h
> @@ -98,7 +98,10 @@ struct Monitor {
> bool is_qmp;
> bool skip_flush;
> bool use_io_thread;
> + bool dynamic; /* true if created via monitor-add */
> + bool dead; /* awaiting drain after monitor-remove */
Neither of these fields appear to be used for anything in this
patch. If they're used in a later patch, then add them at time
of use.
>
> + char *id; /* NULL for unnamed CLI monitors */
> char *mon_cpu_path;
> QTAILQ_ENTRY(Monitor) entry;
>
> @@ -181,6 +184,8 @@ void monitor_data_destroy_qmp(MonitorQMP *mon);
> void coroutine_fn monitor_qmp_dispatcher_co(void *data);
> void qmp_dispatcher_co_wake(void);
>
> +Monitor *monitor_find_by_id(const char *id);
> +
> int get_monitor_def(Monitor *mon, int64_t *pval, const char *name);
> void handle_hmp_command(MonitorHMP *mon, const char *cmdline);
> int hmp_compare_cmd(const char *name, const char *list);
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 00b93ed612..7144255e12 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -622,6 +622,7 @@ void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
>
> void monitor_data_destroy(Monitor *mon)
> {
> + g_free(mon->id);
> g_free(mon->mon_cpu_path);
> qemu_chr_fe_deinit(&mon->chr, false);
> if (monitor_is_qmp(mon)) {
> @@ -633,6 +634,24 @@ void monitor_data_destroy(Monitor *mon)
> qemu_mutex_destroy(&mon->mon_lock);
> }
>
> +/*
> + * Look up a monitor by its id. The monitor_lock is released before
> + * returning, so the caller must hold the BQL to ensure the returned
> + * pointer remains valid (only BQL holders can destroy monitors).
> + */
> +Monitor *monitor_find_by_id(const char *id)
> +{
> + Monitor *mon;
> +
> + QEMU_LOCK_GUARD(&monitor_lock);
> + QTAILQ_FOREACH(mon, &mon_list, entry) {
> + if (mon->id && strcmp(mon->id, id) == 0) {
> + return mon;
> + }
> + }
> + return NULL;
> +}
> +
> void monitor_cleanup(void)
> {
> /*
> @@ -732,7 +751,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_init_qmp(chr, opts->pretty, opts->id, false, errp);
> break;
> case MONITOR_MODE_READLINE:
> if (!allow_hmp) {
> diff --git a/monitor/qmp.c b/monitor/qmp.c
> index 687019811f..afbe2283d6 100644
> --- a/monitor/qmp.c
> +++ b/monitor/qmp.c
> @@ -510,10 +510,10 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
> qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read,
> monitor_qmp_read, monitor_qmp_event,
> NULL, &mon->common, context, true);
> - monitor_list_append(&mon->common);
> }
>
> -void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
> +void monitor_init_qmp(Chardev *chr, bool pretty, const char *id,
> + bool dynamic, Error **errp)
> {
> MonitorQMP *mon = g_new0(MonitorQMP, 1);
>
> @@ -527,12 +527,16 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
> monitor_data_init(&mon->common, true, false,
> qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
>
> + mon->common.id = g_strdup(id);
> + mon->common.dynamic = dynamic;
> mon->pretty = pretty;
>
> qemu_mutex_init(&mon->qmp_queue_lock);
> mon->qmp_requests = g_queue_new();
>
> json_message_parser_init(&mon->parser, handle_qmp_command, mon, NULL);
> + /* Prevent event broadcast to an uninitialized monitor. */
> + mon->commands = &qmp_cap_negotiation_commands;
> if (mon->common.use_io_thread) {
> /*
> * Make sure the old iowatch is gone. It's possible when
> @@ -551,7 +555,8 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp)
> */
> aio_bh_schedule_oneshot(iothread_get_aio_context(mon_iothread),
> monitor_qmp_setup_handlers_bh, mon);
> - /* The bottom half will add @mon to @mon_list */
> + /* Synchronous insert for immediate duplicate detection. */
> + monitor_list_append(&mon->common);
> } else {
> qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read,
> monitor_qmp_read, monitor_qmp_event,
>
> --
> 2.47.3
>
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 :|
On Tue, Apr 07, 2026 at 11:39:31AM +0100, Daniel P. Berrangé wrote:
> On Tue, Apr 07, 2026 at 09:32:45AM +0200, Christian Brauner wrote:
> > Add 'id', 'dynamic', and 'dead' fields to struct Monitor. The id field
> > stores the monitor identifier from MonitorOptions which was previously
> > parsed but discarded. The dynamic flag marks monitors created at runtime
> > via the upcoming monitor-add command, and the dead flag will be used for
> > deferred destruction during monitor-remove.
> >
> > Extend monitor_init_qmp() to accept id and dynamic parameters so these
> > are set before the monitor is added to mon_list.
> >
> > For iothread monitors, move monitor_list_append() from the setup BH to
> > the caller so monitor_find_by_id() can detect duplicates immediately.
> > Without this, two concurrent monitor-add calls could both pass the
> > duplicate check before either BH runs. This means the monitor is now
> > visible in mon_list before its chardev handlers are set up, which was
> > not the case before. This is safe because the request queue is still
> > empty (no chardev handlers means no monitor_qmp_read(), so the
> > dispatcher finds nothing to dispatch) and event broadcast is handled
> > below.
> >
> > This requires initializing mon->commands = &qmp_cap_negotiation_commands
> > before monitor_list_append(). Without it, commands is NULL (from
> > g_new0) and monitor_qapi_event_emit() would not skip the monitor during
> > event broadcast -- its check is specifically for the
> > qmp_cap_negotiation_commands pointer, so a NULL falls through to
> > qmp_send_response() on an uninitialized monitor. CHR_EVENT_OPENED sets
> > commands to the same value again later.
> >
> > Add monitor_find_by_id() to look up monitors by identifier. The lookup
> > takes monitor_lock to serialize with the I/O thread BH that modifies
> > mon_list, but releases it before returning. The caller must hold the
> > BQL to ensure the returned pointer remains valid since only BQL holders
> > can destroy monitors.
> >
> > Free the id string in monitor_data_destroy().
> >
> > Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> > ---
> > include/monitor/monitor.h | 3 ++-
> > monitor/monitor-internal.h | 5 +++++
> > monitor/monitor.c | 21 ++++++++++++++++++++-
> > monitor/qmp.c | 11 ++++++++---
> > 4 files changed, 35 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> > index 296690e1f1..7a2bb603e4 100644
> > --- a/include/monitor/monitor.h
> > +++ b/include/monitor/monitor.h
> > @@ -19,7 +19,8 @@ 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_qmp(Chardev *chr, bool pretty, const char *id,
> > + bool dynamic, 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);
> > diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> > index feca111ae3..4896812d4e 100644
> > --- a/monitor/monitor-internal.h
> > +++ b/monitor/monitor-internal.h
> > @@ -98,7 +98,10 @@ struct Monitor {
> > bool is_qmp;
> > bool skip_flush;
> > bool use_io_thread;
> > + bool dynamic; /* true if created via monitor-add */
> > + bool dead; /* awaiting drain after monitor-remove */
>
> Neither of these fields appear to be used for anything in this
> patch. If they're used in a later patch, then add them at time
> of use.
Ok.
© 2016 - 2026 Red Hat, Inc.