[PULL 9/9] qmp: Resume OOB-enabled monitor before processing the request

From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, peter.maydell@linaro.org
Subject: [PULL 9/9] qmp: Resume OOB-enabled monitor before processing the request
Date: Thu,  4 Feb 2021 15:01:36 +0100	[thread overview]
Message-ID: <20210204140136.2769065-10-armbru@redhat.com> (raw)
In-Reply-To: <20210204140136.2769065-1-armbru@redhat.com>

monitor_qmp_dispatcher_co() needs to resume the monitor if
handle_qmp_command() suspended it.  Two cases:

1. OOB enabled: suspended if mon->qmp_requests has no more space

2. OOB disabled: suspended always

We resume only after we processed the request.  Which can take a long
time.

Resume the monitor right when the queue has space to keep the monitor
available for out-of-band commands even in this corner case.

Leave the "OOB disabled" case alone.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210201161504.1976989-4-armbru@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
[Trailing whitespace tidied up]
---
 monitor/qmp.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/monitor/qmp.c b/monitor/qmp.c
index e37b047c8a..43880fa623 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -214,7 +214,7 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
 {
     QMPRequest *req_obj = NULL;
     QDict *rsp;
-    bool need_resume;
+    bool oob_enabled;
     MonitorQMP *mon;
 
     while (true) {
@@ -273,11 +273,32 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
         aio_co_schedule(qemu_get_aio_context(), qmp_dispatcher_co);
         qemu_coroutine_yield();
 
+        /*
+         * @req_obj has a request, we hold req_obj->mon->qmp_queue_lock
+         */
+
         mon = req_obj->mon;
-        /* qmp_oob_enabled() might change after "qmp_capabilities" */
-        need_resume = !qmp_oob_enabled(mon) ||
-            mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1;
+
+        /*
+         * We need to resume the monitor if handle_qmp_command()
+         * suspended it.  Two cases:
+         * 1. OOB enabled: mon->qmp_requests has no more space
+         *    Resume right away, so that OOB commands can get executed while
+         *    this request is being processed.
+         * 2. OOB disabled: always
+         *    Resume only after we're done processing the request,
+         * We need to save qmp_oob_enabled() for later, because
+         * qmp_qmp_capabilities() can change it.
+         */
+        oob_enabled = qmp_oob_enabled(mon);
+        if (oob_enabled
+            && mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) {
+            monitor_resume(&mon->common);
+        }
+
         qemu_mutex_unlock(&mon->qmp_queue_lock);
+
+        /* Process request */
         if (req_obj->req) {
             if (trace_event_get_state(TRACE_MONITOR_QMP_CMD_IN_BAND)) {
                 QDict *qdict = qobject_to(QDict, req_obj->req);
@@ -298,10 +319,10 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
             qobject_unref(rsp);
         }
 
-        if (need_resume) {
-            /* Pairs with the monitor_suspend() in handle_qmp_command() */
+        if (!oob_enabled) {
             monitor_resume(&mon->common);
         }
+
         qmp_request_free(req_obj);
 
         /*
-- 
2.26.2


