From fb246f05909c57341aec9a3ba42f1908b9cbfde6 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Jan 2021 16:12:00 +0000 Subject: monitor/qmp-cmds.c: Don't include ui/vnc.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The qmp-cmds.c file currently includes ui/vnc.h, which (being located in the ui/ directory rather than include) is really supposed to be for use only by the ui subsystem. In fact the function prototypes we need (vnc_display_password(), etc) are all declared in include/ui/console.h, so we can switch to including that instead. (ui/vnc.h includes include/ui/console.h, so this change strictly reduces the quantity of headers qmp-cmds.c pulls in.) Signed-off-by: Peter Maydell Message-Id: <20210104161200.15068-1-peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Markus Armbruster --- monitor/qmp-cmds.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c index 9909361..c7df8c0 100644 --- a/monitor/qmp-cmds.c +++ b/monitor/qmp-cmds.c @@ -23,7 +23,7 @@ #include "qemu/uuid.h" #include "chardev/char.h" #include "ui/qemu-spice.h" -#include "ui/vnc.h" +#include "ui/console.h" #include "sysemu/kvm.h" #include "sysemu/runstate.h" #include "sysemu/runstate-action.h" -- cgit v1.1 From 5086c9973a1303cce4791f3c244256e579191410 Mon Sep 17 00:00:00 2001 From: Zhang Han Date: Mon, 28 Dec 2020 15:11:26 +0800 Subject: qobject: open brace '{' following struct go on the same line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Put open brace '{' on the same line of struct. Signed-off-by: Zhang Han Message-Id: <20201228071129.24563-2-zhanghan64@huawei.com> Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- qobject/json-parser.c | 3 +-- qobject/qjson.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/qobject/json-parser.c b/qobject/json-parser.c index d351039..008b326 100644 --- a/qobject/json-parser.c +++ b/qobject/json-parser.c @@ -31,8 +31,7 @@ struct JSONToken { char str[]; }; -typedef struct JSONParserContext -{ +typedef struct JSONParserContext { Error *err; JSONToken *current; GQueue *buf; diff --git a/qobject/qjson.c b/qobject/qjson.c index bcc376e..167fcb4 100644 --- a/qobject/qjson.c +++ b/qobject/qjson.c @@ -22,8 +22,7 @@ #include "qapi/qmp/qnum.h" #include "qapi/qmp/qstring.h" -typedef struct JSONParsingState -{ +typedef struct JSONParsingState { JSONMessageParser parser; QObject *result; Error *err; -- cgit v1.1 From be08fb1897145e5fc3d11e05c2896b259887d9aa Mon Sep 17 00:00:00 2001 From: Zhang Han Date: Mon, 28 Dec 2020 15:11:27 +0800 Subject: qobject: code indent should never use tabs Transfer tabs to spaces. Signed-off-by: Zhang Han Message-Id: <20201228071129.24563-3-zhanghan64@huawei.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- qobject/qdict.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qobject/qdict.c b/qobject/qdict.c index d844433..d4d016a 100644 --- a/qobject/qdict.c +++ b/qobject/qdict.c @@ -39,8 +39,8 @@ QDict *qdict_new(void) */ static unsigned int tdb_hash(const char *name) { - unsigned value; /* Used to compute the hash value. */ - unsigned i; /* Used to cycle through random values. */ + unsigned value; /* Used to compute the hash value. */ + unsigned i; /* Used to cycle through random values. */ /* Set the initial value from the key size. */ for (value = 0x238F13AF * strlen(name), i=0; name[i]; i++) -- cgit v1.1 From f3d71c6e8d1ce6445c38e2247938903c1f661154 Mon Sep 17 00:00:00 2001 From: Zhang Han Date: Mon, 28 Dec 2020 15:11:28 +0800 Subject: qobject: spaces required around that operators Add spaces around operators. Signed-off-by: Zhang Han Message-Id: <20201228071129.24563-4-zhanghan64@huawei.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- qobject/qdict.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qobject/qdict.c b/qobject/qdict.c index d4d016a..6c15ed1 100644 --- a/qobject/qdict.c +++ b/qobject/qdict.c @@ -43,8 +43,8 @@ static unsigned int tdb_hash(const char *name) unsigned i; /* Used to cycle through random values. */ /* Set the initial value from the key size. */ - for (value = 0x238F13AF * strlen(name), i=0; name[i]; i++) - value = (value + (((const unsigned char *)name)[i] << (i*5 % 24))); + for (value = 0x238F13AF * strlen(name), i = 0; name[i]; i++) + value = (value + (((const unsigned char *)name)[i] << (i * 5 % 24))); return (1103515243 * value + 12345); } -- cgit v1.1 From 1841f0112c4d8a0a561462eed25fc4357fe40afc Mon Sep 17 00:00:00 2001 From: Zhang Han Date: Mon, 28 Dec 2020 15:11:29 +0800 Subject: qobject: braces {} are necessary for all arms of this statement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add braces {} for arms of if/for statement Signed-off-by: Zhang Han Message-Id: <20201228071129.24563-5-zhanghan64@huawei.com> Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- qobject/qdict.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/qobject/qdict.c b/qobject/qdict.c index 6c15ed1..0216ca7 100644 --- a/qobject/qdict.c +++ b/qobject/qdict.c @@ -43,8 +43,9 @@ static unsigned int tdb_hash(const char *name) unsigned i; /* Used to cycle through random values. */ /* Set the initial value from the key size. */ - for (value = 0x238F13AF * strlen(name), i = 0; name[i]; i++) + for (value = 0x238F13AF * strlen(name), i = 0; name[i]; i++) { value = (value + (((const unsigned char *)name)[i] << (i * 5 % 24))); + } return (1103515243 * value + 12345); } @@ -93,8 +94,9 @@ static QDictEntry *qdict_find(const QDict *qdict, QDictEntry *entry; QLIST_FOREACH(entry, &qdict->table[bucket], next) - if (!strcmp(entry->key, key)) + if (!strcmp(entry->key, key)) { return entry; + } return NULL; } -- cgit v1.1 From 781386afd2b809a8a63b65e9bfb645c4a8abdf50 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 27 Jan 2021 15:47:34 +0100 Subject: docs/interop/qmp-spec: Document the request queue limit Signed-off-by: Markus Armbruster Message-Id: <20210127144734.2367693-1-armbru@redhat.com> Reviewed-by: John Snow --- docs/interop/qmp-spec.txt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/interop/qmp-spec.txt b/docs/interop/qmp-spec.txt index cdf5842..b0e8351 100644 --- a/docs/interop/qmp-spec.txt +++ b/docs/interop/qmp-spec.txt @@ -133,9 +133,11 @@ to pass "id" with out-of-band commands. Passing it with all commands is recommended for clients that accept capability "oob". If the client sends in-band commands faster than the server can -execute them, the server will stop reading the requests from the QMP -channel until the request queue length is reduced to an acceptable -range. +execute them, the server will stop reading requests until the request +queue length is reduced to an acceptable range. + +To ensure commands to be executed out-of-band get read and executed, +the client should have at most eight in-band commands in flight. Only a few commands support out-of-band execution. The ones that do have "allow-oob": true in output of query-qmp-schema. -- cgit v1.1 From 395a95080ad9ccf08b71fa4b7909455ba42f2202 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 1 Feb 2021 17:15:02 +0100 Subject: qmp: Fix up comments after commit 9ce44e2ce2 Commit 9ce44e2ce2 "qmp: Move dispatcher to a coroutine" replaced monitor_qmp_bh_dispatcher() by monitor_qmp_dispatcher_co(), but neglected to update comments. Do that now. Signed-off-by: Markus Armbruster Message-Id: <20210201161504.1976989-2-armbru@redhat.com> Reviewed-by: Kevin Wolf --- monitor/qmp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/monitor/qmp.c b/monitor/qmp.c index 8f91af3..f6a1e77 100644 --- a/monitor/qmp.c +++ b/monitor/qmp.c @@ -79,7 +79,7 @@ static void monitor_qmp_cleanup_queue_and_resume(MonitorQMP *mon) qemu_mutex_lock(&mon->qmp_queue_lock); /* - * Same condition as in monitor_qmp_bh_dispatcher(), but before + * Same condition as in monitor_qmp_dispatcher_co(), but before * removing an element from the queue (hence no `- 1`). * Also, the queue should not be empty either, otherwise the * monitor hasn't been suspended yet (or was already resumed). @@ -349,7 +349,7 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) /* * Suspend the monitor when we can't queue more requests after - * this one. Dequeuing in monitor_qmp_bh_dispatcher() or + * this one. Dequeuing in monitor_qmp_dispatcher_co() or * monitor_qmp_cleanup_queue_and_resume() will resume it. * Note that when OOB is disabled, we queue at most one command, * for backward compatibility. -- cgit v1.1 From f680405f45afab692bd8021a35d40d037366abf1 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 1 Feb 2021 17:15:03 +0100 Subject: qmp: Add more tracepoints Add tracepoints for in-band request enqueue and dequeue, processing of queued in-band errors, and responses. Signed-off-by: Markus Armbruster Message-Id: <20210201161504.1976989-3-armbru@redhat.com> Reviewed-by: Kevin Wolf --- monitor/qmp.c | 7 +++++++ monitor/trace-events | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/monitor/qmp.c b/monitor/qmp.c index f6a1e77..e37b047 100644 --- a/monitor/qmp.c +++ b/monitor/qmp.c @@ -113,6 +113,7 @@ void qmp_send_response(MonitorQMP *mon, const QDict *rsp) json = qobject_to_json_pretty(data, mon->pretty); assert(json != NULL); + trace_monitor_qmp_respond(mon, json->str); g_string_append_c(json, '\n'); monitor_puts(&mon->common, json->str); @@ -251,6 +252,9 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data) } } + trace_monitor_qmp_in_band_dequeue(req_obj, + req_obj->mon->qmp_requests->length); + if (qatomic_xchg(&qmp_dispatcher_co_busy, true) == true) { /* * Someone rescheduled us (probably because a new requests @@ -287,6 +291,7 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data) monitor_qmp_dispatch(mon, req_obj->req); } else { assert(req_obj->err); + trace_monitor_qmp_err_in_band(error_get_pretty(req_obj->err)); rsp = qmp_error_response(req_obj->err); req_obj->err = NULL; monitor_qmp_respond(mon, rsp); @@ -364,6 +369,8 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) * handled in time order. Ownership for req_obj, req, * etc. will be delivered to the handler side. */ + trace_monitor_qmp_in_band_enqueue(req_obj, mon, + mon->qmp_requests->length); assert(mon->qmp_requests->length < QMP_REQ_QUEUE_LEN_MAX); g_queue_push_tail(mon->qmp_requests, req_obj); qemu_mutex_unlock(&mon->qmp_queue_lock); diff --git a/monitor/trace-events b/monitor/trace-events index 0365ac4..348dcfc 100644 --- a/monitor/trace-events +++ b/monitor/trace-events @@ -10,6 +10,10 @@ monitor_protocol_event_queue(uint32_t event, void *qdict, uint64_t rate) "event= monitor_suspend(void *ptr, int cnt) "mon %p: %d" # qmp.c +monitor_qmp_in_band_enqueue(void *req, void *mon, unsigned len) "%p mon %p len %u" +monitor_qmp_in_band_dequeue(void *req, unsigned len) "%p len %u" monitor_qmp_cmd_in_band(const char *id) "%s" +monitor_qmp_err_in_band(const char *desc) "%s" monitor_qmp_cmd_out_of_band(const char *id) "%s" +monitor_qmp_respond(void *mon, const char *json) "mon %p resp: %s" handle_qmp_command(void *mon, const char *req) "mon %p req: %s" -- cgit v1.1 From 88daf0996cd0488e93e67bcb0af258f2c24f117a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 1 Feb 2021 17:15:04 +0100 Subject: qmp: Resume OOB-enabled monitor before processing the request 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 Message-Id: <20210201161504.1976989-4-armbru@redhat.com> Reviewed-by: Kevin Wolf [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 e37b047..43880fa 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); /* -- cgit v1.1