* [PATCH v4 1/5] monitor: store monitor id in Monitor struct
2026-04-09 7:18 [PATCH v4 0/5] monitor: add dynamic QMP monitor hotplug support Christian Brauner
@ 2026-04-09 7:18 ` Christian Brauner
2026-04-09 7:18 ` [PATCH v4 2/5] monitor/qmp: add infrastructure for safe dynamic monitor removal Christian Brauner
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Christian Brauner @ 2026-04-09 7:18 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Eric Blake, Fabiano Rosas, Laurent Vivier,
Paolo Bonzini, Thomas Huth, Philippe Mathieu-Daudé,
Daniel P. Berrangé, Christian Brauner
Add an 'id' field to struct Monitor. The id field stores the monitor
identifier from MonitorOptions which was previously parsed but
discarded.
Auto-generate a unique id ("mon0", "mon1", ...) for QMP monitors
created via CLI without an explicit id, so that every QMP monitor is
addressable by monitor-remove and always appears with an id in
query-monitors output.
Extend monitor_init_qmp() to accept an id parameter so it is 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 | 4 +++-
monitor/monitor.c | 21 ++++++++++++++++++++-
monitor/qmp.c | 15 ++++++++++++---
4 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 296690e1f1..a4e6aaa97f 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,
+ 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..24d3b1900e 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -98,7 +98,7 @@ struct Monitor {
bool is_qmp;
bool skip_flush;
bool use_io_thread;
-
+ char *id;
char *mon_cpu_path;
QTAILQ_ENTRY(Monitor) entry;
@@ -181,6 +181,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..10a32150e9 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, errp);
break;
case MONITOR_MODE_READLINE:
if (!allow_hmp) {
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 687019811f..bba69a3a40 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,
+ Error **errp)
{
MonitorQMP *mon = g_new0(MonitorQMP, 1);
@@ -527,12 +527,20 @@ 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));
+ if (id) {
+ mon->common.id = g_strdup(id);
+ } else {
+ static unsigned int qmp_monitor_id_counter;
+ mon->common.id = g_strdup_printf("mon%u", qmp_monitor_id_counter++);
+ }
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 +559,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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 2/5] monitor/qmp: add infrastructure for safe dynamic monitor removal
2026-04-09 7:18 [PATCH v4 0/5] monitor: add dynamic QMP monitor hotplug support Christian Brauner
2026-04-09 7:18 ` [PATCH v4 1/5] monitor: store monitor id in Monitor struct Christian Brauner
@ 2026-04-09 7:18 ` Christian Brauner
2026-04-09 7:18 ` [PATCH v4 3/5] qapi: add monitor-add, monitor-remove, query-monitors commands Christian Brauner
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Christian Brauner @ 2026-04-09 7:18 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Eric Blake, Fabiano Rosas, Laurent Vivier,
Paolo Bonzini, Thomas Huth, Philippe Mathieu-Daudé,
Daniel P. Berrangé, Christian Brauner
Add 'dead' field to struct Monitor for deferred destruction during
monitor-remove.
Add monitor_qmp_destroy() to allow destroying a single QMP monitor at
runtime without shutting down the entire dispatcher coroutine.
Convert monitor_accept_input from a oneshot BH (aio_bh_schedule_oneshot)
to a persistent BH (aio_bh_new + qemu_bh_schedule). Oneshot BHs cannot
be cancelled, so monitor_resume() racing with destruction would schedule
a callback against memory that monitor_qmp_destroy() is about to free.
A persistent BH can be deleted during destruction, cancelling any
pending schedule.
Move qemu_chr_fe_accept_input() under mon_lock in monitor_accept_input()
so it cannot race with monitor_qmp_destroy() which deletes the BH under
the same lock.
Extract monitor_cancel_out_watch() for cancelling a pending out_watch
GSource. g_source_remove() only searches the default GMainContext but
iothread monitors attach the watch to the iothread context, so
g_main_context_find_source_by_id() with the correct context followed by
g_source_destroy() is needed. When chardev handlers have been
disconnected via qemu_chr_fe_set_handlers(NULL), s->gcontext is reset to
NULL by qemu_chr_be_update_read_handlers(). A watch created after the
reset (e.g. by a self-removal response flush) lands on the default
GMainContext rather than the iothread context. Fall back to searching
the default context when the iothread context search misses.
The monitor_qmp_destroy() sequence is:
1. Delete accept_input_bh (cancel pending resume).
2. Under mon_lock: set skip_flush so no further writes can create
new out_watch GSources, then cancel any existing out_watch.
skip_flush must be set first because the chardev gcontext has
already been reset to NULL by handler disconnect -- a flush at
this point would attach the watch to the default GMainContext
rather than the iothread context, and
monitor_cancel_out_watch() searching the iothread context would
miss it, leaking a GSource that fires monitor_unblocked() on
freed memory.
3. For iothread monitors: synchronize with the iothread via
aio_wait_bh_oneshot(). An in-flight monitor_unblocked() GSource
callback may be blocked on mon_lock (held in step 2) and will
resume after we release it. Since BHs cannot fire while a
GSource callback is dispatching, the no-op BH only runs after
monitor_unblocked() has returned, making destruction safe.
4. Final drain of the request queue to catch requests enqueued by an
in-flight monitor_qmp_read() that raced with the drain in
qmp_monitor_remove().
5. monitor_data_destroy() + g_free().
Add qmp_dispatcher_current_mon tracking in the dispatcher coroutine to
handle self-removal (a monitor sends monitor-remove targeting itself).
Both the dispatcher yield points and QMP command handlers run under the
BQL, so no additional locking is needed. After dispatching each
request, the dispatcher checks the dead flag: if set, it closes the
chardev connection, calls monitor_qmp_destroy(), and clears the tracking
pointer. monitor_resume() is skipped because the chardev handlers are
already disconnected, and resume would schedule a BH against a monitor
about to be freed.
Add a setup_pending flag for iothread monitors so qmp_monitor_remove()
can reject removal before the setup BH has completed.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
monitor/monitor-internal.h | 8 ++++++
monitor/monitor.c | 49 ++++++++++++++++++++++++++--------
monitor/qmp.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 112 insertions(+), 11 deletions(-)
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 24d3b1900e..f655ac5611 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -98,6 +98,9 @@ struct Monitor {
bool is_qmp;
bool skip_flush;
bool use_io_thread;
+ bool dead; /* awaiting drain after monitor-remove */
+ QEMUBH *accept_input_bh; /* persistent BH for monitor_accept_input */
+
char *id;
char *mon_cpu_path;
QTAILQ_ENTRY(Monitor) entry;
@@ -135,6 +138,7 @@ typedef struct {
Monitor common;
JSONMessageParser parser;
bool pretty;
+ bool setup_pending; /* iothread BH has not yet set up chardev handlers */
/*
* When a client connects, we're in capabilities negotiation mode.
* @commands is &qmp_cap_negotiation_commands then. When command
@@ -173,15 +177,19 @@ void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
bool use_io_thread);
void monitor_data_destroy(Monitor *mon);
int monitor_can_read(void *opaque);
+void monitor_cancel_out_watch(Monitor *mon);
void monitor_list_append(Monitor *mon);
void monitor_fdsets_cleanup(void);
void qmp_send_response(MonitorQMP *mon, const QDict *rsp);
void monitor_data_destroy_qmp(MonitorQMP *mon);
+void monitor_qmp_destroy(MonitorQMP *mon);
+void monitor_qmp_drain_queue(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);
+bool monitor_qmp_dispatcher_is_servicing(MonitorQMP *mon);
int get_monitor_def(Monitor *mon, int64_t *pval, const char *name);
void handle_hmp_command(MonitorHMP *mon, const char *cmdline);
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 10a32150e9..2bf0a4934f 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -146,6 +146,28 @@ static gboolean monitor_unblocked(void *do_not_use, GIOCondition cond,
return G_SOURCE_REMOVE;
}
+/* Cancel a pending out_watch GSource. Caller must hold mon_lock. */
+void monitor_cancel_out_watch(Monitor *mon)
+{
+ if (mon->out_watch) {
+ GMainContext *ctx = NULL;
+ GSource *src;
+
+ if (mon->use_io_thread) {
+ ctx = iothread_get_g_main_context(mon_iothread);
+ }
+ src = g_main_context_find_source_by_id(ctx, mon->out_watch);
+ if (!src && ctx) {
+ /* Handler disconnect may have reset gcontext to NULL. */
+ src = g_main_context_find_source_by_id(NULL, mon->out_watch);
+ }
+ if (src) {
+ g_source_destroy(src);
+ }
+ mon->out_watch = 0;
+ }
+}
+
/* Caller must hold mon->mon_lock */
void monitor_flush_locked(Monitor *mon)
{
@@ -545,13 +567,13 @@ static void monitor_accept_input(void *opaque)
MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
assert(hmp_mon->rs);
readline_restart(hmp_mon->rs);
+ qemu_chr_fe_accept_input(&mon->chr);
qemu_mutex_unlock(&mon->mon_lock);
readline_show_prompt(hmp_mon->rs);
} else {
+ qemu_chr_fe_accept_input(&mon->chr);
qemu_mutex_unlock(&mon->mon_lock);
}
-
- qemu_chr_fe_accept_input(&mon->chr);
}
void monitor_resume(Monitor *mon)
@@ -561,15 +583,7 @@ void monitor_resume(Monitor *mon)
}
if (qatomic_dec_fetch(&mon->suspend_cnt) == 0) {
- AioContext *ctx;
-
- if (mon->use_io_thread) {
- ctx = iothread_get_aio_context(mon_iothread);
- } else {
- ctx = qemu_get_aio_context();
- }
-
- aio_bh_schedule_oneshot(ctx, monitor_accept_input, mon);
+ qemu_bh_schedule(mon->accept_input_bh);
}
trace_monitor_suspend(mon, -1);
@@ -610,6 +624,8 @@ static void monitor_iothread_init(void)
void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
bool use_io_thread)
{
+ AioContext *ctx;
+
if (use_io_thread && !mon_iothread) {
monitor_iothread_init();
}
@@ -618,6 +634,13 @@ void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
mon->outbuf = g_string_new(NULL);
mon->skip_flush = skip_flush;
mon->use_io_thread = use_io_thread;
+
+ if (use_io_thread) {
+ ctx = iothread_get_aio_context(mon_iothread);
+ } else {
+ ctx = qemu_get_aio_context();
+ }
+ mon->accept_input_bh = aio_bh_new(ctx, monitor_accept_input, mon);
}
void monitor_data_destroy(Monitor *mon)
@@ -631,6 +654,10 @@ void monitor_data_destroy(Monitor *mon)
readline_free(container_of(mon, MonitorHMP, common)->rs);
}
g_string_free(mon->outbuf, true);
+ if (mon->accept_input_bh) {
+ qemu_bh_delete(mon->accept_input_bh);
+ mon->accept_input_bh = NULL;
+ }
qemu_mutex_destroy(&mon->mon_lock);
}
diff --git a/monitor/qmp.c b/monitor/qmp.c
index bba69a3a40..a56f7240e0 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -26,6 +26,7 @@
#include "chardev/char-io.h"
#include "monitor-internal.h"
+#include "qemu/aio-wait.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-control.h"
#include "qobject/qdict.h"
@@ -71,6 +72,9 @@ typedef struct QMPRequest QMPRequest;
QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
+/* Monitor being serviced by the dispatcher. Protected by BQL. */
+static MonitorQMP *qmp_dispatcher_current_mon;
+
static bool qmp_oob_enabled(MonitorQMP *mon)
{
return mon->capab[QMP_CAPABILITY_OOB];
@@ -98,6 +102,12 @@ static void monitor_qmp_cleanup_req_queue_locked(MonitorQMP *mon)
}
}
+void monitor_qmp_drain_queue(MonitorQMP *mon)
+{
+ QEMU_LOCK_GUARD(&mon->qmp_queue_lock);
+ monitor_qmp_cleanup_req_queue_locked(mon);
+}
+
static void monitor_qmp_cleanup_queue_and_resume(MonitorQMP *mon)
{
QEMU_LOCK_GUARD(&mon->qmp_queue_lock);
@@ -287,6 +297,7 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
*/
mon = req_obj->mon;
+ qmp_dispatcher_current_mon = mon;
/*
* We need to resume the monitor if handle_qmp_command()
@@ -342,11 +353,26 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
qobject_unref(rsp);
}
+ /*
+ * Self-removal: monitor-remove marked this monitor dead.
+ * Close chardev, destroy, skip monitor_resume().
+ */
+ if (mon->common.dead) {
+ qemu_chr_fe_set_handlers(&mon->common.chr, NULL, NULL,
+ NULL, NULL, NULL, NULL, true);
+ qmp_request_free(req_obj);
+ monitor_qmp_destroy(mon);
+ monitor_fdsets_cleanup();
+ qmp_dispatcher_current_mon = NULL;
+ continue;
+ }
+
if (!oob_enabled) {
monitor_resume(&mon->common);
}
qmp_request_free(req_obj);
+ qmp_dispatcher_current_mon = NULL;
}
qatomic_set(&qmp_dispatcher_co, NULL);
}
@@ -499,6 +525,44 @@ void monitor_data_destroy_qmp(MonitorQMP *mon)
g_queue_free(mon->qmp_requests);
}
+static void monitor_qmp_iothread_quiesce(void *opaque)
+{
+ /* No-op: synchronization point only */
+}
+
+/*
+ * Destroy a single QMP monitor.
+ * The monitor must already have been removed from mon_list.
+ */
+void monitor_qmp_destroy(MonitorQMP *mon)
+{
+ qemu_bh_delete(mon->common.accept_input_bh);
+ mon->common.accept_input_bh = NULL;
+
+ WITH_QEMU_LOCK_GUARD(&mon->common.mon_lock) {
+ /* Disable flushes before cancel -- gcontext is already wrong. */
+ mon->common.skip_flush = true;
+ monitor_cancel_out_watch(&mon->common);
+ }
+
+ /* Synchronize with in-flight iothread callbacks. */
+ if (mon->common.use_io_thread) {
+ aio_wait_bh_oneshot(iothread_get_aio_context(mon_iothread),
+ monitor_qmp_iothread_quiesce, NULL);
+ }
+
+ /* Catch requests from a racing monitor_qmp_read(). */
+ monitor_qmp_drain_queue(mon);
+
+ monitor_data_destroy(&mon->common);
+ g_free(mon);
+}
+
+bool monitor_qmp_dispatcher_is_servicing(MonitorQMP *mon)
+{
+ return qmp_dispatcher_current_mon == mon;
+}
+
static void monitor_qmp_setup_handlers_bh(void *opaque)
{
MonitorQMP *mon = opaque;
@@ -510,6 +574,7 @@ 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);
+ qatomic_set(&mon->setup_pending, false);
}
void monitor_init_qmp(Chardev *chr, bool pretty, const char *id,
@@ -557,6 +622,7 @@ void monitor_init_qmp(Chardev *chr, bool pretty, const char *id,
* since chardev might be running in the monitor I/O
* thread. Schedule a bottom half.
*/
+ mon->setup_pending = true;
aio_bh_schedule_oneshot(iothread_get_aio_context(mon_iothread),
monitor_qmp_setup_handlers_bh, mon);
/* Synchronous insert for immediate duplicate detection. */
--
2.47.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 3/5] qapi: add monitor-add, monitor-remove, query-monitors commands
2026-04-09 7:18 [PATCH v4 0/5] monitor: add dynamic QMP monitor hotplug support Christian Brauner
2026-04-09 7:18 ` [PATCH v4 1/5] monitor: store monitor id in Monitor struct Christian Brauner
2026-04-09 7:18 ` [PATCH v4 2/5] monitor/qmp: add infrastructure for safe dynamic monitor removal Christian Brauner
@ 2026-04-09 7:18 ` Christian Brauner
2026-04-09 7:18 ` [PATCH v4 4/5] tests/qtest: add tests for dynamic monitor add/remove Christian Brauner
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Christian Brauner @ 2026-04-09 7:18 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Eric Blake, Fabiano Rosas, Laurent Vivier,
Paolo Bonzini, Thomas Huth, Philippe Mathieu-Daudé,
Daniel P. Berrangé, Christian Brauner
Add QMP commands for monitor lifecycle management:
- monitor-add: Create a QMP monitor on an existing chardev at runtime.
The chardev must exist and not already have a frontend attached
(enforced by qemu_chr_fe_init()). The new monitor starts in
capability negotiation mode.
- monitor-remove: Remove a QMP monitor. The removal sequence is:
1. Mark dead and remove from mon_list under monitor_lock. This
must happen before disconnecting chardev handlers to prevent
event broadcast from calling monitor_flush_locked() after the
gcontext reset, which would create an out_watch on the wrong
GMainContext (see monitor_cancel_out_watch()).
2. Cancel any pending out_watch while gcontext still points to the
correct context.
3. Disconnect chardev handlers. For self-removal, preserve
gcontext by passing the iothread GMainContext and keep the
chardev connection open so the command response can be flushed.
For normal removal, pass context=NULL and close the connection.
4. Drain pending requests from any in-flight monitor_qmp_read().
5. For self-removal, defer destruction to the dispatcher (the
response must be flushed first). Otherwise destroy immediately
via monitor_qmp_destroy().
- query-monitors: Introspect all active monitors with their id, mode,
and chardev name.
The motivating use case is systemd-vmspawn: when an external client
requests raw QMP access, vmspawn can create an independent QMP session
on demand rather than pre-allocating spare monitors at launch or
building an id-rewriting proxy.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
monitor/qmp-cmds-control.c | 105 +++++++++++++++++++++++++++++++++++++++++++++
qapi/control.json | 97 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 202 insertions(+)
diff --git a/monitor/qmp-cmds-control.c b/monitor/qmp-cmds-control.c
index 150ca9f5cb..33d02c8fb6 100644
--- a/monitor/qmp-cmds-control.c
+++ b/monitor/qmp-cmds-control.c
@@ -219,3 +219,108 @@ SchemaInfoList *qmp_query_qmp_schema(Error **errp)
}
return schema;
}
+
+void qmp_monitor_add(const char *id, const char *chardev,
+ bool has_pretty, bool pretty, Error **errp)
+{
+ Chardev *chr;
+
+ /* Reject duplicate monitor id */
+ if (monitor_find_by_id(id)) {
+ error_setg(errp, "monitor '%s' already exists", id);
+ return;
+ }
+
+ chr = qemu_chr_find(chardev);
+ if (!chr) {
+ error_setg(errp, "chardev '%s' not found", chardev);
+ return;
+ }
+
+ monitor_init_qmp(chr, has_pretty && pretty, id, errp);
+}
+
+void qmp_monitor_remove(const char *id, Error **errp)
+{
+ Monitor *mon;
+ MonitorQMP *qmp_mon;
+ bool self_remove;
+
+ mon = monitor_find_by_id(id);
+ if (!mon) {
+ error_setg(errp, "monitor '%s' not found", id);
+ return;
+ }
+
+ if (!mon->is_qmp) {
+ error_setg(errp, "monitor '%s' is not a QMP monitor", id);
+ return;
+ }
+
+ qmp_mon = container_of(mon, MonitorQMP, common);
+
+ if (qatomic_read(&qmp_mon->setup_pending)) {
+ error_setg(errp, "monitor '%s' is still initializing", id);
+ return;
+ }
+
+ self_remove = monitor_qmp_dispatcher_is_servicing(qmp_mon);
+
+ /* Remove from mon_list before chardev disconnect. */
+ WITH_QEMU_LOCK_GUARD(&monitor_lock) {
+ mon->dead = true;
+ QTAILQ_REMOVE(&mon_list, mon, entry);
+ }
+
+ /* Cancel out_watch while gcontext still points to the right ctx. */
+ WITH_QEMU_LOCK_GUARD(&mon->mon_lock) {
+ monitor_cancel_out_watch(mon);
+ }
+
+ /*
+ * Clear chardev handlers. Self-removal preserves gcontext so the
+ * response flush creates out_watch on the correct GMainContext.
+ */
+ if (self_remove) {
+ GMainContext *ctx = mon->use_io_thread
+ ? iothread_get_g_main_context(mon_iothread) : NULL;
+
+ qemu_chr_fe_set_handlers(&mon->chr, NULL, NULL, NULL, NULL,
+ NULL, ctx, false);
+ } else {
+ qemu_chr_fe_set_handlers(&mon->chr, NULL, NULL, NULL, NULL,
+ NULL, NULL, true);
+ }
+
+ /* Drain requests from any in-flight monitor_qmp_read(). */
+ monitor_qmp_drain_queue(qmp_mon);
+
+ /* Self-removal: defer destruction so the response is flushed first. */
+ if (self_remove) {
+ return;
+ }
+
+ monitor_qmp_destroy(qmp_mon);
+ monitor_fdsets_cleanup();
+}
+
+MonitorInfoList *qmp_query_monitors(Error **errp)
+{
+ MonitorInfoList *list = NULL;
+ Monitor *mon;
+
+ WITH_QEMU_LOCK_GUARD(&monitor_lock) {
+ QTAILQ_FOREACH(mon, &mon_list, entry) {
+ MonitorInfo *info = g_new0(MonitorInfo, 1);
+ Chardev *chr = qemu_chr_fe_get_driver(&mon->chr);
+
+ info->id = g_strdup(mon->id);
+ info->mode = mon->is_qmp ? MONITOR_MODE_CONTROL
+ : MONITOR_MODE_READLINE;
+ info->chardev = g_strdup(chr ? chr->label : "unknown");
+ QAPI_LIST_PREPEND(list, info);
+ }
+ }
+
+ return list;
+}
diff --git a/qapi/control.json b/qapi/control.json
index 9a5302193d..eb4a30b59e 100644
--- a/qapi/control.json
+++ b/qapi/control.json
@@ -211,3 +211,100 @@
'*pretty': 'bool',
'chardev': 'str'
} }
+
+##
+# @monitor-add:
+#
+# Add a QMP monitor on an existing character device backend.
+#
+# The chardev must already exist (created via chardev-add or CLI).
+# The monitor begins in capability negotiation mode -- the first
+# client to connect receives the QMP greeting.
+#
+# @id: Monitor identifier, must be unique among monitors
+#
+# @chardev: Name of the character device backend to attach to
+#
+# @pretty: Enable pretty-printing of QMP responses (default: false)
+#
+# Errors:
+# - If @id is already in use
+# - If @chardev does not exist
+#
+# Since: 11.1
+#
+# .. qmp-example::
+#
+# -> { "execute": "monitor-add",
+# "arguments": { "id": "extra-qmp",
+# "chardev": "qmp-extra" } }
+# <- { "return": {} }
+##
+{ 'command': 'monitor-add',
+ 'data': { 'id': 'str',
+ 'chardev': 'str',
+ '*pretty': 'bool' } }
+
+##
+# @monitor-remove:
+#
+# Remove a QMP monitor.
+#
+# The underlying chardev is NOT removed -- use chardev-remove
+# separately if desired.
+#
+# If a client is currently connected, the connection is dropped.
+#
+# @id: Monitor identifier
+#
+# Errors:
+# - If @id does not exist
+# - If the monitor is not a QMP monitor
+#
+# Since: 11.1
+#
+# .. qmp-example::
+#
+# -> { "execute": "monitor-remove",
+# "arguments": { "id": "extra-qmp" } }
+# <- { "return": {} }
+##
+{ 'command': 'monitor-remove',
+ 'data': { 'id': 'str' } }
+
+##
+# @MonitorInfo:
+#
+# Information about a QMP/HMP monitor.
+#
+# @id: Monitor identifier. Always present for QMP monitors (auto-
+# generated if not explicitly set). Absent for HMP monitors.
+#
+# @mode: Monitor mode (readline or control)
+#
+# @chardev: Name of the attached character device
+#
+# Since: 11.1
+##
+{ 'struct': 'MonitorInfo',
+ 'data': { '*id': 'str',
+ 'mode': 'MonitorMode',
+ 'chardev': 'str' } }
+
+##
+# @query-monitors:
+#
+# Return information about all active monitors.
+#
+# Returns: a list of @MonitorInfo for each active monitor
+#
+# Since: 11.1
+#
+# .. qmp-example::
+#
+# -> { "execute": "query-monitors" }
+# <- { "return": [ { "id": "mon0", "mode": "control",
+# "chardev": "compat_monitor0" } ] }
+##
+{ 'command': 'query-monitors',
+ 'returns': ['MonitorInfo'] }
--
2.47.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 4/5] tests/qtest: add tests for dynamic monitor add/remove
2026-04-09 7:18 [PATCH v4 0/5] monitor: add dynamic QMP monitor hotplug support Christian Brauner
` (2 preceding siblings ...)
2026-04-09 7:18 ` [PATCH v4 3/5] qapi: add monitor-add, monitor-remove, query-monitors commands Christian Brauner
@ 2026-04-09 7:18 ` Christian Brauner
2026-04-09 13:01 ` Fabiano Rosas
2026-04-09 7:18 ` [PATCH v4 5/5] tests/functional: add e2e test for dynamic QMP monitor hotplug Christian Brauner
2026-04-09 19:43 ` [PATCH v4 0/5] monitor: add dynamic QMP monitor hotplug support Daniel P. Berrangé
5 siblings, 1 reply; 10+ messages in thread
From: Christian Brauner @ 2026-04-09 7:18 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Eric Blake, Fabiano Rosas, Laurent Vivier,
Paolo Bonzini, Thomas Huth, Philippe Mathieu-Daudé,
Daniel P. Berrangé, Christian Brauner
Test the monitor-add, monitor-remove, and query-monitors QMP commands:
- Basic lifecycle: chardev-add -> monitor-add -> query-monitors ->
monitor-remove -> chardev-remove
- Error: duplicate monitor id
- Error: monitor-remove on nonexistent id
- Error: monitor-add with nonexistent chardev
- Error: second monitor on same chardev (chardev already in use)
- Removal of CLI-created QMP monitor succeeds
- Error: monitor-remove on HMP monitor
- Re-add after remove: same id and chardev reusable after removal
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
tests/qtest/qmp-test.c | 193 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 193 insertions(+)
diff --git a/tests/qtest/qmp-test.c b/tests/qtest/qmp-test.c
index edf0886787..3ff6de3626 100644
--- a/tests/qtest/qmp-test.c
+++ b/tests/qtest/qmp-test.c
@@ -337,6 +337,193 @@ static void test_qmp_missing_any_arg(void)
qtest_quit(qts);
}
+static void test_qmp_monitor_add_remove(void)
+{
+ QTestState *qts;
+ QDict *resp;
+
+ qts = qtest_init(common_args);
+
+ /* Create a null chardev for the dynamic monitor */
+ resp = qtest_qmp(qts,
+ "{'execute': 'chardev-add',"
+ " 'arguments': {'id': 'monitor-chardev',"
+ " 'backend': {'type': 'null',"
+ " 'data': {}}}}");
+ g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
+
+ /* Add a dynamic monitor */
+ resp = qtest_qmp(qts,
+ "{'execute': 'monitor-add',"
+ " 'arguments': {'id': 'dyn-mon',"
+ " 'chardev': 'monitor-chardev'}}");
+ g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
+
+ /* Verify it shows up in query-monitors */
+ resp = qtest_qmp(qts, "{'execute': 'query-monitors'}");
+ g_assert(!qdict_haskey(resp, "error"));
+ qobject_unref(resp);
+
+ /* Error: duplicate id */
+ resp = qtest_qmp(qts,
+ "{'execute': 'monitor-add',"
+ " 'arguments': {'id': 'dyn-mon',"
+ " 'chardev': 'monitor-chardev'}}");
+ qmp_expect_error_and_unref(resp, "GenericError");
+
+ /* Remove the dynamic monitor */
+ resp = qtest_qmp(qts,
+ "{'execute': 'monitor-remove',"
+ " 'arguments': {'id': 'dyn-mon'}}");
+ g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
+
+ /* Error: remove nonexistent */
+ resp = qtest_qmp(qts,
+ "{'execute': 'monitor-remove',"
+ " 'arguments': {'id': 'dyn-mon'}}");
+ qmp_expect_error_and_unref(resp, "GenericError");
+
+ /* Add again after remove -- same id and chardev should work */
+ resp = qtest_qmp(qts,
+ "{'execute': 'monitor-add',"
+ " 'arguments': {'id': 'dyn-mon',"
+ " 'chardev': 'monitor-chardev'}}");
+ g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
+
+ /* Clean up */
+ resp = qtest_qmp(qts,
+ "{'execute': 'monitor-remove',"
+ " 'arguments': {'id': 'dyn-mon'}}");
+ g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
+
+ resp = qtest_qmp(qts,
+ "{'execute': 'chardev-remove',"
+ " 'arguments': {'id': 'monitor-chardev'}}");
+ g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
+
+ qtest_quit(qts);
+}
+
+static void test_qmp_monitor_error_paths(void)
+{
+ QTestState *qts;
+ QDict *resp;
+
+ qts = qtest_init(common_args);
+
+ /* Error: chardev does not exist */
+ resp = qtest_qmp(qts,
+ "{'execute': 'monitor-add',"
+ " 'arguments': {'id': 'bad-mon',"
+ " 'chardev': 'nonexistent'}}");
+ qmp_expect_error_and_unref(resp, "GenericError");
+
+ /* Error: remove nonexistent monitor */
+ resp = qtest_qmp(qts,
+ "{'execute': 'monitor-remove',"
+ " 'arguments': {'id': 'bogus'}}");
+ qmp_expect_error_and_unref(resp, "GenericError");
+
+ qtest_quit(qts);
+}
+
+static void test_qmp_monitor_chardev_in_use(void)
+{
+ QTestState *qts;
+ QDict *resp;
+
+ qts = qtest_init(common_args);
+
+ /* Create a null chardev */
+ resp = qtest_qmp(qts,
+ "{'execute': 'chardev-add',"
+ " 'arguments': {'id': 'shared-chr',"
+ " 'backend': {'type': 'null',"
+ " 'data': {}}}}");
+ g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
+
+ /* Attach first monitor */
+ resp = qtest_qmp(qts,
+ "{'execute': 'monitor-add',"
+ " 'arguments': {'id': 'mon-1',"
+ " 'chardev': 'shared-chr'}}");
+ g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
+
+ /* Error: second monitor on the same chardev */
+ resp = qtest_qmp(qts,
+ "{'execute': 'monitor-add',"
+ " 'arguments': {'id': 'mon-2',"
+ " 'chardev': 'shared-chr'}}");
+ qmp_expect_error_and_unref(resp, "GenericError");
+
+ /* Clean up */
+ resp = qtest_qmp(qts,
+ "{'execute': 'monitor-remove',"
+ " 'arguments': {'id': 'mon-1'}}");
+ g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
+
+ resp = qtest_qmp(qts,
+ "{'execute': 'chardev-remove',"
+ " 'arguments': {'id': 'shared-chr'}}");
+ g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
+
+ qtest_quit(qts);
+}
+
+static void test_qmp_monitor_remove_cli(void)
+{
+ QTestState *qts;
+ QDict *resp;
+
+ /* Launch with a named CLI monitor on a null chardev */
+ qts = qtest_initf("%s -chardev null,id=cli-chr"
+ " -mon id=cli-mon,chardev=cli-chr,mode=control",
+ common_args);
+
+ /* CLI-created QMP monitors can be removed */
+ resp = qtest_qmp(qts,
+ "{'execute': 'monitor-remove',"
+ " 'arguments': {'id': 'cli-mon'}}");
+ g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
+
+ resp = qtest_qmp(qts,
+ "{'execute': 'chardev-remove',"
+ " 'arguments': {'id': 'cli-chr'}}");
+ g_assert(qdict_haskey(resp, "return"));
+ qobject_unref(resp);
+
+ qtest_quit(qts);
+}
+
+static void test_qmp_monitor_remove_hmp(void)
+{
+ QTestState *qts;
+ QDict *resp;
+
+ qts = qtest_initf("%s -chardev null,id=hmp-chr"
+ " -mon id=hmp-mon,chardev=hmp-chr,mode=readline",
+ common_args);
+
+ /* Error: monitor-remove must reject HMP monitors */
+ resp = qtest_qmp(qts,
+ "{'execute': 'monitor-remove',"
+ " 'arguments': {'id': 'hmp-mon'}}");
+ qmp_expect_error_and_unref(resp, "GenericError");
+
+ qtest_quit(qts);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -348,6 +535,12 @@ int main(int argc, char *argv[])
#endif
qtest_add_func("qmp/preconfig", test_qmp_preconfig);
qtest_add_func("qmp/missing-any-arg", test_qmp_missing_any_arg);
+ qtest_add_func("qmp/monitor-add-remove", test_qmp_monitor_add_remove);
+ qtest_add_func("qmp/monitor-error-paths", test_qmp_monitor_error_paths);
+ qtest_add_func("qmp/monitor-chardev-in-use",
+ test_qmp_monitor_chardev_in_use);
+ qtest_add_func("qmp/monitor-remove-cli", test_qmp_monitor_remove_cli);
+ qtest_add_func("qmp/monitor-remove-hmp", test_qmp_monitor_remove_hmp);
return g_test_run();
}
--
2.47.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v4 4/5] tests/qtest: add tests for dynamic monitor add/remove
2026-04-09 7:18 ` [PATCH v4 4/5] tests/qtest: add tests for dynamic monitor add/remove Christian Brauner
@ 2026-04-09 13:01 ` Fabiano Rosas
0 siblings, 0 replies; 10+ messages in thread
From: Fabiano Rosas @ 2026-04-09 13:01 UTC (permalink / raw)
To: Christian Brauner, qemu-devel
Cc: Markus Armbruster, Eric Blake, Laurent Vivier, Paolo Bonzini,
Thomas Huth, Philippe Mathieu-Daudé, Daniel P. Berrangé,
Christian Brauner
Christian Brauner <brauner@kernel.org> writes:
> Test the monitor-add, monitor-remove, and query-monitors QMP commands:
>
> - Basic lifecycle: chardev-add -> monitor-add -> query-monitors ->
> monitor-remove -> chardev-remove
> - Error: duplicate monitor id
> - Error: monitor-remove on nonexistent id
> - Error: monitor-add with nonexistent chardev
> - Error: second monitor on same chardev (chardev already in use)
> - Removal of CLI-created QMP monitor succeeds
> - Error: monitor-remove on HMP monitor
> - Re-add after remove: same id and chardev reusable after removal
>
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 5/5] tests/functional: add e2e test for dynamic QMP monitor hotplug
2026-04-09 7:18 [PATCH v4 0/5] monitor: add dynamic QMP monitor hotplug support Christian Brauner
` (3 preceding siblings ...)
2026-04-09 7:18 ` [PATCH v4 4/5] tests/qtest: add tests for dynamic monitor add/remove Christian Brauner
@ 2026-04-09 7:18 ` Christian Brauner
2026-04-09 19:43 ` [PATCH v4 0/5] monitor: add dynamic QMP monitor hotplug support Daniel P. Berrangé
5 siblings, 0 replies; 10+ messages in thread
From: Christian Brauner @ 2026-04-09 7:18 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Eric Blake, Fabiano Rosas, Laurent Vivier,
Paolo Bonzini, Thomas Huth, Philippe Mathieu-Daudé,
Daniel P. Berrangé, Christian Brauner
Add functional tests that exercise dynamic monitor hotplug with real
socket connections:
- Hotplug cycle: chardev-add a unix socket, monitor-add, connect to the
socket, receive the QMP greeting, negotiate capabilities, send
query-version, disconnect, remove the monitor and chardev, then repeat
the entire cycle a second time to verify cleanup and reuse.
- Self-removal: a dynamically-added monitor sends monitor-remove
targeting itself, verifying that the response is delivered before
the connection drops and that the monitor is gone afterwards.
- Large response: send query-qmp-schema on a dynamic monitor to
exercise the output buffer flush path with a large response payload.
- Events after negotiation: trigger STOP/RESUME events via the main
monitor and verify they are delivered on the dynamic monitor.
This complements the qtest unit tests by verifying that a real QMP
client can connect to a dynamically-added monitor and exchange messages.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
tests/functional/generic/meson.build | 1 +
tests/functional/generic/test_monitor_hotplug.py | 170 +++++++++++++++++++++++
2 files changed, 171 insertions(+)
diff --git a/tests/functional/generic/meson.build b/tests/functional/generic/meson.build
index 09763c5d22..c94105c62e 100644
--- a/tests/functional/generic/meson.build
+++ b/tests/functional/generic/meson.build
@@ -4,6 +4,7 @@ tests_generic_system = [
'empty_cpu_model',
'info_usernet',
'linters',
+ 'monitor_hotplug',
'version',
'vnc',
]
diff --git a/tests/functional/generic/test_monitor_hotplug.py b/tests/functional/generic/test_monitor_hotplug.py
new file mode 100644
index 0000000000..f81236e7b4
--- /dev/null
+++ b/tests/functional/generic/test_monitor_hotplug.py
@@ -0,0 +1,170 @@
+#!/usr/bin/env python3
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+# Functional test for dynamic QMP monitor hotplug
+#
+# Copyright (c) 2026 Christian Brauner
+
+import os
+
+from qemu_test import QemuSystemTest
+
+from qemu.qmp.legacy import QEMUMonitorProtocol
+
+
+class MonitorHotplug(QemuSystemTest):
+
+ def setUp(self):
+ super().setUp()
+ sock_dir = self.socket_dir()
+ self._sock_path = os.path.join(sock_dir.name, 'hotplug.sock')
+
+ def _add_monitor(self):
+ """Create a chardev + monitor and return the socket path."""
+ sock = self._sock_path
+ self.vm.cmd('chardev-add', id='hotplug-chr', backend={
+ 'type': 'socket',
+ 'data': {
+ 'addr': {
+ 'type': 'unix',
+ 'data': {'path': sock}
+ },
+ 'server': True,
+ 'wait': False,
+ }
+ })
+ self.vm.cmd('monitor-add', id='hotplug-mon',
+ chardev='hotplug-chr')
+ return sock
+
+ def _remove_monitor(self):
+ """Remove the monitor + chardev."""
+ self.vm.cmd('monitor-remove', id='hotplug-mon')
+ self.vm.cmd('chardev-remove', id='hotplug-chr')
+
+ def _connect_and_handshake(self, sock_path):
+ """
+ Connect to the dynamic monitor socket, perform the QMP
+ greeting and capability negotiation, send a command, then
+ disconnect.
+ """
+ qmp = QEMUMonitorProtocol(sock_path)
+
+ # connect(negotiate=True) receives the greeting, validates it,
+ # and sends qmp_capabilities automatically.
+ greeting = qmp.connect(negotiate=True)
+ self.assertIn('QMP', greeting)
+ self.assertIn('version', greeting['QMP'])
+ self.assertIn('capabilities', greeting['QMP'])
+
+ # Send a real command to prove the session is fully functional
+ resp = qmp.cmd_obj({'execute': 'query-version'})
+ self.assertIn('return', resp)
+ self.assertIn('qemu', resp['return'])
+
+ qmp.close()
+
+ def test_hotplug_cycle(self):
+ """
+ Hotplug a monitor, do the full QMP handshake, unplug it,
+ then repeat the whole cycle a second time.
+ """
+ self.set_machine('none')
+ self.vm.add_args('-nodefaults')
+ self.vm.launch()
+
+ # First cycle
+ sock = self._add_monitor()
+ self._connect_and_handshake(sock)
+ self._remove_monitor()
+
+ # Second cycle -- same ids, same path, must work
+ sock = self._add_monitor()
+ self._connect_and_handshake(sock)
+ self._remove_monitor()
+
+ def test_self_removal(self):
+ """
+ A dynamically-added monitor sends monitor-remove targeting
+ itself. Verify the response is delivered before the
+ connection drops, and that the monitor is gone afterwards.
+ """
+ self.set_machine('none')
+ self.vm.add_args('-nodefaults')
+ self.vm.launch()
+
+ sock = self._add_monitor()
+
+ qmp = QEMUMonitorProtocol(sock)
+ greeting = qmp.connect(negotiate=True)
+ self.assertIn('QMP', greeting)
+
+ # Self-removal: the dynamic monitor removes itself
+ resp = qmp.cmd_obj({'execute': 'monitor-remove',
+ 'arguments': {'id': 'hotplug-mon'}})
+ self.assertIn('return', resp)
+
+ qmp.close()
+
+ # The main monitor should no longer list the removed monitor
+ monitors = self.vm.cmd('query-monitors')
+ for m in monitors:
+ self.assertNotEqual(m.get('id'), 'hotplug-mon')
+
+ # Clean up the chardev
+ self.vm.cmd('chardev-remove', id='hotplug-chr')
+
+ def test_large_response(self):
+ """
+ Send a command with a large response (query-qmp-schema) on a
+ dynamically-added monitor to exercise the output buffer flush
+ path.
+ """
+ self.set_machine('none')
+ self.vm.add_args('-nodefaults')
+ self.vm.launch()
+
+ sock = self._add_monitor()
+
+ qmp = QEMUMonitorProtocol(sock)
+ qmp.connect(negotiate=True)
+
+ resp = qmp.cmd_obj({'execute': 'query-qmp-schema'})
+ self.assertIn('return', resp)
+ self.assertIsInstance(resp['return'], list)
+ self.assertGreater(len(resp['return']), 0)
+
+ qmp.close()
+ self._remove_monitor()
+
+ def test_events_after_negotiation(self):
+ """
+ Verify that QMP events are delivered on a dynamically-added
+ monitor after capability negotiation completes.
+ """
+ self.set_machine('none')
+ self.vm.add_args('-nodefaults')
+ self.vm.launch()
+
+ sock = self._add_monitor()
+
+ qmp = QEMUMonitorProtocol(sock)
+ qmp.connect(negotiate=True)
+
+ # Trigger a STOP event via the main monitor, then read it
+ # from the dynamic monitor.
+ self.vm.cmd('stop')
+ resp = qmp.pull_event(wait=True)
+ self.assertEqual(resp['event'], 'STOP')
+
+ self.vm.cmd('cont')
+ resp = qmp.pull_event(wait=True)
+ self.assertEqual(resp['event'], 'RESUME')
+
+ qmp.close()
+ self._remove_monitor()
+
+
+if __name__ == '__main__':
+ QemuSystemTest.main()
--
2.47.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v4 0/5] monitor: add dynamic QMP monitor hotplug support
2026-04-09 7:18 [PATCH v4 0/5] monitor: add dynamic QMP monitor hotplug support Christian Brauner
` (4 preceding siblings ...)
2026-04-09 7:18 ` [PATCH v4 5/5] tests/functional: add e2e test for dynamic QMP monitor hotplug Christian Brauner
@ 2026-04-09 19:43 ` Daniel P. Berrangé
2026-04-10 7:36 ` Christian Brauner
5 siblings, 1 reply; 10+ messages in thread
From: Daniel P. Berrangé @ 2026-04-09 19:43 UTC (permalink / raw)
To: Christian Brauner
Cc: qemu-devel, Markus Armbruster, Eric Blake, Fabiano Rosas,
Laurent Vivier, Paolo Bonzini, Thomas Huth,
Philippe Mathieu-Daudé
On Thu, Apr 09, 2026 at 09:18:17AM +0200, Christian Brauner wrote:
> QEMU supports runtime hotplug for chardevs, devices, block backends,
> and netdevs. Monitors are the only major subsystem that lacks this --
> all QMP monitors must be configured at launch via -mon or -qmp CLI
> options.
>
> This series adds monitor-add, monitor-remove, and query-monitors QMP
> commands so that management tools can create independent QMP sessions
> on demand at runtime.
There's nothing inherently wrong with your proposal. It is following
the standard historical design practice we've taken for enabling
hotplug/unplug of all the other backend types. Conceptually though
our historical practice is more of an accident of evolution rather
than an ideal state.
By this I mean we have a general purpose object framework
internally and commands '-object', and 'object_add' / 'object_del'
for hotplug/unplug in QMP and HMP. Conceptually these could handle
all of our objects, devices, netdev chardevs, monitors, etc, etc,
removing the need for the specialized add/remove QMP commands for
each backend type, and the specialized CLI argsf.
The blocker has been lack of time to convert everything/anything,
since while the conversions are not especially difficult they are
certainly time consuming due to the sheer number of types that
exist in many cases.
I'm thinking the monitor, however, is a decent opportunity to
"do the right thing" from a design POV. We only have three classes
needed in QOM, the monitor generic base class, a HMP subclass and
a QMP subclass.
If we convert QMP/HMP into QOM objects, we could then use the
pre-existing object_add/object_del commands in both HMP and QMP.
I'm not asking you to do that conversion work though. I've
been doing some experimentation myself today to test the
viability of this idea and it looks promising.
Given we've got an entire 4 month dev cycle before these
proposed monitor-add/remove commands could get into a QEMU
release, I think it is worth trialling the QOM conversion
for a short while.
> I've implemented a QMP-to-Varlink bridge in systemd. This allows
> systemd-vmspawn to control virtual machines and containers through a
> unified Varlink interface. Varlink allows protocol upgrades. For
> example, it is possible to switch from Varlink to say http. I'm
> allowing Varlink clients to upgrade from the Varlink interface to the
> native QMP interface. Such clients get a new monitor assigned that
> allows them to manage the virtual machine directly via QMP. The main
> monitor remains unaffected and tied to the generic Varlink interface. We
> can't pre-allocate monitors as we have no control over how many protocol
> upgrades we actually get but it won't just be one. And having unused
> monitors around really isn't ideal either.
>
> Having the ability to hotplug monitors would really be helpful. I'm not
> yet super well-versed in qemu internals so this might be done wrong but
> testing works so far.
>
> My systemd patch that triggered this is at
> https://github.com/systemd/systemd/pull/41449.
>
> The usage pattern mirrors chardev hotplug:
>
> -> chardev-add id=qmp-extra backend=socket,...
> -> monitor-add id=extra-qmp chardev=qmp-extra
> [client connects to socket, gets QMP greeting, negotiates, sends commands]
> -> monitor-remove id=extra-qmp
> -> chardev-remove id=qmp-extra
With my idea the usage pattern would be basically the same
flow:
-> chardev-add id=qmp-extra backend=socket,...
-> object-add qom-type=monitor-qmp id=extra-qmp chardev=qmp-extra
[client connects to socket, gets QMP greeting, negotiates, sends commands]
-> object-del id=extra-qmp
-> chardev-remove id=qmp-extra
> Patches 1-2 add the data model (id field in Monitor) and the
> infrastructure for safe per-monitor destruction without shutting down
> the shared dispatcher coroutine.
>
> Patch 3 adds the QAPI schema and command handlers.
>
> Patches 4-5 add qtest unit tests and a functional e2e test that
> performs a full hotplug -> connect -> handshake -> unplug cycle.
Patches 2, 4 and 5 would still be desirable even with the QOM
conversion I'm thinking about.
Patch 1 would be redundant since QOM would give us an ID facility
as standard.
Patch 3 would be redundant since we'd use object_add/object_del
instead.
>
> > meson test "qtest-x86_64/qmp-test" "func-x86_64-monitor_hotplug" -v
> ninja: Entering directory `/home/brauner/src/git/qemu/build'
> [45/45] Linking target qemu-img
> 1/2 qemu:func-quick+func-x86_64 / func-x86_64-monitor_hotplug RUNNING
> >>> QEMU_TEST_QEMU_IMG=/home/brauner/src/git/qemu/build/qemu-img ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 MESON_BUILD_ROOT=/home/brauner/src/git/qemu/build LD_LIBRARY_PATH=/home/brauner/src/git/qemu/build/tests/tcg/plugins:/home/brauner/src/git/qemu/build/contrib/plugins:/home/brauner/src/go/deps/raft/.libs/:/home/brauner/src/go/deps/cowsql/.libs/ MALLOC_PERTURB_=165 PYTHONPATH=/home/brauner/src/git/qemu/python:/home/brauner/src/git/qemu/tests/functional QEMU_TEST_GDB=/usr/bin/gdb QEMU_TEST_QEMU_BINARY=/home/brauner/src/git/qemu/build/qemu-system-x86_64 MESON_TEST_ITERATION=1 UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 RUST_BACKTRACE=1 /home/brauner/src/git/qemu/build/pyvenv/bin/python3 /home/brauner/src/git/qemu/tests/functional/generic/test_monitor_hotplug.py
> 2/2 qemu:qtest+qtest-x86_64 / qtest-x86_64/qmp-test RUNNING
> >>> ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MALLOC_PERTURB_=244 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 PYTHON=/home/brauner/src/git/qemu/build/pyvenv/bin/python3 QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon QTEST_QEMU_IMG=./qemu-img G_TEST_DBUS_DAEMON=/home/brauner/src/git/qemu/tests/dbus-vmstate-daemon.sh QTEST_QEMU_BINARY=./qemu-system-x86_64 MESON_TEST_ITERATION=1 UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 RUST_BACKTRACE=1 /home/brauner/src/git/qemu/build/tests/qtest/qmp-test --tap -k
> ▶ 1/2 test_monitor_hotplug.MonitorHotplug.test_events_after_negotiation OK
> ▶ 2/2 /x86_64/qmp/protocol OK
> ▶ 1/2 test_monitor_hotplug.MonitorHotplug.test_hotplug_cycle OK
> ▶ 1/2 test_monitor_hotplug.MonitorHotplug.test_large_response OK
> ▶ 2/2 /x86_64/qmp/oob OK
> ▶ 1/2 test_monitor_hotplug.MonitorHotplug.test_self_removal OK
> 1/2 qemu:func-quick+func-x86_64 / func-x86_64-monitor_hotplug OK 0.30s 4 subtests passed
>
> ▶ 2/2 /x86_64/qmp/preconfig OK
> ▶ 2/2 /x86_64/qmp/missing-any-arg OK
> ▶ 2/2 /x86_64/qmp/monitor-add-remove OK
> ▶ 2/2 /x86_64/qmp/monitor-error-paths OK
> ▶ 2/2 /x86_64/qmp/monitor-chardev-in-use OK
> ▶ 2/2 /x86_64/qmp/monitor-remove-cli OK
> ▶ 2/2 /x86_64/qmp/monitor-remove-hmp OK
> 2/2 qemu:qtest+qtest-x86_64 / qtest-x86_64/qmp-test OK 1.09s 9 subtests passed
>
> Ok: 2
> Expected Fail: 0
> Fail: 0
> Unexpected Pass: 0
> Skipped: 0
> Timeout: 0
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
> Changes in v4:
> - Move 'dead' field from patch 1 to patch 2 where it is first used.
> - Allow removal of any QMP monitor, drop the 'dynamic' gate in
> qmp_monitor_remove(). Drop the 'dynamic' field from struct Monitor
> and from MonitorInfo entirely since it no longer serves a purpose.
> - Auto-generate monitor ids ("mon0", "mon1", ...) for QMP monitors
> created via CLI without an explicit id, so every QMP monitor is
> addressable by monitor-remove and always appears with an id in
> query-monitors output.
> - Change Since: 11.0 to Since: 11.1 throughout the QAPI schema.
> - Drop "GenericError" from QAPI error descriptions.
> - Update monitor-remove QAPI doc to reflect that any QMP monitor can
> be removed, not just dynamically added ones.
> - Update qtest: test_qmp_monitor_remove_cli now expects success
> instead of error.
> - Link to v3: https://patch.msgid.link/20260407-work-qmp-monitor-hotplug-v3-0-cb259800fffb@kernel.org
>
> Changes in v3:
> - Use SPDX license identifier in functional test.
> - Use framework's socket_dir() helper instead of manual tempfile
> handling for socket paths in the functional test, drop tearDown().
> - Tighten struct field comments in monitor-internal.h.
> - Wrap long qtest_add_func() registration line.
> - Link to v2: https://patch.msgid.link/20260405-work-qmp-monitor-hotplug-v2-0-ad5bedd2917a@kernel.org
>
> Changes in v2:
> - Fix use-after-free in self-removal path: skip monitor_resume() when
> the monitor is dead to avoid scheduling a BH against a monitor that
> is about to be freed by monitor_qmp_destroy().
> - Hold monitor_lock in monitor_find_by_id() to prevent races with
> the I/O thread BH that appends to mon_list.
> - Deduplicate monitor-remove commit message: trim the gcontext/out_watch
> explanation that repeated the infrastructure commit, reference
> monitor_cancel_out_watch() instead.
> - Add missing test descriptions to patch 4 (chardev-in-use, CLI monitor
> rejection, HMP monitor rejection) and patch 5 (self-removal, large
> response, events).
> - Fix cover letter wording.
> - Link to v1: https://patch.msgid.link/20260402-work-qmp-monitor-hotplug-v1-0-6313a5cdd574@kernel.org
>
> ---
> Christian Brauner (5):
> monitor: store monitor id in Monitor struct
> monitor/qmp: add infrastructure for safe dynamic monitor removal
> qapi: add monitor-add, monitor-remove, query-monitors commands
> tests/qtest: add tests for dynamic monitor add/remove
> tests/functional: add e2e test for dynamic QMP monitor hotplug
>
> include/monitor/monitor.h | 3 +-
> monitor/monitor-internal.h | 10 ++
> monitor/monitor.c | 70 ++++++--
> monitor/qmp-cmds-control.c | 105 ++++++++++++
> monitor/qmp.c | 81 +++++++++-
> qapi/control.json | 97 ++++++++++++
> tests/functional/generic/meson.build | 1 +
> tests/functional/generic/test_monitor_hotplug.py | 170 ++++++++++++++++++++
> tests/qtest/qmp-test.c | 193 +++++++++++++++++++++++
> 9 files changed, 714 insertions(+), 16 deletions(-)
> ---
> base-commit: 6d3e9dddefdd2e8e6a4942ba9399df0e47df21ed
> change-id: 20260402-work-qmp-monitor-hotplug-fba7c618e3db
>
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] 10+ messages in thread* Re: [PATCH v4 0/5] monitor: add dynamic QMP monitor hotplug support
2026-04-09 19:43 ` [PATCH v4 0/5] monitor: add dynamic QMP monitor hotplug support Daniel P. Berrangé
@ 2026-04-10 7:36 ` Christian Brauner
2026-04-10 16:25 ` Daniel P. Berrangé
0 siblings, 1 reply; 10+ messages in thread
From: Christian Brauner @ 2026-04-10 7:36 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Markus Armbruster, Eric Blake, Fabiano Rosas,
Laurent Vivier, Paolo Bonzini, Thomas Huth,
Philippe Mathieu-Daudé
On Thu, Apr 09, 2026 at 08:43:40PM +0100, Daniel P. Berrangé wrote:
> On Thu, Apr 09, 2026 at 09:18:17AM +0200, Christian Brauner wrote:
> > QEMU supports runtime hotplug for chardevs, devices, block backends,
> > and netdevs. Monitors are the only major subsystem that lacks this --
> > all QMP monitors must be configured at launch via -mon or -qmp CLI
> > options.
> >
> > This series adds monitor-add, monitor-remove, and query-monitors QMP
> > commands so that management tools can create independent QMP sessions
> > on demand at runtime.
>
> There's nothing inherently wrong with your proposal. It is following
> the standard historical design practice we've taken for enabling
> hotplug/unplug of all the other backend types. Conceptually though
> our historical practice is more of an accident of evolution rather
> than an ideal state.
>
> By this I mean we have a general purpose object framework
> internally and commands '-object', and 'object_add' / 'object_del'
> for hotplug/unplug in QMP and HMP. Conceptually these could handle
> all of our objects, devices, netdev chardevs, monitors, etc, etc,
> removing the need for the specialized add/remove QMP commands for
> each backend type, and the specialized CLI argsf.
If you actually plan on converting old types you're now left with two
ways of adding objects. And if you plan on ultimately deprecating the
old format it's going to be very annoying to fade those out.
Or you're not going to convert and then you have blockdev-add and then
the new object pattern.
I'm not sure that I see the value in that. Imho, that's just pain for
new users. Just providing my opinion here.
> The blocker has been lack of time to convert everything/anything,
> since while the conversions are not especially difficult they are
> certainly time consuming due to the sheer number of types that
> exist in many cases.
>
>
> I'm thinking the monitor, however, is a decent opportunity to
> "do the right thing" from a design POV. We only have three classes
> needed in QOM, the monitor generic base class, a HMP subclass and
> a QMP subclass.
>
> If we convert QMP/HMP into QOM objects, we could then use the
> pre-existing object_add/object_del commands in both HMP and QMP.
>
> I'm not asking you to do that conversion work though. I've
> been doing some experimentation myself today to test the
> viability of this idea and it looks promising.
How difficult is this to do? I've had bad experiences with projects
promising to do it all very differently and taking the patch away from
me and then not following through or like 2 years later or the feature
just vanished into nothing. I'm not saying this is the case here but I'm
a bit weary of not having any way to move this forward myself other than
"any news on this?" mails. It's frustrating for both sides.
So if this is doable just give me what you got and let me see whether I
can take it over the finish line.
Also, I really dislike spending a lot of time understanding something
and it all working and then being told "actually, let me do this". I
avoid this like the plague doing this to developers in the kernel.
> Given we've got an entire 4 month dev cycle before these
> proposed monitor-add/remove commands could get into a QEMU
> release, I think it is worth trialling the QOM conversion
> for a short while.
I'm not sure what your governance model is so I'm just going to ask: Is
this just a review or is this a project level decision?
> > I've implemented a QMP-to-Varlink bridge in systemd. This allows
> > systemd-vmspawn to control virtual machines and containers through a
> > unified Varlink interface. Varlink allows protocol upgrades. For
> > example, it is possible to switch from Varlink to say http. I'm
> > allowing Varlink clients to upgrade from the Varlink interface to the
> > native QMP interface. Such clients get a new monitor assigned that
> > allows them to manage the virtual machine directly via QMP. The main
> > monitor remains unaffected and tied to the generic Varlink interface. We
> > can't pre-allocate monitors as we have no control over how many protocol
> > upgrades we actually get but it won't just be one. And having unused
> > monitors around really isn't ideal either.
> >
> > Having the ability to hotplug monitors would really be helpful. I'm not
> > yet super well-versed in qemu internals so this might be done wrong but
> > testing works so far.
> >
> > My systemd patch that triggered this is at
> > https://github.com/systemd/systemd/pull/41449.
> >
> > The usage pattern mirrors chardev hotplug:
> >
> > -> chardev-add id=qmp-extra backend=socket,...
> > -> monitor-add id=extra-qmp chardev=qmp-extra
> > [client connects to socket, gets QMP greeting, negotiates, sends commands]
> > -> monitor-remove id=extra-qmp
> > -> chardev-remove id=qmp-extra
>
> With my idea the usage pattern would be basically the same
> flow:
>
> -> chardev-add id=qmp-extra backend=socket,...
> -> object-add qom-type=monitor-qmp id=extra-qmp chardev=qmp-extra
> [client connects to socket, gets QMP greeting, negotiates, sends commands]
> -> object-del id=extra-qmp
> -> chardev-remove id=qmp-extra
>
> > Patches 1-2 add the data model (id field in Monitor) and the
> > infrastructure for safe per-monitor destruction without shutting down
> > the shared dispatcher coroutine.
> >
> > Patch 3 adds the QAPI schema and command handlers.
> >
> > Patches 4-5 add qtest unit tests and a functional e2e test that
> > performs a full hotplug -> connect -> handshake -> unplug cycle.
>
> Patches 2, 4 and 5 would still be desirable even with the QOM
> conversion I'm thinking about.
>
> Patch 1 would be redundant since QOM would give us an ID facility
> as standard.
>
> Patch 3 would be redundant since we'd use object_add/object_del
> instead.
>
> >
> > > meson test "qtest-x86_64/qmp-test" "func-x86_64-monitor_hotplug" -v
> > ninja: Entering directory `/home/brauner/src/git/qemu/build'
> > [45/45] Linking target qemu-img
> > 1/2 qemu:func-quick+func-x86_64 / func-x86_64-monitor_hotplug RUNNING
> > >>> QEMU_TEST_QEMU_IMG=/home/brauner/src/git/qemu/build/qemu-img ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 MESON_BUILD_ROOT=/home/brauner/src/git/qemu/build LD_LIBRARY_PATH=/home/brauner/src/git/qemu/build/tests/tcg/plugins:/home/brauner/src/git/qemu/build/contrib/plugins:/home/brauner/src/go/deps/raft/.libs/:/home/brauner/src/go/deps/cowsql/.libs/ MALLOC_PERTURB_=165 PYTHONPATH=/home/brauner/src/git/qemu/python:/home/brauner/src/git/qemu/tests/functional QEMU_TEST_GDB=/usr/bin/gdb QEMU_TEST_QEMU_BINARY=/home/brauner/src/git/qemu/build/qemu-system-x86_64 MESON_TEST_ITERATION=1 UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 RUST_BACKTRACE=1 /home/brauner/src/git/qemu/build/pyvenv/bin/python3 /home/brauner/src/git/qemu/tests/functional/generic/test_monitor_hotplug.py
> > 2/2 qemu:qtest+qtest-x86_64 / qtest-x86_64/qmp-test RUNNING
> > >>> ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MALLOC_PERTURB_=244 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 PYTHON=/home/brauner/src/git/qemu/build/pyvenv/bin/python3 QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon QTEST_QEMU_IMG=./qemu-img G_TEST_DBUS_DAEMON=/home/brauner/src/git/qemu/tests/dbus-vmstate-daemon.sh QTEST_QEMU_BINARY=./qemu-system-x86_64 MESON_TEST_ITERATION=1 UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 RUST_BACKTRACE=1 /home/brauner/src/git/qemu/build/tests/qtest/qmp-test --tap -k
> > ▶ 1/2 test_monitor_hotplug.MonitorHotplug.test_events_after_negotiation OK
> > ▶ 2/2 /x86_64/qmp/protocol OK
> > ▶ 1/2 test_monitor_hotplug.MonitorHotplug.test_hotplug_cycle OK
> > ▶ 1/2 test_monitor_hotplug.MonitorHotplug.test_large_response OK
> > ▶ 2/2 /x86_64/qmp/oob OK
> > ▶ 1/2 test_monitor_hotplug.MonitorHotplug.test_self_removal OK
> > 1/2 qemu:func-quick+func-x86_64 / func-x86_64-monitor_hotplug OK 0.30s 4 subtests passed
> >
> > ▶ 2/2 /x86_64/qmp/preconfig OK
> > ▶ 2/2 /x86_64/qmp/missing-any-arg OK
> > ▶ 2/2 /x86_64/qmp/monitor-add-remove OK
> > ▶ 2/2 /x86_64/qmp/monitor-error-paths OK
> > ▶ 2/2 /x86_64/qmp/monitor-chardev-in-use OK
> > ▶ 2/2 /x86_64/qmp/monitor-remove-cli OK
> > ▶ 2/2 /x86_64/qmp/monitor-remove-hmp OK
> > 2/2 qemu:qtest+qtest-x86_64 / qtest-x86_64/qmp-test OK 1.09s 9 subtests passed
> >
> > Ok: 2
> > Expected Fail: 0
> > Fail: 0
> > Unexpected Pass: 0
> > Skipped: 0
> > Timeout: 0
> >
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> > Changes in v4:
> > - Move 'dead' field from patch 1 to patch 2 where it is first used.
> > - Allow removal of any QMP monitor, drop the 'dynamic' gate in
> > qmp_monitor_remove(). Drop the 'dynamic' field from struct Monitor
> > and from MonitorInfo entirely since it no longer serves a purpose.
> > - Auto-generate monitor ids ("mon0", "mon1", ...) for QMP monitors
> > created via CLI without an explicit id, so every QMP monitor is
> > addressable by monitor-remove and always appears with an id in
> > query-monitors output.
> > - Change Since: 11.0 to Since: 11.1 throughout the QAPI schema.
> > - Drop "GenericError" from QAPI error descriptions.
> > - Update monitor-remove QAPI doc to reflect that any QMP monitor can
> > be removed, not just dynamically added ones.
> > - Update qtest: test_qmp_monitor_remove_cli now expects success
> > instead of error.
> > - Link to v3: https://patch.msgid.link/20260407-work-qmp-monitor-hotplug-v3-0-cb259800fffb@kernel.org
> >
> > Changes in v3:
> > - Use SPDX license identifier in functional test.
> > - Use framework's socket_dir() helper instead of manual tempfile
> > handling for socket paths in the functional test, drop tearDown().
> > - Tighten struct field comments in monitor-internal.h.
> > - Wrap long qtest_add_func() registration line.
> > - Link to v2: https://patch.msgid.link/20260405-work-qmp-monitor-hotplug-v2-0-ad5bedd2917a@kernel.org
> >
> > Changes in v2:
> > - Fix use-after-free in self-removal path: skip monitor_resume() when
> > the monitor is dead to avoid scheduling a BH against a monitor that
> > is about to be freed by monitor_qmp_destroy().
> > - Hold monitor_lock in monitor_find_by_id() to prevent races with
> > the I/O thread BH that appends to mon_list.
> > - Deduplicate monitor-remove commit message: trim the gcontext/out_watch
> > explanation that repeated the infrastructure commit, reference
> > monitor_cancel_out_watch() instead.
> > - Add missing test descriptions to patch 4 (chardev-in-use, CLI monitor
> > rejection, HMP monitor rejection) and patch 5 (self-removal, large
> > response, events).
> > - Fix cover letter wording.
> > - Link to v1: https://patch.msgid.link/20260402-work-qmp-monitor-hotplug-v1-0-6313a5cdd574@kernel.org
> >
> > ---
> > Christian Brauner (5):
> > monitor: store monitor id in Monitor struct
> > monitor/qmp: add infrastructure for safe dynamic monitor removal
> > qapi: add monitor-add, monitor-remove, query-monitors commands
> > tests/qtest: add tests for dynamic monitor add/remove
> > tests/functional: add e2e test for dynamic QMP monitor hotplug
> >
> > include/monitor/monitor.h | 3 +-
> > monitor/monitor-internal.h | 10 ++
> > monitor/monitor.c | 70 ++++++--
> > monitor/qmp-cmds-control.c | 105 ++++++++++++
> > monitor/qmp.c | 81 +++++++++-
> > qapi/control.json | 97 ++++++++++++
> > tests/functional/generic/meson.build | 1 +
> > tests/functional/generic/test_monitor_hotplug.py | 170 ++++++++++++++++++++
> > tests/qtest/qmp-test.c | 193 +++++++++++++++++++++++
> > 9 files changed, 714 insertions(+), 16 deletions(-)
> > ---
> > base-commit: 6d3e9dddefdd2e8e6a4942ba9399df0e47df21ed
> > change-id: 20260402-work-qmp-monitor-hotplug-fba7c618e3db
> >
>
> 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] 10+ messages in thread* Re: [PATCH v4 0/5] monitor: add dynamic QMP monitor hotplug support
2026-04-10 7:36 ` Christian Brauner
@ 2026-04-10 16:25 ` Daniel P. Berrangé
0 siblings, 0 replies; 10+ messages in thread
From: Daniel P. Berrangé @ 2026-04-10 16:25 UTC (permalink / raw)
To: Christian Brauner
Cc: qemu-devel, Markus Armbruster, Eric Blake, Fabiano Rosas,
Laurent Vivier, Paolo Bonzini, Thomas Huth,
Philippe Mathieu-Daudé
On Fri, Apr 10, 2026 at 09:36:02AM +0200, Christian Brauner wrote:
> On Thu, Apr 09, 2026 at 08:43:40PM +0100, Daniel P. Berrangé wrote:
> > On Thu, Apr 09, 2026 at 09:18:17AM +0200, Christian Brauner wrote:
> > > QEMU supports runtime hotplug for chardevs, devices, block backends,
> > > and netdevs. Monitors are the only major subsystem that lacks this --
> > > all QMP monitors must be configured at launch via -mon or -qmp CLI
> > > options.
> > >
> > > This series adds monitor-add, monitor-remove, and query-monitors QMP
> > > commands so that management tools can create independent QMP sessions
> > > on demand at runtime.
> >
> > There's nothing inherently wrong with your proposal. It is following
> > the standard historical design practice we've taken for enabling
> > hotplug/unplug of all the other backend types. Conceptually though
> > our historical practice is more of an accident of evolution rather
> > than an ideal state.
> >
> > By this I mean we have a general purpose object framework
> > internally and commands '-object', and 'object_add' / 'object_del'
> > for hotplug/unplug in QMP and HMP. Conceptually these could handle
> > all of our objects, devices, netdev chardevs, monitors, etc, etc,
> > removing the need for the specialized add/remove QMP commands for
> > each backend type, and the specialized CLI argsf.
>
> If you actually plan on converting old types you're now left with two
> ways of adding objects. And if you plan on ultimately deprecating the
> old format it's going to be very annoying to fade those out.
We have two ways to dealing with this.
A formal deprecation & removal process which lets us fully
kill off the old way of doing things across 3 releases (1
year).
https://www.qemu.org/docs/master/about/deprecated.html
We use that alot, but for something as fundamental as the
monitor I don't think we would take that route as the
impact on users is too great. Instead we would follow the
approach of preserving the existing CLI arg and internally
rewriting it to the new syntax. So we only have one (new)
code path internally, except for a thin shim as the CLI
layer for conversion which has minimal long term burden on
maintainers.
Eventually we're likely to have a clean compat back with
new binaries, where we can drop all the legacy cruft from
the CLI.
> > The blocker has been lack of time to convert everything/anything,
> > since while the conversions are not especially difficult they are
> > certainly time consuming due to the sheer number of types that
> > exist in many cases.
> >
> >
> > I'm thinking the monitor, however, is a decent opportunity to
> > "do the right thing" from a design POV. We only have three classes
> > needed in QOM, the monitor generic base class, a HMP subclass and
> > a QMP subclass.
> >
> > If we convert QMP/HMP into QOM objects, we could then use the
> > pre-existing object_add/object_del commands in both HMP and QMP.
> >
> > I'm not asking you to do that conversion work though. I've
> > been doing some experimentation myself today to test the
> > viability of this idea and it looks promising.
>
> How difficult is this to do? I've had bad experiences with projects
> promising to do it all very differently and taking the patch away from
> me and then not following through or like 2 years later or the feature
> just vanished into nothing. I'm not saying this is the case here but I'm
> a bit weary of not having any way to move this forward myself other than
> "any news on this?" mails. It's frustrating for both sides.
>
> So if this is doable just give me what you got and let me see whether I
> can take it over the finish line.
I've just posted a series which gets it as far as being able to
use -object and object_add. I don't want your use case to be
delayed indefinitely - whichever design & impl, I'd want something
to be mergeable for the next schedule release.
> Also, I really dislike spending a lot of time understanding something
> and it all working and then being told "actually, let me do this". I
> avoid this like the plague doing this to developers in the kernel.
I'm sorry I didn't notice your series when it was the v1 posting,
as I would have liked to raise this idea earlier. It is delicate
balancing act between being accessible to new contributions, vs
trying to align with longer term design preferences.
> > Given we've got an entire 4 month dev cycle before these
> > proposed monitor-add/remove commands could get into a QEMU
> > release, I think it is worth trialling the QOM conversion
> > for a short while.
>
> I'm not sure what your governance model is so I'm just going to ask: Is
> this just a review or is this a project level decision?
This is just my design opinion as one of many QEMU maintainers,
albeit informed by many historical discussions on this topic.
Ultimately nothing is final until a patch series is accepted
and queued by a maintainer.
Hopefully others will give some input - I'm especialy looking
at Markus (CCd) for this as our QAPI & QMP design expert/maintainer.
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] 10+ messages in thread