[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 10/14] qmp: cleanup qmp queues properly
From: |
Eric Blake |
Subject: |
[Qemu-devel] [PULL 10/14] qmp: cleanup qmp queues properly |
Date: |
Tue, 27 Mar 2018 09:30:10 -0500 |
From: Peter Xu <address@hidden>
Marc-André Lureau reported that we can have this happen:
1. client1 connects, send command C1
2. client1 disconnects before getting response for C1
3. client2 connects, who might receive response of C1
However client2 should not receive remaining responses for client1.
Basically, we should clean up the request/response queue elements when:
- after a session is closed
- before destroying the queues
Some helpers are introduced to achieve that. We need to make sure we're
with the lock when operating on those queues. This also needed the
declaration of QMPRequest moved earlier.
Reported-by: Marc-André Lureau <address@hidden>
Signed-off-by: Peter Xu <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
[eblake: drop pointless qmp_response_free(), drop queue flush on connect
since a clean queue on disconnect is sufficient]
Tested-by: Christian Borntraeger <address@hidden>
Signed-off-by: Eric Blake <address@hidden>
---
monitor.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 52 insertions(+), 19 deletions(-)
diff --git a/monitor.c b/monitor.c
index 3b1ef34711b..32d6114ac3a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -234,6 +234,22 @@ static struct {
QEMUBH *qmp_respond_bh;
} mon_global;
+struct QMPRequest {
+ /* Owner of the request */
+ Monitor *mon;
+ /* "id" field of the request */
+ QObject *id;
+ /* Request object to be handled */
+ QObject *req;
+ /*
+ * Whether we need to resume the monitor afterward. This flag is
+ * used to emulate the old QMP server behavior that the current
+ * command must be completed before execution of the next one.
+ */
+ bool need_resume;
+};
+typedef struct QMPRequest QMPRequest;
+
/* QMP checker flags */
#define QMP_ACCEPT_UNKNOWNS 1
@@ -310,6 +326,38 @@ int monitor_read_password(Monitor *mon, ReadLineFunc
*readline_func,
}
}
+static void qmp_request_free(QMPRequest *req)
+{
+ qobject_decref(req->id);
+ qobject_decref(req->req);
+ g_free(req);
+}
+
+/* Must with the mon->qmp.qmp_queue_lock held */
+static void monitor_qmp_cleanup_req_queue_locked(Monitor *mon)
+{
+ while (!g_queue_is_empty(mon->qmp.qmp_requests)) {
+ qmp_request_free(g_queue_pop_head(mon->qmp.qmp_requests));
+ }
+}
+
+/* Must with the mon->qmp.qmp_queue_lock held */
+static void monitor_qmp_cleanup_resp_queue_locked(Monitor *mon)
+{
+ while (!g_queue_is_empty(mon->qmp.qmp_responses)) {
+ qobject_decref(g_queue_pop_head(mon->qmp.qmp_responses));
+ }
+}
+
+static void monitor_qmp_cleanup_queues(Monitor *mon)
+{
+ qemu_mutex_lock(&mon->qmp.qmp_queue_lock);
+ monitor_qmp_cleanup_req_queue_locked(mon);
+ monitor_qmp_cleanup_resp_queue_locked(mon);
+ qemu_mutex_unlock(&mon->qmp.qmp_queue_lock);
+}
+
+
static void monitor_flush_locked(Monitor *mon);
static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
@@ -701,6 +749,8 @@ static void monitor_data_destroy(Monitor *mon)
QDECREF(mon->outbuf);
qemu_mutex_destroy(&mon->out_lock);
qemu_mutex_destroy(&mon->qmp.qmp_queue_lock);
+ monitor_qmp_cleanup_req_queue_locked(mon);
+ monitor_qmp_cleanup_resp_queue_locked(mon);
g_queue_free(mon->qmp.qmp_requests);
g_queue_free(mon->qmp.qmp_responses);
}
@@ -4009,22 +4059,6 @@ static void monitor_qmp_respond(Monitor *mon, QObject
*rsp,
qobject_decref(rsp);
}
-struct QMPRequest {
- /* Owner of the request */
- Monitor *mon;
- /* "id" field of the request */
- QObject *id;
- /* Request object to be handled */
- QObject *req;
- /*
- * Whether we need to resume the monitor afterward. This flag is
- * used to emulate the old QMP server behavior that the current
- * command must be completed before execution of the next one.
- */
- bool need_resume;
-};
-typedef struct QMPRequest QMPRequest;
-
/*
* Dispatch one single QMP request. The function will free the req_obj
* and objects inside it before return.
@@ -4191,9 +4225,7 @@ static void handle_qmp_command(JSONMessageParser *parser,
GQueue *tokens)
qapi_event_send_command_dropped(id,
COMMAND_DROP_REASON_QUEUE_FULL,
&error_abort);
- qobject_decref(id);
- qobject_decref(req);
- g_free(req_obj);
+ qmp_request_free(req_obj);
return;
}
}
@@ -4335,6 +4367,7 @@ static void monitor_qmp_event(void *opaque, int event)
mon_refcount++;
break;
case CHR_EVENT_CLOSED:
+ monitor_qmp_cleanup_queues(mon);
json_message_parser_destroy(&mon->qmp.parser);
json_message_parser_init(&mon->qmp.parser, handle_qmp_command);
mon_refcount--;
--
2.14.3
- [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1, Eric Blake, 2018/03/27
- [Qemu-devel] [PULL 02/14] tests: Silence false positive warning on generated test name, Eric Blake, 2018/03/27
- [Qemu-devel] [PULL 04/14] error: Remove NULL checks on error_propagate() calls, Eric Blake, 2018/03/27
- [Qemu-devel] [PULL 03/14] error: Strip trailing '\n' from error string arguments (again again), Eric Blake, 2018/03/27
- [Qemu-devel] [PULL 01/14] qmp-test: fix response leak, Eric Blake, 2018/03/27
- [Qemu-devel] [PULL 07/14] qapi: restrict allow-oob value to be "true", Eric Blake, 2018/03/27
- [Qemu-devel] [PULL 12/14] tests: Add parameter to qtest_init_without_qmp_handshake, Eric Blake, 2018/03/27
- [Qemu-devel] [PULL 10/14] qmp: cleanup qmp queues properly,
Eric Blake <=
- [Qemu-devel] [PULL 11/14] monitor: new parameter "x-oob", Eric Blake, 2018/03/27
- [Qemu-devel] [PULL 13/14] tests: qmp-test: add test for new "x-oob", Eric Blake, 2018/03/27
- [Qemu-devel] [PULL 06/14] qmp: fix qmp_capabilities error regression, Eric Blake, 2018/03/27
- [Qemu-devel] [PULL 14/14] hmp.c: Revert hmp_info_cpus output format change, Eric Blake, 2018/03/27
- [Qemu-devel] [PULL 09/14] tests: add oob-test for qapi-schema, Eric Blake, 2018/03/27
- [Qemu-devel] [PULL 08/14] tests: let qapi-schema tests detect oob, Eric Blake, 2018/03/27
- [Qemu-devel] [PULL 05/14] qdict: remove useless cast, Eric Blake, 2018/03/27