From: Christian Brauner <[email protected]>
The removal sequence is:
1. 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, passing context=NULL and close
the connection.
4. Drain pending requests from any in-flight monitor_qmp_read().
5. Destroy the monitor object
Signed-off-by: Christian Brauner (Amutable) <[email protected]>
[DB: extracted from a larger commit and refactored to apply
to the new monitor class structure. Remove 'self delete'
feature which requires complex special-case code]
Signed-off-by: Daniel P. Berrangé <[email protected]>
---
monitor/monitor-internal.h | 2 ++
monitor/monitor.c | 22 ++++++++++++++++
monitor/qmp.c | 54 ++++++++++++++++++++++++++++++++++++--
3 files changed, 76 insertions(+), 2 deletions(-)
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index a82e1aacb6..5522e05464 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -178,6 +178,7 @@ struct MonitorQMP {
Monitor parent_obj;
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
@@ -206,6 +207,7 @@ extern MonitorList mon_list;
bool monitor_requires_iothread(const 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);
int monitor_set_cpu(Monitor *mon, int cpu_index);
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 0e1c623555..b155f58546 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -177,6 +177,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 (monitor_requires_iothread(mon)) {
+ 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)
{
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 2131be82c7..5301927f09 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -184,6 +184,12 @@ static void
monitor_qmp_cleanup_req_queue_locked(MonitorQMP *mon)
}
}
+static 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);
@@ -597,6 +603,7 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
monitor_qmp_read, monitor_qmp_event,
NULL, &mon->parent_obj, context, true);
monitor_list_append(&mon->parent_obj);
+ qatomic_set(&mon->setup_pending, false);
}
void monitor_new_qmp(const char *chardev_id, bool pretty, Error **errp)
@@ -645,6 +652,7 @@ static void monitor_qmp_complete(UserCreatable *uc, Error
**errp)
* 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);
/* The bottom half will add @mon to @mon_list */
@@ -656,8 +664,14 @@ static void monitor_qmp_complete(UserCreatable *uc, Error
**errp)
}
}
+static void monitor_qmp_iothread_quiesce(void *opaque)
+{
+ /* No-op: synchronization point only */
+}
+
static bool monitor_qmp_prepare_delete(UserCreatable *uc, Error **errp)
{
+ Monitor *mon = MONITOR(uc);
MonitorQMP *qmp = MONITOR_QMP(uc);
if (monitor_qmp_dispatcher_is_servicing(qmp)) {
@@ -665,8 +679,44 @@ static bool monitor_qmp_prepare_delete(UserCreatable *uc,
Error **errp)
return false;
}
- error_setg(errp, "Deleting QMP monitors is not supported");
- return false;
+ if (qatomic_read(&qmp->setup_pending)) {
+ error_setg(errp, "monitor is still initializing");
+ return false;
+ }
+
+ /* Remove from mon_list before chardev disconnect. */
+ WITH_QEMU_LOCK_GUARD(&monitor_lock) {
+ 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);
+ }
+
+ 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);
+
+ WITH_QEMU_LOCK_GUARD(&mon->mon_lock) {
+ /* Disable flushes before cancel -- gcontext is already wrong. */
+ qemu_chr_fe_set_open(&mon->chr, false);
+ monitor_cancel_out_watch(mon);
+ }
+
+ /* Synchronize with in-flight iothread callbacks. */
+ if (monitor_requires_iothread(mon)) {
+ 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(qmp);
+ monitor_fdsets_cleanup();
+
+ return true;
}
static void monitor_qmp_accept_input(Monitor *mon)
--
2.54.0
