[PATCH 2/5] monitor/qmp: add infrastructure for safe dynamic monitor removal

Christian Brauner posted 5 patches 1 week, 1 day ago
Maintainers: "Dr. David Alan Gilbert" <dave@treblig.org>, Markus Armbruster <armbru@redhat.com>, Eric Blake <eblake@redhat.com>, Thomas Huth <th.huth+qemu@posteo.eu>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Daniel P. Berrangé" <berrange@redhat.com>, Fabiano Rosas <farosas@suse.de>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
There is a newer version of this series
[PATCH 2/5] monitor/qmp: add infrastructure for safe dynamic monitor removal
Posted by Christian Brauner 1 week, 1 day ago
Extract monitor_qmp_destroy() from the shutdown-time monitor_cleanup()
path to allow destroying a single QMP monitor at runtime without
shutting down the entire dispatcher coroutine.

Export monitor_qmp_cleanup_queue_and_resume() so that monitor-remove
can drain pending requests before destroying the monitor.

Add qmp_dispatcher_current_mon tracking in the dispatcher coroutine to
handle the case where a monitor sends monitor-remove targeting itself.
After dispatching each request, the dispatcher checks the dead flag: if
set, it calls monitor_qmp_destroy() and clears the tracking pointer.

Both the dispatcher yield points and QMP command handlers run under the
BQL, so no additional locking is needed for qmp_dispatcher_current_mon.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 monitor/monitor-internal.h |  3 +++
 monitor/qmp.c              | 36 +++++++++++++++++++++++++++++++++++-
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 8f5fe7c111..40f76ef5a0 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -181,10 +181,13 @@ 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_cleanup_queue_and_resume(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/qmp.c b/monitor/qmp.c
index 5fbc7af074..0d393f3c96 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -71,6 +71,13 @@ typedef struct QMPRequest QMPRequest;
 
 QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
 
+/*
+ * The monitor currently being serviced by the dispatcher coroutine.
+ * Both the dispatcher and QMP command handlers (monitor-remove) run
+ * under the BQL, so no additional locking is needed.
+ */
+static MonitorQMP *qmp_dispatcher_current_mon;
+
 static bool qmp_oob_enabled(MonitorQMP *mon)
 {
     return mon->capab[QMP_CAPABILITY_OOB];
@@ -98,7 +105,7 @@ static void monitor_qmp_cleanup_req_queue_locked(MonitorQMP *mon)
     }
 }
 
-static void monitor_qmp_cleanup_queue_and_resume(MonitorQMP *mon)
+void monitor_qmp_cleanup_queue_and_resume(MonitorQMP *mon)
 {
     QEMU_LOCK_GUARD(&mon->qmp_queue_lock);
 
@@ -287,6 +294,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()
@@ -347,6 +355,16 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
         }
 
         qmp_request_free(req_obj);
+
+        /*
+         * If monitor-remove was called while we were dispatching a
+         * request on this monitor, the monitor was marked dead and
+         * removed from mon_list but destruction was deferred to us.
+         */
+        if (mon->common.dead) {
+            monitor_qmp_destroy(mon);
+        }
+        qmp_dispatcher_current_mon = NULL;
     }
     qatomic_set(&qmp_dispatcher_co, NULL);
 }
@@ -499,6 +517,22 @@ void monitor_data_destroy_qmp(MonitorQMP *mon)
     g_queue_free(mon->qmp_requests);
 }
 
+/*
+ * Destroy a single dynamically-added QMP monitor.
+ * The monitor must already have been removed from mon_list.
+ */
+void monitor_qmp_destroy(MonitorQMP *mon)
+{
+    monitor_flush(&mon->common);
+    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;

-- 
2.47.3