From: Christian Brauner <brauner@kernel.org> 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, 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) <brauner@kernel.org> [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é <berrange@redhat.com> --- monitor/monitor-internal.h | 3 +++ monitor/monitor.c | 22 ++++++++++++++++ monitor/qmp.c | 53 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 76 insertions(+), 2 deletions(-) diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h index a82e1aacb6..2e8e5ec721 100644 --- a/monitor/monitor-internal.h +++ b/monitor/monitor-internal.h @@ -134,6 +134,7 @@ struct Monitor { char *chardev_id; CharFrontend chr; int suspend_cnt; /* Needs to be accessed atomically */ + bool dead; /* awaiting drain after monitor-remove */ QEMUBH *accept_input_bh; /* persistent BH for monitor_accept_input */ char *mon_cpu_path; QTAILQ_ENTRY(Monitor) entry; @@ -178,6 +179,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 +208,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 909f4f6052..6017d486af 100644 --- a/monitor/monitor.c +++ b/monitor/monitor.c @@ -176,6 +176,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 3f3d5b9d1c..d471428488 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); @@ -655,8 +661,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)) { @@ -664,8 +676,45 @@ 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) { + 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); + } + + 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