* [Qemu-devel] [PATCH 00/24] qapi: add async command type
@ 2016-10-10 9:22 Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 01/24] tests: start generic qemu-qmp tests Marc-André Lureau
` (23 more replies)
0 siblings, 24 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
Hi,
One of initial design goals of QMP was to have "asynchronous command
completion" (http://wiki.qemu.org/Features/QAPI). Unfortunately, that
goal was not achieved, and the broken bits left were removed
progressively until commit 65207c59d that removed it completely.
The benefits that can be trade-off depending on the requirements are:
1) allow the command handler to re-enter the main loop if the command
cannot be handled synchronously, or if it is long-lasting. This is
currently not possible and make some bugs such as rhbz#1230527 tricky
(see below) to solve. Furthermore, many QMP commands do IO and could
be considered slow today.
2) allow concurrent commands and events. This mainly implies hanlding
concurrency in qemu and out-of-order replies for the client. Let's
point out here is that QMP client already have to handle events
between a request and the reply, so they must have some support for
dispatching unrelated messages reply.
The traditional approach to solving the above in qemu is the following
scheme:
-> { "execute": "do-foo" }
<- { "return": {} }
<- { "event": "FOO_DONE" }
It has several flaws:
- FOO_DONE event has no semantic link with do-foo in the qapi
schema. It is not possible to generalize that pattern when writing
qmp clients. It makes documentation and usage harder.
- the FOO_DONE event has no clear association with the command that
triggered it: commands have to come up with additional specific
association schemes (ids, path etc)
- FOO_DONE is broadcasted to all clients, but they may have no way to
interprete it or interest in it
- the arguably useless empty return
For some cases, it makes sense to use that scheme, or a more complete
one: to have an "handler" associated with an on-going operation, that
can be queried, modified, cancelled etc (block jobs etc). Also, some
operations have a global side-effect, in which case that cmd+event
scheme is right, as all clients are listening for global events.
However, for the simple case where a client want to perform a "local
context" operation (dump, query etc), QAPI can easily do better
without resorting to this pattern: it can return when the operation is
done. That would make QAPI schema, usage and documentation more
obvious.
The following series implements an async solution, by introducing a
context associated with a command, it can:
- defer the return
- return only to the caller (no broadcasted event)
- return with the 'id' associated with the call (as originally intended)
- optionnally allow cancellation when the client is gone
- track on-going qapi command(s) per client
1) existing qemu commands can be gradually replaced by async:true
variants when needed, and by carefully reviewing the concurrency
aspects. The async:true commands marshaller helpers are splitted in
half, the calling and return functions. The command is called with a
QmpReturn context, that can return immediately or later, using the
generated return helper.
2) allow concurrent commands when 'async' is negotiated. If the client
doesn't support 'async', then the monitor is suspended during command
execution (events are still sent). Effectively, async commands behave
like normal commands from client POV in this case, giving full
backward compatibility.
The screendump command is converted to an async:true version to solve
rhbz#1230527. The command shows basic cancellation (this could be
extended if needed). HMP remains sync, but it would be worth to make
it possible to call async:true qapi commands, I started looking at
this.
The last patch cleans up qmp_dispatch usage to have consistant checks
between qga & qemu, and simplify QmpClient/parser_feed usage.
The series goes on-top of armbru/qapi-next, with a small fix for
qmp-test.c to fix make check with all arch.
Marc-André Lureau (24):
tests: start generic qemu-qmp tests
tests: change /0.15/* tests to /qmp/*
qmp: teach qmp_dispatch() to take a pre-filled QDict
qmp: use a return callback for the command reply
qmp: add QmpClient
qmp: add qmp_return_is_cancelled()
qmp: introduce async command type
qapi: ignore top-level 'id' field
qmp: take 'id' from request
qmp: check that async command have an 'id'
scripts: learn 'async' qapi commands
tests: add dispatch async tests
monitor: add 'async' capability
monitor: add !qmp pre-conditions
monitor: suspend when running async and client has no async
qmp: update qmp-spec about async capability
qtest: add qtest-timeout
qtest: add qtest_init_qmp_caps()
tests: add tests for async and non-async clients
qapi: improve 'screendump' documentation
console: graphic_hw_update return true if async
console: add graphic_hw_update_done()
console: make screendump async
qmp: move json-message-parser to QmpClient
hmp.c | 2 +-
hw/display/qxl-render.c | 14 +-
hw/display/qxl.c | 8 +-
monitor.c | 215 +++++++++++++-------------
qapi/qmp-dispatch.c | 257 +++++++++++++++++++++++++++++---
qapi/qmp-registry.c | 25 +++-
qga/main.c | 71 ++-------
qobject/json-lexer.c | 4 +-
qtest.c | 48 ++++++
tests/libqtest.c | 13 +-
tests/qmp-test.c | 138 +++++++++++++++++
tests/test-qmp-commands.c | 189 +++++++++++++++++++----
ui/console.c | 86 ++++++++++-
tests/Makefile.include | 3 +
scripts/qapi-commands.py | 139 +++++++++++++----
scripts/qapi-introspect.py | 7 +-
scripts/qapi.py | 14 +-
MAINTAINERS | 1 +
docs/qmp-spec.txt | 27 +++-
hw/display/qxl.h | 2 +-
include/qapi/qmp/dispatch.h | 65 +++++++-
include/ui/console.h | 5 +-
qapi-schema.json | 51 ++++++-
qapi/introspect.json | 2 +-
tests/libqtest.h | 9 ++
tests/qapi-schema/async.err | 0
tests/qapi-schema/async.exit | 1 +
tests/qapi-schema/async.json | 1 +
tests/qapi-schema/async.out | 7 +
tests/qapi-schema/qapi-schema-test.json | 4 +
tests/qapi-schema/qapi-schema-test.out | 5 +
tests/qapi-schema/test-qapi.py | 7 +-
32 files changed, 1136 insertions(+), 284 deletions(-)
create mode 100644 tests/qmp-test.c
create mode 100644 tests/qapi-schema/async.err
create mode 100644 tests/qapi-schema/async.exit
create mode 100644 tests/qapi-schema/async.json
create mode 100644 tests/qapi-schema/async.out
--
2.10.0
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 01/24] tests: start generic qemu-qmp tests
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 20:09 ` Eric Blake
2016-10-10 9:22 ` [Qemu-devel] [PATCH 02/24] tests: change /0.15/* tests to /qmp/* Marc-André Lureau
` (22 subsequent siblings)
23 siblings, 1 reply; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
These 2 tests exhibit two qmp bugs fixed by the previous patches.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20160922203927.28241-4-marcandre.lureau@redhat.com>
[Rename tests/test-qemu-qmp.c to tests/qmp-test.c, cover it in
MAINTAINERS, add a file comment]
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/qmp-test.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++
tests/Makefile.include | 2 ++
MAINTAINERS | 1 +
3 files changed, 82 insertions(+)
create mode 100644 tests/qmp-test.c
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
new file mode 100644
index 0000000..480ff28
--- /dev/null
+++ b/tests/qmp-test.c
@@ -0,0 +1,79 @@
+/*
+ * QTest testcase for QMP
+ *
+ * Copyright (c) 2016 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+/*
+ * This program tests QMP commands maintained with the QMP core.
+ * These are defined in qmp.c. Tests for QMP commands defined in
+ * another subsystem should go into a test program maintained with
+ * that subsystem.
+ *
+ * TODO Actually cover the commands. The tests we got so far only
+ * demonstrate specific bugs we've fixed.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+
+static void test_object_add_without_props(void)
+{
+ QDict *ret, *error;
+ const gchar *klass, *desc;
+
+ ret = qmp("{'execute': 'object-add',"
+ " 'arguments': { 'qom-type': 'memory-backend-ram', 'id': 'ram1' } }");
+ g_assert_nonnull(ret);
+
+ error = qdict_get_qdict(ret, "error");
+ klass = qdict_get_try_str(error, "class");
+ desc = qdict_get_try_str(error, "desc");
+
+ g_assert_cmpstr(klass, ==, "GenericError");
+ g_assert_cmpstr(desc, ==, "can't create backend with size 0");
+
+ QDECREF(ret);
+}
+
+static void test_qom_set_without_value(void)
+{
+ QDict *ret, *error;
+ const gchar *klass, *desc;
+
+ ret = qmp("{'execute': 'qom-set',"
+ " 'arguments': { 'path': '/machine', 'property': 'rtc-time' } }");
+ g_assert_nonnull(ret);
+
+ error = qdict_get_qdict(ret, "error");
+ klass = qdict_get_try_str(error, "class");
+ desc = qdict_get_try_str(error, "desc");
+
+ g_assert_cmpstr(klass, ==, "GenericError");
+ g_assert_cmpstr(desc, ==, "Parameter 'value' is missing");
+
+ QDECREF(ret);
+}
+
+int main(int argc, char **argv)
+{
+ int ret;
+
+ g_test_init(&argc, &argv, NULL);
+
+ qtest_start("-machine none");
+
+ qtest_add_func("/qemu-qmp/object-add-without-props",
+ test_object_add_without_props);
+ qtest_add_func("/qemu-qmp/qom-set-without-value",
+ test_qom_set_without_value);
+
+ ret = g_test_run();
+
+ qtest_end();
+
+ return ret;
+}
diff --git a/tests/Makefile.include b/tests/Makefile.include
index a77777c..b929c48 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -307,6 +307,7 @@ check-qtest-s390x-y = tests/boot-serial-test$(EXESUF)
check-qtest-generic-y += tests/qom-test$(EXESUF)
check-qtest-generic-y += tests/ptimer-test$(EXESUF)
+check-qtest-generic-y += tests/qmp-test$(EXESUF)
qapi-schema += alternate-any.json
qapi-schema += alternate-array.json
@@ -650,6 +651,7 @@ tests/tpci200-test$(EXESUF): tests/tpci200-test.o
tests/display-vga-test$(EXESUF): tests/display-vga-test.o
tests/ipoctal232-test$(EXESUF): tests/ipoctal232-test.o
tests/qom-test$(EXESUF): tests/qom-test.o
+tests/qmp-test$(EXESUF): tests/qmp-test.o
tests/drive_del-test$(EXESUF): tests/drive_del-test.o $(libqos-pc-obj-y)
tests/qdev-monitor-test$(EXESUF): tests/qdev-monitor-test.o $(libqos-pc-obj-y)
tests/nvme-test$(EXESUF): tests/nvme-test.o
diff --git a/MAINTAINERS b/MAINTAINERS
index a4d2dd4..41f4ea1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1285,6 +1285,7 @@ F: qmp.c
F: monitor.c
F: docs/*qmp-*
F: scripts/qmp/
+F: tests/qmp-test.c
T: git git://repo.or.cz/qemu/armbru.git qapi-next
Register API
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 02/24] tests: change /0.15/* tests to /qmp/*
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 01/24] tests: start generic qemu-qmp tests Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 20:10 ` Eric Blake
2016-10-10 9:22 ` [Qemu-devel] [PATCH 03/24] qmp: teach qmp_dispatch() to take a pre-filled QDict Marc-André Lureau
` (21 subsequent siblings)
23 siblings, 1 reply; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
Presumably 0.15 was the version it was first introduced, but
qmp keeps evolving. There is no point in having that version
as test prefix, 'qmp' makes more sense here.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/test-qmp-commands.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 81cbe54..5ff2f9b 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -267,11 +267,11 @@ int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
- g_test_add_func("/0.15/dispatch_cmd", test_dispatch_cmd);
- g_test_add_func("/0.15/dispatch_cmd_failure", test_dispatch_cmd_failure);
- g_test_add_func("/0.15/dispatch_cmd_io", test_dispatch_cmd_io);
- g_test_add_func("/0.15/dealloc_types", test_dealloc_types);
- g_test_add_func("/0.15/dealloc_partial", test_dealloc_partial);
+ g_test_add_func("/qmp/dispatch_cmd", test_dispatch_cmd);
+ g_test_add_func("/qmp/dispatch_cmd_failure", test_dispatch_cmd_failure);
+ g_test_add_func("/qmp/dispatch_cmd_io", test_dispatch_cmd_io);
+ g_test_add_func("/qmp/dealloc_types", test_dealloc_types);
+ g_test_add_func("/qmp/dealloc_partial", test_dealloc_partial);
module_call_init(MODULE_INIT_QAPI);
g_test_run();
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 03/24] qmp: teach qmp_dispatch() to take a pre-filled QDict
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 01/24] tests: start generic qemu-qmp tests Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 02/24] tests: change /0.15/* tests to /qmp/* Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 20:20 ` Eric Blake
2016-10-10 9:22 ` [Qemu-devel] [PATCH 04/24] qmp: use a return callback for the command reply Marc-André Lureau
` (20 subsequent siblings)
23 siblings, 1 reply; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
Give an optionnal qdict for the dispatch call to be used for the
reply. The qemu monitor has the request "id" pre-filled, simplifying a
bit the code.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
monitor.c | 26 +++++++++++---------------
qapi/qmp-dispatch.c | 5 ++---
qga/main.c | 2 +-
tests/test-qmp-commands.c | 8 ++++----
include/qapi/qmp/dispatch.h | 2 +-
5 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/monitor.c b/monitor.c
index 4ff74b7..f485ebf 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3732,12 +3732,13 @@ static QDict *qmp_check_input_obj(QObject *input_obj, Error **errp)
static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
{
- QObject *req, *rsp = NULL, *id = NULL;
- QDict *qdict = NULL;
+ QObject *req, *rsp, *id = NULL;
+ QDict *qdict, *rqdict = qdict_new();
const char *cmd_name;
Monitor *mon = cur_mon;
Error *err = NULL;
+ rsp = QOBJECT(rqdict);
req = json_parser_parse_err(tokens, NULL, &err);
if (err || !req || qobject_type(req) != QTYPE_QDICT) {
if (!err) {
@@ -3752,8 +3753,11 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
}
id = qdict_get(qdict, "id");
- qobject_incref(id);
- qdict_del(qdict, "id");
+ if (id) {
+ qobject_incref(id);
+ qdict_del(qdict, "id");
+ qdict_put_obj(rqdict, "id", id);
+ }
cmd_name = qdict_get_str(qdict, "execute");
trace_handle_qmp_command(mon, cmd_name);
@@ -3762,27 +3766,19 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
goto err_out;
}
- rsp = qmp_dispatch(req);
+ rsp = qmp_dispatch(req, rqdict);
err_out:
if (err) {
- qdict = qdict_new();
- qdict_put_obj(qdict, "error", qmp_build_error_object(err));
+ qdict_put_obj(rqdict, "error", qmp_build_error_object(err));
error_free(err);
- rsp = QOBJECT(qdict);
}
if (rsp) {
- if (id) {
- qdict_put_obj(qobject_to_qdict(rsp), "id", id);
- id = NULL;
- }
-
monitor_json_emitter(mon, rsp);
}
- qobject_decref(id);
- qobject_decref(rsp);
+ QDECREF(rqdict);
qobject_decref(req);
}
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 505eb41..dbf2e5b 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -116,15 +116,14 @@ QObject *qmp_build_error_object(Error *err)
error_get_pretty(err));
}
-QObject *qmp_dispatch(QObject *request)
+QObject *qmp_dispatch(QObject *request, QDict *rsp)
{
Error *err = NULL;
QObject *ret;
- QDict *rsp;
ret = do_qmp_dispatch(request, &err);
- rsp = qdict_new();
+ rsp = rsp ?: qdict_new();
if (err) {
qdict_put_obj(rsp, "error", qmp_build_error_object(err));
error_free(err);
diff --git a/qga/main.c b/qga/main.c
index 0b9d04e..d236e0e 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -555,7 +555,7 @@ static void process_command(GAState *s, QDict *req)
g_assert(req);
g_debug("processing command");
- rsp = qmp_dispatch(QOBJECT(req));
+ rsp = qmp_dispatch(QOBJECT(req), NULL);
if (rsp) {
ret = send_response(s, rsp);
if (ret) {
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 5ff2f9b..1adc1a2 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -94,7 +94,7 @@ static void test_dispatch_cmd(void)
qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd")));
- resp = qmp_dispatch(QOBJECT(req));
+ resp = qmp_dispatch(QOBJECT(req), NULL);
assert(resp != NULL);
assert(!qdict_haskey(qobject_to_qdict(resp), "error"));
@@ -111,7 +111,7 @@ static void test_dispatch_cmd_failure(void)
qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd2")));
- resp = qmp_dispatch(QOBJECT(req));
+ resp = qmp_dispatch(QOBJECT(req), NULL);
assert(resp != NULL);
assert(qdict_haskey(qobject_to_qdict(resp), "error"));
@@ -125,7 +125,7 @@ static void test_dispatch_cmd_failure(void)
qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd")));
- resp = qmp_dispatch(QOBJECT(req));
+ resp = qmp_dispatch(QOBJECT(req), NULL);
assert(resp != NULL);
assert(qdict_haskey(qobject_to_qdict(resp), "error"));
@@ -139,7 +139,7 @@ static QObject *test_qmp_dispatch(QDict *req)
QDict *resp;
QObject *ret;
- resp_obj = qmp_dispatch(QOBJECT(req));
+ resp_obj = qmp_dispatch(QOBJECT(req), NULL);
assert(resp_obj);
resp = qobject_to_qdict(resp_obj);
assert(resp && !qdict_haskey(resp, "error"));
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 57651ea..db48348 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -38,7 +38,7 @@ void qmp_register_command(const char *name, QmpCommandFunc *fn,
QmpCommandOptions options);
void qmp_unregister_command(const char *name);
QmpCommand *qmp_find_command(const char *name);
-QObject *qmp_dispatch(QObject *request);
+QObject *qmp_dispatch(QObject *request, QDict *rsp);
void qmp_disable_command(const char *name);
void qmp_enable_command(const char *name);
bool qmp_command_is_enabled(const QmpCommand *cmd);
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 04/24] qmp: use a return callback for the command reply
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (2 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 03/24] qmp: teach qmp_dispatch() to take a pre-filled QDict Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 05/24] qmp: add QmpClient Marc-André Lureau
` (19 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
Introduce QmpDispatchReturn, a callback called when a command reply is
ready to be sent. Future patches will extend the context to enable async
replies. Currently, all qmp commands reply in the calling context (they
are all sync).
QmpReturn and associated functions is used internally for the sync
dispatch, but will be the basis of the following async context.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
monitor.c | 19 +++++++++------
qapi/qmp-dispatch.c | 58 +++++++++++++++++++++++++++++++++++----------
qga/main.c | 22 ++++++++---------
tests/test-qmp-commands.c | 54 ++++++++++++++++++++++-------------------
include/qapi/qmp/dispatch.h | 22 ++++++++++++++++-
5 files changed, 119 insertions(+), 56 deletions(-)
diff --git a/monitor.c b/monitor.c
index f485ebf..46a07d8 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3730,15 +3730,21 @@ static QDict *qmp_check_input_obj(QObject *input_obj, Error **errp)
return input_dict;
}
+static void qmp_dispatch_return(QObject *rsp, void *opaque)
+{
+ Monitor *mon = opaque;
+
+ monitor_json_emitter(mon, rsp);
+}
+
static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
{
- QObject *req, *rsp, *id = NULL;
+ QObject *req, *id = NULL;
QDict *qdict, *rqdict = qdict_new();
const char *cmd_name;
Monitor *mon = cur_mon;
Error *err = NULL;
- rsp = QOBJECT(rqdict);
req = json_parser_parse_err(tokens, NULL, &err);
if (err || !req || qobject_type(req) != QTYPE_QDICT) {
if (!err) {
@@ -3766,16 +3772,15 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
goto err_out;
}
- rsp = qmp_dispatch(req, rqdict);
+ qmp_dispatch(req, rqdict, qmp_dispatch_return, mon);
+ qobject_decref(req);
+ return;
err_out:
if (err) {
qdict_put_obj(rqdict, "error", qmp_build_error_object(err));
error_free(err);
- }
-
- if (rsp) {
- monitor_json_emitter(mon, rsp);
+ monitor_json_emitter(mon, QOBJECT(rqdict));
}
QDECREF(rqdict);
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index dbf2e5b..7e9d03f 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -62,7 +62,7 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp)
return dict;
}
-static QObject *do_qmp_dispatch(QObject *request, Error **errp)
+static QObject *do_qmp_dispatch(QObject *request, QmpReturn *qret, Error **errp)
{
Error *local_err = NULL;
const char *command;
@@ -116,23 +116,57 @@ QObject *qmp_build_error_object(Error *err)
error_get_pretty(err));
}
-QObject *qmp_dispatch(QObject *request, QDict *rsp)
+static void qmp_return_free(QmpReturn *qret)
+{
+ QDict *rsp = qret->rsp;
+
+ qobject_decref(QOBJECT(rsp));
+ g_free(qret);
+}
+
+static void do_qmp_return(QmpReturn *qret)
+{
+ QDict *rsp = qret->rsp;
+
+ qret->return_cb(QOBJECT(rsp), qret->opaque);
+
+ qmp_return_free(qret);
+}
+
+void qmp_return(QmpReturn *qret, QObject *cmd_rsp)
+{
+ qdict_put_obj(qret->rsp, "return", cmd_rsp ?: QOBJECT(qdict_new()));
+
+ do_qmp_return(qret);
+}
+
+void qmp_return_error(QmpReturn *qret, Error *err)
+{
+ qdict_put_obj(qret->rsp, "error", qmp_build_error_object(err));
+ error_free(err);
+
+ do_qmp_return(qret);
+}
+
+void qmp_dispatch(QObject *request, QDict *rsp,
+ QmpDispatchReturn *return_cb, void *opaque)
{
Error *err = NULL;
+ QmpReturn *qret = g_new0(QmpReturn, 1);
QObject *ret;
- ret = do_qmp_dispatch(request, &err);
+ assert(return_cb);
+
+ qret->rsp = rsp ?: qdict_new();
+ qret->return_cb = return_cb;
+ qret->opaque = opaque;
+
+ ret = do_qmp_dispatch(request, qret, &err);
- rsp = rsp ?: qdict_new();
if (err) {
- qdict_put_obj(rsp, "error", qmp_build_error_object(err));
- error_free(err);
+ assert(!ret);
+ qmp_return_error(qret, err);
} else if (ret) {
- qdict_put_obj(rsp, "return", ret);
- } else {
- QDECREF(rsp);
- return NULL;
+ qmp_return(qret, ret);
}
-
- return QOBJECT(rsp);
}
diff --git a/qga/main.c b/qga/main.c
index d236e0e..349e4bf 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -548,21 +548,21 @@ static int send_response(GAState *s, QObject *payload)
return 0;
}
-static void process_command(GAState *s, QDict *req)
+static void dispatch_return_cb(QObject *rsp, void *opaque)
{
- QObject *rsp = NULL;
- int ret;
+ GAState *s = opaque;
+ int ret = send_response(s, rsp);
+
+ if (ret) {
+ g_warning("error sending response: %s", strerror(ret));
+ }
+}
+static void process_command(GAState *s, QDict *req)
+{
g_assert(req);
g_debug("processing command");
- rsp = qmp_dispatch(QOBJECT(req), NULL);
- if (rsp) {
- ret = send_response(s, rsp);
- if (ret) {
- g_warning("error sending response: %s", strerror(ret));
- }
- qobject_decref(rsp);
- }
+ qmp_dispatch(QOBJECT(req), NULL, dispatch_return_cb, s);
}
/* handle requests/control events coming in over the channel */
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 1adc1a2..88cdd3a 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -85,23 +85,30 @@ __org_qemu_x_Union1 *qmp___org_qemu_x_command(__org_qemu_x_EnumList *a,
return ret;
}
+static void dispatch_cmd_return(QObject *resp, void *opaque)
+{
+ assert(resp != NULL);
+ assert(!qdict_haskey(qobject_to_qdict(resp), "error"));
+}
/* test commands with no input and no return value */
static void test_dispatch_cmd(void)
{
QDict *req = qdict_new();
- QObject *resp;
qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd")));
- resp = qmp_dispatch(QOBJECT(req), NULL);
- assert(resp != NULL);
- assert(!qdict_haskey(qobject_to_qdict(resp), "error"));
+ qmp_dispatch(QOBJECT(req), NULL, dispatch_cmd_return, NULL);
- qobject_decref(resp);
QDECREF(req);
}
+static void dispatch_cmd_error_return(QObject *resp, void *opaque)
+{
+ assert(resp != NULL);
+ assert(qdict_haskey(qobject_to_qdict(resp), "error"));
+}
+
/* test commands that return an error due to invalid parameters */
static void test_dispatch_cmd_failure(void)
{
@@ -111,11 +118,7 @@ static void test_dispatch_cmd_failure(void)
qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd2")));
- resp = qmp_dispatch(QOBJECT(req), NULL);
- assert(resp != NULL);
- assert(qdict_haskey(qobject_to_qdict(resp), "error"));
-
- qobject_decref(resp);
+ qmp_dispatch(QOBJECT(req), NULL, dispatch_cmd_error_return, NULL);
QDECREF(req);
/* check that with extra arguments it throws an error */
@@ -125,28 +128,29 @@ static void test_dispatch_cmd_failure(void)
qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd")));
- resp = qmp_dispatch(QOBJECT(req), NULL);
- assert(resp != NULL);
- assert(qdict_haskey(qobject_to_qdict(resp), "error"));
-
- qobject_decref(resp);
+ qmp_dispatch(QOBJECT(req), NULL, dispatch_cmd_error_return, NULL);
QDECREF(req);
}
+static QObject *dispatch_ret;
+
+static void qmp_dispatch_return(QObject *resp_obj, void *opaque)
+{
+ QDict *resp = qobject_to_qdict(resp_obj);
+ assert(resp && !qdict_haskey(resp, "error"));
+ dispatch_ret = qdict_get(resp, "return");
+ assert(dispatch_ret);
+ qobject_incref(dispatch_ret);
+}
+
static QObject *test_qmp_dispatch(QDict *req)
{
- QObject *resp_obj;
- QDict *resp;
QObject *ret;
- resp_obj = qmp_dispatch(QOBJECT(req), NULL);
- assert(resp_obj);
- resp = qobject_to_qdict(resp_obj);
- assert(resp && !qdict_haskey(resp, "error"));
- ret = qdict_get(resp, "return");
- assert(ret);
- qobject_incref(ret);
- qobject_decref(resp_obj);
+ qmp_dispatch(QOBJECT(req), NULL, qmp_dispatch_return, NULL);
+ ret = dispatch_ret;
+ dispatch_ret = NULL;
+
return ret;
}
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index db48348..780b3e2 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -17,6 +17,14 @@
#include "qapi/qmp/qobject.h"
#include "qapi/qmp/qdict.h"
+typedef void (QmpDispatchReturn) (QObject *rsp, void *opaque);
+
+typedef struct QmpReturn {
+ QDict *rsp;
+ QmpDispatchReturn *return_cb;
+ void *opaque;
+} QmpReturn;
+
typedef void (QmpCommandFunc)(QDict *, QObject **, Error **);
typedef enum QmpCommandOptions
@@ -38,7 +46,8 @@ void qmp_register_command(const char *name, QmpCommandFunc *fn,
QmpCommandOptions options);
void qmp_unregister_command(const char *name);
QmpCommand *qmp_find_command(const char *name);
-QObject *qmp_dispatch(QObject *request, QDict *rsp);
+void qmp_dispatch(QObject *request, QDict *rsp,
+ QmpDispatchReturn *return_cb, void *opaque);
void qmp_disable_command(const char *name);
void qmp_enable_command(const char *name);
bool qmp_command_is_enabled(const QmpCommand *cmd);
@@ -48,4 +57,15 @@ QObject *qmp_build_error_object(Error *err);
typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque);
void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque);
+/*
+ * qmp_return{_error}:
+ *
+ * Construct the command reply, and call the
+ * return_cb() associated with the dispatch.
+ *
+ * Finally, free the QmpReturn.
+ */
+void qmp_return(QmpReturn *qret, QObject *cmd_rsp);
+void qmp_return_error(QmpReturn *qret, Error *err);
+
#endif
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 05/24] qmp: add QmpClient
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (3 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 04/24] qmp: use a return callback for the command reply Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 06/24] qmp: add qmp_return_is_cancelled() Marc-André Lureau
` (18 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
Add a new QmpClient structure holding the dispatch return callback
and the list of pending QmpReturns.
When a client disconnects, call qmp_client_destroy(). This will remove
all pending returns from the client list, and prevent a reply from being
sent later: new clients will only receive reply they expect, and not one
from a previous client.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
monitor.c | 11 ++++++++---
qapi/qmp-dispatch.c | 41 +++++++++++++++++++++++++++++++++++------
qga/main.c | 10 ++++++----
tests/test-qmp-commands.c | 32 ++++++++++++++++++++++----------
include/qapi/qmp/dispatch.h | 22 ++++++++++++++++------
5 files changed, 87 insertions(+), 29 deletions(-)
diff --git a/monitor.c b/monitor.c
index 46a07d8..d7baa86 100644
--- a/monitor.c
+++ b/monitor.c
@@ -173,6 +173,7 @@ typedef struct {
* mode.
*/
bool in_command_mode; /* are we in command mode? */
+ QmpClient client;
} MonitorQMP;
/*
@@ -593,6 +594,7 @@ static void monitor_data_destroy(Monitor *mon)
if (monitor_is_qmp(mon)) {
json_message_parser_destroy(&mon->qmp.parser);
}
+ qmp_client_destroy(&mon->qmp.client);
g_free(mon->rs);
QDECREF(mon->outbuf);
qemu_mutex_destroy(&mon->out_lock);
@@ -3730,9 +3732,9 @@ static QDict *qmp_check_input_obj(QObject *input_obj, Error **errp)
return input_dict;
}
-static void qmp_dispatch_return(QObject *rsp, void *opaque)
+static void qmp_dispatch_return(QmpClient *client, QObject *rsp)
{
- Monitor *mon = opaque;
+ Monitor *mon = container_of(client, Monitor, qmp.client);
monitor_json_emitter(mon, rsp);
}
@@ -3772,7 +3774,7 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
goto err_out;
}
- qmp_dispatch(req, rqdict, qmp_dispatch_return, mon);
+ qmp_dispatch(&mon->qmp.client, req, rqdict);
qobject_decref(req);
return;
@@ -3870,6 +3872,8 @@ static void monitor_qmp_event(void *opaque, int event)
case CHR_EVENT_CLOSED:
json_message_parser_destroy(&mon->qmp.parser);
json_message_parser_init(&mon->qmp.parser, handle_qmp_command);
+ qmp_client_destroy(&mon->qmp.client);
+ qmp_client_init(&mon->qmp.client, qmp_dispatch_return);
mon_refcount--;
monitor_fdsets_cleanup();
break;
@@ -3997,6 +4001,7 @@ void monitor_init(CharDriverState *chr, int flags)
monitor_qmp_event, mon);
qemu_chr_fe_set_echo(chr, true);
json_message_parser_init(&mon->qmp.parser, handle_qmp_command);
+ qmp_client_init(&mon->qmp.client, qmp_dispatch_return);
} else {
qemu_chr_add_handlers(chr, monitor_can_read, monitor_read,
monitor_event, mon);
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 7e9d03f..c8821b7 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -120,6 +120,10 @@ static void qmp_return_free(QmpReturn *qret)
{
QDict *rsp = qret->rsp;
+ if (qret->client) {
+ QLIST_REMOVE(qret, link);
+ }
+
qobject_decref(QOBJECT(rsp));
g_free(qret);
}
@@ -127,8 +131,11 @@ static void qmp_return_free(QmpReturn *qret)
static void do_qmp_return(QmpReturn *qret)
{
QDict *rsp = qret->rsp;
+ QmpClient *client = qret->client;
- qret->return_cb(QOBJECT(rsp), qret->opaque);
+ if (client) {
+ client->return_cb(client, QOBJECT(rsp));
+ }
qmp_return_free(qret);
}
@@ -148,25 +155,47 @@ void qmp_return_error(QmpReturn *qret, Error *err)
do_qmp_return(qret);
}
-void qmp_dispatch(QObject *request, QDict *rsp,
- QmpDispatchReturn *return_cb, void *opaque)
+void qmp_client_init(QmpClient *client, QmpDispatchReturn *return_cb)
+{
+ client->return_cb = return_cb;
+ QLIST_INIT(&client->pending);
+}
+
+void qmp_client_destroy(QmpClient *client)
+{
+ QmpReturn *ret, *next;
+
+ /* Remove the weak references to the pending returns. The
+ * dispatched function is the owner of QmpReturn, and will have to
+ * qmp_return(). (it might be interesting to have a way to notify
+ * that the client disconnected to cancel an on-going
+ * operation) */
+ QLIST_FOREACH_SAFE(ret, &client->pending, link, next) {
+ ret->client = NULL;
+ QLIST_REMOVE(ret, link);
+ }
+}
+
+void qmp_dispatch(QmpClient *client, QObject *request, QDict *rsp)
{
Error *err = NULL;
QmpReturn *qret = g_new0(QmpReturn, 1);
QObject *ret;
- assert(return_cb);
+ assert(client);
qret->rsp = rsp ?: qdict_new();
- qret->return_cb = return_cb;
- qret->opaque = opaque;
+ qret->client = client;
+ QLIST_INSERT_HEAD(&client->pending, qret, link);
ret = do_qmp_dispatch(request, qret, &err);
if (err) {
assert(!ret);
qmp_return_error(qret, err);
+ return;
} else if (ret) {
qmp_return(qret, ret);
+ return;
}
}
diff --git a/qga/main.c b/qga/main.c
index 349e4bf..267e3bb 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -89,6 +89,7 @@ struct GAState {
#endif
gchar *pstate_filepath;
GAPersistentState pstate;
+ QmpClient client;
};
struct GAState *ga_state;
@@ -548,9 +549,9 @@ static int send_response(GAState *s, QObject *payload)
return 0;
}
-static void dispatch_return_cb(QObject *rsp, void *opaque)
+static void dispatch_return_cb(QmpClient *client, QObject *rsp)
{
- GAState *s = opaque;
+ GAState *s = container_of(client, GAState, client);
int ret = send_response(s, rsp);
if (ret) {
@@ -562,7 +563,7 @@ static void process_command(GAState *s, QDict *req)
{
g_assert(req);
g_debug("processing command");
- qmp_dispatch(QOBJECT(req), NULL, dispatch_return_cb, s);
+ qmp_dispatch(&ga_state->client, QOBJECT(req), NULL);
}
/* handle requests/control events coming in over the channel */
@@ -1284,6 +1285,7 @@ static int run_agent(GAState *s, GAConfig *config)
ga_command_state_init_all(s->command_state);
json_message_parser_init(&s->parser, process_event);
ga_state = s;
+ qmp_client_init(&s->client, dispatch_return_cb);
#ifndef _WIN32
if (!register_signal_handlers()) {
g_critical("failed to register signal handlers");
@@ -1379,7 +1381,7 @@ end:
}
g_free(s->pstate_filepath);
g_free(s->state_filepath_isfrozen);
-
+ qmp_client_destroy(&s->client);
if (config->daemonize) {
unlink(config->pid_filepath);
}
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 88cdd3a..fd559f2 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -85,7 +85,7 @@ __org_qemu_x_Union1 *qmp___org_qemu_x_command(__org_qemu_x_EnumList *a,
return ret;
}
-static void dispatch_cmd_return(QObject *resp, void *opaque)
+static void dispatch_cmd_return(QmpClient *client, QObject *resp)
{
assert(resp != NULL);
assert(!qdict_haskey(qobject_to_qdict(resp), "error"));
@@ -94,16 +94,20 @@ static void dispatch_cmd_return(QObject *resp, void *opaque)
/* test commands with no input and no return value */
static void test_dispatch_cmd(void)
{
+ QmpClient client;
QDict *req = qdict_new();
- qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd")));
+ qmp_client_init(&client, dispatch_cmd_return);
- qmp_dispatch(QOBJECT(req), NULL, dispatch_cmd_return, NULL);
+ qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd")));
+ qmp_dispatch(&client, QOBJECT(req), NULL);
QDECREF(req);
+
+ qmp_client_destroy(&client);
}
-static void dispatch_cmd_error_return(QObject *resp, void *opaque)
+static void dispatch_cmd_error_return(QmpClient *client, QObject *resp)
{
assert(resp != NULL);
assert(qdict_haskey(qobject_to_qdict(resp), "error"));
@@ -112,13 +116,15 @@ static void dispatch_cmd_error_return(QObject *resp, void *opaque)
/* test commands that return an error due to invalid parameters */
static void test_dispatch_cmd_failure(void)
{
+ QmpClient client;
QDict *req = qdict_new();
QDict *args = qdict_new();
- QObject *resp;
+
+ qmp_client_init(&client, dispatch_cmd_error_return);
qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd2")));
- qmp_dispatch(QOBJECT(req), NULL, dispatch_cmd_error_return, NULL);
+ qmp_dispatch(&client, QOBJECT(req), NULL);
QDECREF(req);
/* check that with extra arguments it throws an error */
@@ -128,13 +134,16 @@ static void test_dispatch_cmd_failure(void)
qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd")));
- qmp_dispatch(QOBJECT(req), NULL, dispatch_cmd_error_return, NULL);
+ qmp_dispatch(&client, QOBJECT(req), NULL);
+
QDECREF(req);
+
+ qmp_client_destroy(&client);
}
static QObject *dispatch_ret;
-static void qmp_dispatch_return(QObject *resp_obj, void *opaque)
+static void qmp_dispatch_return(QmpClient *client, QObject *resp_obj)
{
QDict *resp = qobject_to_qdict(resp_obj);
assert(resp && !qdict_haskey(resp, "error"));
@@ -146,8 +155,12 @@ static void qmp_dispatch_return(QObject *resp_obj, void *opaque)
static QObject *test_qmp_dispatch(QDict *req)
{
QObject *ret;
+ QmpClient client;
+
+ qmp_client_init(&client, qmp_dispatch_return);
+ qmp_dispatch(&client, QOBJECT(req), NULL);
+ qmp_client_destroy(&client);
- qmp_dispatch(QOBJECT(req), NULL, qmp_dispatch_return, NULL);
ret = dispatch_ret;
dispatch_ret = NULL;
@@ -266,7 +279,6 @@ static void test_dealloc_partial(void)
qapi_free_UserDefTwo(ud2);
}
-
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 780b3e2..3287676 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -17,14 +17,23 @@
#include "qapi/qmp/qobject.h"
#include "qapi/qmp/qdict.h"
-typedef void (QmpDispatchReturn) (QObject *rsp, void *opaque);
+typedef struct QmpClient QmpClient;
+
+typedef void (QmpDispatchReturn) (QmpClient *client, QObject *rsp);
typedef struct QmpReturn {
QDict *rsp;
- QmpDispatchReturn *return_cb;
- void *opaque;
+ QmpClient *client;
+
+ QLIST_ENTRY(QmpReturn) link;
} QmpReturn;
+struct QmpClient {
+ QmpDispatchReturn *return_cb;
+
+ QLIST_HEAD(, QmpReturn) pending;
+};
+
typedef void (QmpCommandFunc)(QDict *, QObject **, Error **);
typedef enum QmpCommandOptions
@@ -46,8 +55,9 @@ void qmp_register_command(const char *name, QmpCommandFunc *fn,
QmpCommandOptions options);
void qmp_unregister_command(const char *name);
QmpCommand *qmp_find_command(const char *name);
-void qmp_dispatch(QObject *request, QDict *rsp,
- QmpDispatchReturn *return_cb, void *opaque);
+void qmp_client_init(QmpClient *client, QmpDispatchReturn *return_cb);
+void qmp_client_destroy(QmpClient *client);
+void qmp_dispatch(QmpClient *client, QObject *request, QDict *rsp);
void qmp_disable_command(const char *name);
void qmp_enable_command(const char *name);
bool qmp_command_is_enabled(const QmpCommand *cmd);
@@ -61,7 +71,7 @@ void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque);
* qmp_return{_error}:
*
* Construct the command reply, and call the
- * return_cb() associated with the dispatch.
+ * return_cb() associated with the QmpClient.
*
* Finally, free the QmpReturn.
*/
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 06/24] qmp: add qmp_return_is_cancelled()
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (4 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 05/24] qmp: add QmpClient Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 07/24] qmp: introduce async command type Marc-André Lureau
` (17 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
If the client is gone, no need to return. The async handler can use this
information to avoid unnecessary work and exit earlier.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qapi/qmp-dispatch.c | 10 ++++++++++
include/qapi/qmp/dispatch.h | 8 ++++++++
2 files changed, 18 insertions(+)
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index c8821b7..0c4022f 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -155,6 +155,16 @@ void qmp_return_error(QmpReturn *qret, Error *err)
do_qmp_return(qret);
}
+bool qmp_return_is_cancelled(QmpReturn *qret)
+{
+ if (!qret->client) {
+ qmp_return_free(qret);
+ return true;
+ }
+
+ return false;
+}
+
void qmp_client_init(QmpClient *client, QmpDispatchReturn *return_cb)
{
client->return_cb = return_cb;
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 3287676..bc64d4e 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -78,4 +78,12 @@ void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque);
void qmp_return(QmpReturn *qret, QObject *cmd_rsp);
void qmp_return_error(QmpReturn *qret, Error *err);
+/*
+ * qmp_return_is_cancelled:
+ *
+ * Return true if the QmpReturn is cancelled, and free the QmpReturn
+ * in this case.
+ */
+bool qmp_return_is_cancelled(QmpReturn *qret);
+
#endif
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 07/24] qmp: introduce async command type
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (5 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 06/24] qmp: add qmp_return_is_cancelled() Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 08/24] qapi: ignore top-level 'id' field Marc-André Lureau
` (16 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
Qemu used to have command types, but it was removed in
42a502a7a60632234f0dd5028924926a7eac6c94. Add it back, and add a new
type of command, QmpCommandFuncAsync: they can return later thanks to
QmpReturn. This commit introduces the new types and register functions.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qapi/qmp-dispatch.c | 21 ++++++++++++++-------
qapi/qmp-registry.c | 25 ++++++++++++++++++++++---
include/qapi/qmp/dispatch.h | 14 +++++++++++++-
3 files changed, 49 insertions(+), 11 deletions(-)
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 0c4022f..7e1ef54 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -95,13 +95,20 @@ static QObject *do_qmp_dispatch(QObject *request, QmpReturn *qret, Error **errp)
QINCREF(args);
}
- cmd->fn(args, &ret, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- } else if (cmd->options & QCO_NO_SUCCESS_RESP) {
- g_assert(!ret);
- } else if (!ret) {
- ret = QOBJECT(qdict_new());
+ switch (cmd->type) {
+ case QCT_NORMAL:
+ cmd->fn(args, &ret, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ } else if (cmd->options & QCO_NO_SUCCESS_RESP) {
+ g_assert(!ret);
+ } else if (!ret) {
+ ret = QOBJECT(qdict_new());
+ }
+ break;
+ case QCT_ASYNC:
+ cmd->fn_async(args, qret);
+ break;
}
QDECREF(args);
diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
index e805368..52b5f4c 100644
--- a/qapi/qmp-registry.c
+++ b/qapi/qmp-registry.c
@@ -18,16 +18,35 @@
static QTAILQ_HEAD(QmpCommandList, QmpCommand) qmp_commands =
QTAILQ_HEAD_INITIALIZER(qmp_commands);
-void qmp_register_command(const char *name, QmpCommandFunc *fn,
- QmpCommandOptions options)
+static QmpCommand *qmp_command_new(const char *name,
+ QmpCommandOptions options)
{
QmpCommand *cmd = g_malloc0(sizeof(*cmd));
cmd->name = name;
- cmd->fn = fn;
cmd->enabled = true;
cmd->options = options;
QTAILQ_INSERT_TAIL(&qmp_commands, cmd, node);
+
+ return cmd;
+}
+
+void qmp_register_command(const char *name, QmpCommandFunc *fn,
+ QmpCommandOptions options)
+{
+ QmpCommand *cmd = qmp_command_new(name, options);
+
+ cmd->type = QCT_NORMAL;
+ cmd->fn = fn;
+}
+
+void qmp_register_async_command(const char *name, QmpCommandFuncAsync *fn,
+ QmpCommandOptions options)
+{
+ QmpCommand *cmd = qmp_command_new(name, options);
+
+ cmd->type = QCT_ASYNC;
+ cmd->fn_async = fn;
}
void qmp_unregister_command(const char *name)
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index bc64d4e..e13e381 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -35,6 +35,12 @@ struct QmpClient {
};
typedef void (QmpCommandFunc)(QDict *, QObject **, Error **);
+typedef void (QmpCommandFuncAsync)(QDict *, QmpReturn *);
+
+typedef enum QmpCommandType {
+ QCT_NORMAL,
+ QCT_ASYNC,
+} QmpCommandType;
typedef enum QmpCommandOptions
{
@@ -44,8 +50,12 @@ typedef enum QmpCommandOptions
typedef struct QmpCommand
{
+ QmpCommandType type;
const char *name;
- QmpCommandFunc *fn;
+ union {
+ QmpCommandFunc *fn;
+ QmpCommandFuncAsync *fn_async;
+ };
QmpCommandOptions options;
QTAILQ_ENTRY(QmpCommand) node;
bool enabled;
@@ -53,6 +63,8 @@ typedef struct QmpCommand
void qmp_register_command(const char *name, QmpCommandFunc *fn,
QmpCommandOptions options);
+void qmp_register_async_command(const char *name, QmpCommandFuncAsync *fn,
+ QmpCommandOptions options);
void qmp_unregister_command(const char *name);
QmpCommand *qmp_find_command(const char *name);
void qmp_client_init(QmpClient *client, QmpDispatchReturn *return_cb);
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 08/24] qapi: ignore top-level 'id' field
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (6 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 07/24] qmp: introduce async command type Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 09/24] qmp: take 'id' from request Marc-André Lureau
` (15 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
The following patch copies the 'id' to the reply, without touching the
original request (which should eventually be const later) so it must
pass the check function without error.
'id' is documented as part of qmp-spec, it is valid at top-level entry.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qapi/qmp-dispatch.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 7e1ef54..1fedc97 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -48,6 +48,8 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp)
return NULL;
}
has_exec_key = true;
+ } else if (!strcmp(arg_name, "id")) {
+ /* top-level 'id' is accepted */
} else if (strcmp(arg_name, "arguments")) {
error_setg(errp, QERR_QMP_EXTRA_MEMBER, arg_name);
return NULL;
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 09/24] qmp: take 'id' from request
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (7 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 08/24] qapi: ignore top-level 'id' field Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 10/24] qmp: check that async command have an 'id' Marc-André Lureau
` (14 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
Copy 'id' from request to reply dict. This can be done earlier, such as
done by the monitor (because the qemu monitor may reply directly without
qmp_dispatch), but is now done as well in qmp_dispatch() as convenience
for other users such as QGA and tests.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qapi/qmp-dispatch.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 1fedc97..c42fb87 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -199,7 +199,8 @@ void qmp_dispatch(QmpClient *client, QObject *request, QDict *rsp)
{
Error *err = NULL;
QmpReturn *qret = g_new0(QmpReturn, 1);
- QObject *ret;
+ QObject *ret, *id;
+ QDict *req;
assert(client);
@@ -207,6 +208,13 @@ void qmp_dispatch(QmpClient *client, QObject *request, QDict *rsp)
qret->client = client;
QLIST_INSERT_HEAD(&client->pending, qret, link);
+ req = qobject_to_qdict(request);
+ id = qdict_get(req, "id");
+ if (id) {
+ qobject_incref(id);
+ qdict_put_obj(qret->rsp, "id", id);
+ }
+
ret = do_qmp_dispatch(request, qret, &err);
if (err) {
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 10/24] qmp: check that async command have an 'id'
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (8 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 09/24] qmp: take 'id' from request Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 11/24] scripts: learn 'async' qapi commands Marc-André Lureau
` (13 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
The async support mandates that request have an 'id' (see documentation
in following patch).
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qapi/qmp-dispatch.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index c42fb87..b6a1feb 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -109,6 +109,11 @@ static QObject *do_qmp_dispatch(QObject *request, QmpReturn *qret, Error **errp)
}
break;
case QCT_ASYNC:
+ if (!qdict_haskey(qret->rsp, "id")) {
+ error_setg(errp, "An async command requires an 'id'");
+ break;
+ }
+
cmd->fn_async(args, qret);
break;
}
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 11/24] scripts: learn 'async' qapi commands
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (9 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 10/24] qmp: check that async command have an 'id' Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 12/24] tests: add dispatch async tests Marc-André Lureau
` (12 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
Commands with the 'async' key will be registered as async type (see
related commit), and will allow a synchronous (in scope callback) or
asynchronous return (when ready, in idle etc) by keeping the given
QmpReturn and calling qmp_return function later.
Ex:
{ 'command': 'foo-async,
'data': {'arg': 'str'},
'returns': 'Foo',
'async': true }
generates the following marshaller:
void qmp_marshal_foo_async(QDict *args, QmpReturn *qret)
{
Error *err = NULL;
Visitor *v;
q_obj_foo_async_arg arg = {0};
v = qmp_input_visitor_new(QOBJECT(args), true);
visit_start_struct(v, NULL, NULL, 0, &err);
if (err) {
goto out;
}
visit_type_q_obj_foo_async_arg_members(v, &arg, &err);
if (!err) {
visit_check_struct(v, &err);
}
visit_end_struct(v, NULL);
if (err) {
goto out;
}
qmp_foo_async(arg.arg, qret);
out:
if (err) {
qmp_return_error(qret, err);
}
visit_free(v);
v = qapi_dealloc_visitor_new();
visit_start_struct(v, NULL, NULL, 0, NULL);
visit_type_q_obj_foo_async_arg_members(v, &arg, NULL);
visit_end_struct(v, NULL);
visit_free(v);
}
and return helper:
void qmp_foo_async_return(QmpReturn *qret, Foo *ret_in)
{
Error *err = NULL;
QObject *ret_out = NULL;
qmp_marshal_output_Foo(ret_in, &ret_out, &err);
if (err) {
qmp_return_error(qret, err);
} else {
qmp_return(qret, ret_out);
}
}
The dispatched function may call the return helper within the calling
scope or delay the return. To return an error, it should call
qmp_return_error().
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/Makefile.include | 1 +
scripts/qapi-commands.py | 139 +++++++++++++++++++++++++++++++++--------
scripts/qapi-introspect.py | 7 ++-
scripts/qapi.py | 14 +++--
qapi-schema.json | 12 ++++
qapi/introspect.json | 2 +-
tests/qapi-schema/async.err | 0
tests/qapi-schema/async.exit | 1 +
tests/qapi-schema/async.json | 1 +
tests/qapi-schema/async.out | 7 +++
tests/qapi-schema/test-qapi.py | 7 ++-
11 files changed, 155 insertions(+), 36 deletions(-)
create mode 100644 tests/qapi-schema/async.err
create mode 100644 tests/qapi-schema/async.exit
create mode 100644 tests/qapi-schema/async.json
create mode 100644 tests/qapi-schema/async.out
diff --git a/tests/Makefile.include b/tests/Makefile.include
index b929c48..39ea14f 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -334,6 +334,7 @@ qapi-schema += args-member-unknown.json
qapi-schema += args-name-clash.json
qapi-schema += args-union.json
qapi-schema += args-unknown.json
+qapi-schema += async.json
qapi-schema += bad-base.json
qapi-schema += bad-data.json
qapi-schema += bad-ident.json
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 2f603b0..8454f22 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -16,18 +16,30 @@ from qapi import *
import re
-def gen_command_decl(name, arg_type, boxed, ret_type):
- return mcgen('''
+def gen_command_decl(name, arg_type, boxed, ret_type, async):
+ if async:
+ extra = "QmpReturn *qret"
+ else:
+ extra = 'Error **errp'
+
+ if async:
+ return mcgen('''
+void qmp_%(name)s(%(params)s);
+void qmp_%(name)s_return(QmpReturn *qret%(c_type)s);
+''',
+ c_type=(", " + ret_type.c_type() if ret_type else ""),
+ name=c_name(name),
+ params=gen_params(arg_type, boxed, extra))
+ else:
+ return mcgen('''
%(c_type)s qmp_%(c_name)s(%(params)s);
''',
- c_type=(ret_type and ret_type.c_type()) or 'void',
- c_name=c_name(name),
- params=gen_params(arg_type, boxed, 'Error **errp'))
+ c_type=(ret_type and ret_type.c_type()) or 'void',
+ c_name=c_name(name),
+ params=gen_params(arg_type, boxed, extra))
-def gen_call(name, arg_type, boxed, ret_type):
- ret = ''
-
+def gen_argstr(arg_type, boxed):
argstr = ''
if boxed:
assert arg_type and not arg_type.is_empty()
@@ -39,6 +51,13 @@ def gen_call(name, arg_type, boxed, ret_type):
argstr += 'arg.has_%s, ' % c_name(memb.name)
argstr += 'arg.%s, ' % c_name(memb.name)
+ return argstr
+
+
+def gen_call(name, arg_type, boxed, ret_type):
+ ret = ''
+
+ argstr = gen_argstr(arg_type, boxed)
lhs = ''
if ret_type:
lhs = 'retval = '
@@ -60,6 +79,50 @@ def gen_call(name, arg_type, boxed, ret_type):
return ret
+def gen_async_call(name, arg_type, boxed):
+ argstr = gen_argstr(arg_type, boxed)
+
+ push_indent()
+ ret = mcgen('''
+
+qmp_%(c_name)s(%(args)sqret);
+''',
+ c_name=c_name(name), args=argstr)
+
+ pop_indent()
+ return ret
+
+
+def gen_async_return(name, ret_type):
+ if ret_type:
+ return mcgen('''
+
+void qmp_%(c_name)s_return(QmpReturn *qret, %(ret_type)s ret_in)
+{
+ Error *err = NULL;
+ QObject *ret_out = NULL;
+
+ qmp_marshal_output_%(ret_c_name)s(ret_in, &ret_out, &err);
+
+ if (err) {
+ qmp_return_error(qret, err);
+ } else {
+ qmp_return(qret, ret_out);
+ }
+}
+''',
+ c_name=c_name(name),
+ ret_type=ret_type.c_type(), ret_c_name=ret_type.c_name())
+ else:
+ return mcgen('''
+
+void qmp_%(c_name)s_return(QmpReturn *qret)
+{
+ qmp_return(qret, QOBJECT(qdict_new()));
+}
+''',
+ c_name=c_name(name))
+
def gen_marshal_output(ret_type):
return mcgen('''
@@ -83,18 +146,22 @@ static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,
c_type=ret_type.c_type(), c_name=ret_type.c_name())
-def gen_marshal_proto(name):
- return 'void qmp_marshal_%s(QDict *args, QObject **ret, Error **errp)' % c_name(name)
+def gen_marshal_proto(name, async):
+ if async:
+ tmpl = 'void qmp_marshal_%s(QDict *args, QmpReturn *qret)'
+ else:
+ tmpl = 'void qmp_marshal_%s(QDict *args, QObject **ret, Error **errp)'
+ return tmpl % c_name(name)
-def gen_marshal_decl(name):
+def gen_marshal_decl(name, async):
return mcgen('''
%(proto)s;
''',
- proto=gen_marshal_proto(name))
+ proto=gen_marshal_proto(name, async))
-def gen_marshal(name, arg_type, boxed, ret_type):
+def gen_marshal(name, arg_type, boxed, ret_type, async):
have_args = arg_type and not arg_type.is_empty()
ret = mcgen('''
@@ -103,9 +170,9 @@ def gen_marshal(name, arg_type, boxed, ret_type):
{
Error *err = NULL;
''',
- proto=gen_marshal_proto(name))
+ proto=gen_marshal_proto(name, async))
- if ret_type:
+ if ret_type and not async:
ret += mcgen('''
%(c_type)s retval;
''',
@@ -152,12 +219,28 @@ def gen_marshal(name, arg_type, boxed, ret_type):
}
''')
- ret += gen_call(name, arg_type, boxed, ret_type)
+ if async:
+ ret += gen_async_call(name, arg_type, boxed)
+ else:
+ ret += gen_call(name, arg_type, boxed, ret_type)
ret += mcgen('''
out:
+''')
+
+ if async:
+ ret += mcgen('''
+ if (err) {
+ qmp_return_error(qret, err);
+ }
+''')
+ else:
+ ret += mcgen('''
error_propagate(errp, err);
+''')
+
+ ret += mcgen('''
visit_free(v);
''')
@@ -192,15 +275,17 @@ out:
return ret
-def gen_register_command(name, success_response):
+def gen_register_command(name, success_response, async):
options = 'QCO_NO_OPTIONS'
if not success_response:
options = 'QCO_NO_SUCCESS_RESP'
-
+ func = 'qmp_register_command'
+ if async:
+ func = 'qmp_register_async_command'
ret = mcgen('''
- qmp_register_command("%(name)s", qmp_marshal_%(c_name)s, %(opts)s);
+ %(func)s("%(name)s", qmp_marshal_%(c_name)s, %(opts)s);
''',
- name=name, c_name=c_name(name),
+ func=func, name=name, c_name=c_name(name),
opts=options)
return ret
@@ -239,16 +324,19 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor):
self._visited_ret_types = None
def visit_command(self, name, info, arg_type, ret_type,
- gen, success_response, boxed):
+ gen, success_response, boxed, async):
if not gen:
return
- self.decl += gen_command_decl(name, arg_type, boxed, ret_type)
+ self.decl += gen_command_decl(name, arg_type, boxed,
+ ret_type, async)
if ret_type and ret_type not in self._visited_ret_types:
self._visited_ret_types.add(ret_type)
self.defn += gen_marshal_output(ret_type)
- self.decl += gen_marshal_decl(name)
- self.defn += gen_marshal(name, arg_type, boxed, ret_type)
- self._regy += gen_register_command(name, success_response)
+ if async:
+ self.defn += gen_async_return(name, ret_type)
+ self.decl += gen_marshal_decl(name, async)
+ self.defn += gen_marshal(name, arg_type, boxed, ret_type, async)
+ self._regy += gen_register_command(name, success_response, async)
(input_file, output_dir, do_c, do_h, prefix, opts) = parse_command_line()
@@ -306,6 +394,7 @@ fdef.write(mcgen('''
fdecl.write(mcgen('''
#include "%(prefix)sqapi-types.h"
#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/dispatch.h"
#include "qapi/error.h"
''',
diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
index 541644e..f8a854d 100644
--- a/scripts/qapi-introspect.py
+++ b/scripts/qapi-introspect.py
@@ -28,6 +28,8 @@ def to_json(obj, level=0):
to_json(obj[key], level + 1))
for key in sorted(obj.keys())]
ret = '{' + ', '.join(elts) + '}'
+ elif isinstance(obj, bool):
+ ret = 'true' if obj else 'false'
else:
assert False # not implemented
if level == 1:
@@ -154,12 +156,13 @@ const char %(c_name)s[] = %(c_string)s;
for m in variants.variants]})
def visit_command(self, name, info, arg_type, ret_type,
- gen, success_response, boxed):
+ gen, success_response, boxed, async):
arg_type = arg_type or self._schema.the_empty_object_type
ret_type = ret_type or self._schema.the_empty_object_type
self._gen_json(name, 'command',
{'arg-type': self._use_type(arg_type),
- 'ret-type': self._use_type(ret_type)})
+ 'ret-type': self._use_type(ret_type),
+ 'async': async})
def visit_event(self, name, info, arg_type, boxed):
arg_type = arg_type or self._schema.the_empty_object_type
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 21bc32f..9fc806c 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -737,7 +737,8 @@ def check_exprs(exprs):
add_struct(expr, info)
elif 'command' in expr:
check_keys(expr_elem, 'command', [],
- ['data', 'returns', 'gen', 'success-response', 'boxed'])
+ ['data', 'returns', 'gen', 'success-response', 'boxed',
+ 'async'])
add_name(expr['command'], info, 'command')
elif 'event' in expr:
check_keys(expr_elem, 'event', [], ['data', 'boxed'])
@@ -838,7 +839,7 @@ class QAPISchemaVisitor(object):
pass
def visit_command(self, name, info, arg_type, ret_type,
- gen, success_response, boxed):
+ gen, success_response, boxed, async):
pass
def visit_event(self, name, info, arg_type, boxed):
@@ -1181,7 +1182,7 @@ class QAPISchemaAlternateType(QAPISchemaType):
class QAPISchemaCommand(QAPISchemaEntity):
def __init__(self, name, info, arg_type, ret_type, gen, success_response,
- boxed):
+ boxed, async):
QAPISchemaEntity.__init__(self, name, info)
assert not arg_type or isinstance(arg_type, str)
assert not ret_type or isinstance(ret_type, str)
@@ -1192,6 +1193,7 @@ class QAPISchemaCommand(QAPISchemaEntity):
self.gen = gen
self.success_response = success_response
self.boxed = boxed
+ self.async = async
def check(self, schema):
if self._arg_type_name:
@@ -1216,7 +1218,8 @@ class QAPISchemaCommand(QAPISchemaEntity):
def visit(self, visitor):
visitor.visit_command(self.name, self.info,
self.arg_type, self.ret_type,
- self.gen, self.success_response, self.boxed)
+ self.gen, self.success_response, self.boxed,
+ self.async)
class QAPISchemaEvent(QAPISchemaEntity):
@@ -1420,6 +1423,7 @@ class QAPISchema(object):
data = expr.get('data')
rets = expr.get('returns')
gen = expr.get('gen', True)
+ async = expr.get('async', False)
success_response = expr.get('success-response', True)
boxed = expr.get('boxed', False)
if isinstance(data, OrderedDict):
@@ -1429,7 +1433,7 @@ class QAPISchema(object):
assert len(rets) == 1
rets = self._make_array_type(rets[0], info)
self._def_entity(QAPISchemaCommand(name, info, data, rets, gen,
- success_response, boxed))
+ success_response, boxed, async))
def _def_event(self, expr, info):
name = expr['event']
diff --git a/qapi-schema.json b/qapi-schema.json
index c3dcf11..93b2929 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -20,6 +20,18 @@
# QAPI introspection
{ 'include': 'qapi/introspect.json' }
+##
+# @QMPCapability
+#
+# QMP protocol capabilities
+#
+# @async: enables async messages
+#
+# Since: 2.8
+##
+{ 'enum': 'QMPCapability',
+ 'data': ['async'] }
+
##
# @qmp_capabilities:
#
diff --git a/qapi/introspect.json b/qapi/introspect.json
index 3fd81fb..9e266b1 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -263,7 +263,7 @@
# Since: 2.5
##
{ 'struct': 'SchemaInfoCommand',
- 'data': { 'arg-type': 'str', 'ret-type': 'str' } }
+ 'data': { 'arg-type': 'str', 'ret-type': 'str', 'async': 'bool' } }
##
# @SchemaInfoEvent
diff --git a/tests/qapi-schema/async.err b/tests/qapi-schema/async.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/async.exit b/tests/qapi-schema/async.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/async.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/async.json b/tests/qapi-schema/async.json
new file mode 100644
index 0000000..3b719e3
--- /dev/null
+++ b/tests/qapi-schema/async.json
@@ -0,0 +1 @@
+{ 'command': 'screendump-async', 'data': {'filename': 'str'}, 'async': true }
diff --git a/tests/qapi-schema/async.out b/tests/qapi-schema/async.out
new file mode 100644
index 0000000..d9d013c
--- /dev/null
+++ b/tests/qapi-schema/async.out
@@ -0,0 +1,7 @@
+enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
+ prefix QTYPE
+object q_empty
+object q_obj_screendump-async-arg
+ member filename: str optional=False
+command screendump-async q_obj_screendump-async-arg -> None
+ gen=True success_response=True boxed=False async=True
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index ef74e2c..c030b65 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -36,11 +36,12 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
self._print_variants(variants)
def visit_command(self, name, info, arg_type, ret_type,
- gen, success_response, boxed):
+ gen, success_response, boxed, async):
print 'command %s %s -> %s' % \
(name, arg_type and arg_type.name, ret_type and ret_type.name)
- print ' gen=%s success_response=%s boxed=%s' % \
- (gen, success_response, boxed)
+ print ' gen=%s success_response=%s boxed=%s%s' % \
+ (gen, success_response, boxed,
+ ' async=True' if async else '')
def visit_event(self, name, info, arg_type, boxed):
print 'event %s %s' % (name, arg_type and arg_type.name)
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 12/24] tests: add dispatch async tests
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (10 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 11/24] scripts: learn 'async' qapi commands Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 13/24] monitor: add 'async' capability Marc-André Lureau
` (11 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
Add a few tests to check:
- that async dispatch works
- destroying the client with pending requests
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/test-qmp-commands.c | 109 ++++++++++++++++++++++++++++++++
tests/qapi-schema/qapi-schema-test.json | 4 ++
tests/qapi-schema/qapi-schema-test.out | 5 ++
3 files changed, 118 insertions(+)
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index fd559f2..cca5555 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -85,6 +85,36 @@ __org_qemu_x_Union1 *qmp___org_qemu_x_command(__org_qemu_x_EnumList *a,
return ret;
}
+static GMainLoop *loop;
+
+typedef struct AsyncRet {
+ QmpReturn *qret;
+ UserDefB *ret;
+} AsyncRet;
+
+static gboolean qmp_user_async_idle(gpointer data)
+{
+ struct AsyncRet *async = (struct AsyncRet *)data;
+
+ qmp_user_async_return(async->qret, async->ret);
+ g_free(async);
+
+ g_main_loop_quit(loop);
+
+ return FALSE;
+}
+
+void qmp_user_async(int64_t a, bool has_b, int64_t b, QmpReturn *qret)
+{
+ AsyncRet *async = g_new0(AsyncRet, 1);
+
+ async->ret = g_new0(UserDefB, 1);
+ async->ret->intb = a + (has_b ? b : 0);
+ async->qret = qret;
+
+ g_idle_add(qmp_user_async_idle, async);
+}
+
static void dispatch_cmd_return(QmpClient *client, QObject *resp)
{
assert(resp != NULL);
@@ -142,14 +172,19 @@ static void test_dispatch_cmd_failure(void)
}
static QObject *dispatch_ret;
+static char *ret_id;
static void qmp_dispatch_return(QmpClient *client, QObject *resp_obj)
{
QDict *resp = qobject_to_qdict(resp_obj);
+
assert(resp && !qdict_haskey(resp, "error"));
dispatch_ret = qdict_get(resp, "return");
assert(dispatch_ret);
qobject_incref(dispatch_ret);
+
+ g_free(ret_id);
+ ret_id = g_strdup(qdict_get_try_str(resp, "id"));
}
static QObject *test_qmp_dispatch(QDict *req)
@@ -216,6 +251,76 @@ static void test_dispatch_cmd_io(void)
QDECREF(req);
}
+static void test_dispatch_cmd_async(void)
+{
+ QmpClient client;
+ QDict *dret, *req = qdict_new();
+ QDict *args = qdict_new();
+
+ loop = g_main_loop_new(NULL, FALSE);
+ qmp_client_init(&client, qmp_dispatch_return);
+
+ qdict_put(args, "a", qint_from_int(99));
+ qdict_put(req, "arguments", args);
+ qdict_put(req, "id", qstring_from_str("foo99"));
+ qdict_put(req, "execute", qstring_from_str("user-async"));
+
+ qmp_dispatch(&client, QOBJECT(req), NULL);
+ assert(!dispatch_ret);
+
+ g_main_loop_run(loop);
+ g_main_loop_unref(loop);
+
+ g_assert_cmpstr(ret_id, ==, "foo99");
+ dret = qobject_to_qdict(dispatch_ret);
+ assert(qdict_get_int(dret, "intb") == 99);
+ QDECREF(dret);
+ dispatch_ret = NULL;
+
+ qmp_client_destroy(&client);
+ QDECREF(req);
+}
+
+static void test_destroy_pending_async(void)
+{
+ QmpClient client;
+ QDict *req = qdict_new();
+ QDict *args = qdict_new();
+ QmpReturn *r;
+ int npending = 0;
+
+ loop = g_main_loop_new(NULL, FALSE);
+ qmp_client_init(&client, qmp_dispatch_return);
+
+ qdict_put(args, "a", qint_from_int(99));
+ qdict_put(req, "arguments", args);
+ qdict_put(req, "id", qstring_from_str("foo99"));
+ qdict_put(req, "execute", qstring_from_str("user-async"));
+
+ qmp_dispatch(&client, QOBJECT(req), NULL);
+ qmp_dispatch(&client, QOBJECT(req), NULL);
+ assert(!dispatch_ret);
+ QDECREF(req);
+
+ npending = 0;
+ QLIST_FOREACH(r, &client.pending, link) {
+ npending++;
+ }
+
+ g_assert_cmpint(npending, ==, 2);
+
+ /* destroy with pending async */
+ qmp_client_destroy(&client);
+
+ while (g_main_context_pending(NULL)) {
+ g_main_loop_run(loop);
+ /* no return since the client is gone */
+ assert(!dispatch_ret);
+ }
+
+ g_main_loop_unref(loop);
+}
+
/* test generated dealloc functions for generated types */
static void test_dealloc_types(void)
{
@@ -286,11 +391,15 @@ int main(int argc, char **argv)
g_test_add_func("/qmp/dispatch_cmd", test_dispatch_cmd);
g_test_add_func("/qmp/dispatch_cmd_failure", test_dispatch_cmd_failure);
g_test_add_func("/qmp/dispatch_cmd_io", test_dispatch_cmd_io);
+ g_test_add_func("/qmp/dispatch_cmd_async", test_dispatch_cmd_async);
+ g_test_add_func("/qmp/destroy_pending_async", test_destroy_pending_async);
g_test_add_func("/qmp/dealloc_types", test_dealloc_types);
g_test_add_func("/qmp/dealloc_partial", test_dealloc_partial);
module_call_init(MODULE_INIT_QAPI);
g_test_run();
+ g_free(ret_id);
+
return 0;
}
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 1719463..a522fab 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -130,6 +130,10 @@
{ 'command': 'boxed-struct', 'boxed': true, 'data': 'UserDefZero' }
{ 'command': 'boxed-union', 'data': 'UserDefNativeListUnion', 'boxed': true }
+# async
+{ 'command': 'user-async', 'data': { 'a': 'int', '*b': 'int' },
+ 'returns': 'UserDefB', 'async': true }
+
# For testing integer range flattening in opts-visitor. The following schema
# corresponds to the option format:
#
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 9d99c4e..61b75fd 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -219,11 +219,16 @@ object q_obj_uint64List-wrapper
member data: uint64List optional=False
object q_obj_uint8List-wrapper
member data: uint8List optional=False
+object q_obj_user-async-arg
+ member a: int optional=False
+ member b: int optional=True
object q_obj_user_def_cmd1-arg
member ud1a: UserDefOne optional=False
object q_obj_user_def_cmd2-arg
member ud1a: UserDefOne optional=False
member ud1b: UserDefOne optional=True
+command user-async q_obj_user-async-arg -> UserDefB
+ gen=True success_response=True boxed=False async=True
command user_def_cmd None -> None
gen=True success_response=True boxed=False
command user_def_cmd0 Empty2 -> Empty2
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 13/24] monitor: add 'async' capability
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (11 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 12/24] tests: add dispatch async tests Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 14/24] monitor: add !qmp pre-conditions Marc-André Lureau
` (10 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
Declare that the server supports async.
Check if the client supports it: the following patch will suspend the
qmp monitor if an async command is ongoing.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
monitor.c | 20 +++++++++++++++++---
qapi-schema.json | 4 ++--
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/monitor.c b/monitor.c
index d7baa86..4a20d81 100644
--- a/monitor.c
+++ b/monitor.c
@@ -173,6 +173,7 @@ typedef struct {
* mode.
*/
bool in_command_mode; /* are we in command mode? */
+ bool has_async; /* the client has async capability */
QmpClient client;
} MonitorQMP;
@@ -570,9 +571,22 @@ static void monitor_qapi_event_init(void)
qmp_event_set_func_emit(monitor_qapi_event_queue);
}
-void qmp_qmp_capabilities(Error **errp)
+void qmp_qmp_capabilities(bool has_capabilities,
+ QMPCapabilityList *capabilities, Error **errp)
{
+ bool has_async = false;
+
+ if (has_capabilities) {
+ while (capabilities) {
+ if (capabilities->value == QMP_CAPABILITY_ASYNC) {
+ has_async = true;
+ }
+ capabilities = capabilities->next;
+ }
+ }
+
cur_mon->qmp.in_command_mode = true;
+ cur_mon->qmp.has_async = has_async;
}
static void handle_hmp_command(Monitor *mon, const char *cmdline);
@@ -3852,8 +3866,8 @@ static QObject *get_qmp_greeting(void)
qmp_marshal_query_version(NULL, &ver, NULL);
- return qobject_from_jsonf("{'QMP': {'version': %p, 'capabilities': []}}",
- ver);
+ return qobject_from_jsonf("{'QMP': {'version': %p, 'capabilities': ["
+ "'async']}}", ver);
}
static void monitor_qmp_event(void *opaque, int event)
diff --git a/qapi-schema.json b/qapi-schema.json
index 93b2929..95d3c72 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -37,7 +37,7 @@
#
# Enable QMP capabilities.
#
-# Arguments: None.
+# @capabilities: #optional an array of QMPCapability (since 2.8)
#
# Example:
#
@@ -51,7 +51,7 @@
# Since: 0.13
#
##
-{ 'command': 'qmp_capabilities' }
+{ 'command': 'qmp_capabilities', 'data': { '*capabilities': ['QMPCapability'] } }
##
# @LostTickPolicy:
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 14/24] monitor: add !qmp pre-conditions
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (12 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 13/24] monitor: add 'async' capability Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 15/24] monitor: suspend when running async and client has no async Marc-André Lureau
` (9 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
It's not always obvious whether a function is meant to be used only with
HMP or QMP. Add a few pre-conditions to document this aspect and
eventually catch run-time bugs.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
monitor.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/monitor.c b/monitor.c
index 4a20d81..61abb86 100644
--- a/monitor.c
+++ b/monitor.c
@@ -255,6 +255,8 @@ bool monitor_cur_is_qmp(void)
void monitor_read_command(Monitor *mon, int show_prompt)
{
+ assert(!monitor_is_qmp(mon));
+
if (!mon->rs)
return;
@@ -266,6 +268,8 @@ void monitor_read_command(Monitor *mon, int show_prompt)
int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
void *opaque)
{
+ assert(!monitor_is_qmp(mon));
+
if (mon->rs) {
readline_start(mon->rs, "Password: ", 1, readline_func, opaque);
/* prompt is printed on return from the command handler */
@@ -2558,6 +2562,8 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
const mon_cmd_t *cmd;
char cmdname[256];
+ assert(!monitor_is_qmp(mon));
+
/* extract the command name */
p = get_command_name(*cmdp, cmdname, sizeof(cmdname));
if (!p)
@@ -2602,6 +2608,8 @@ static QDict *monitor_parse_arguments(Monitor *mon,
char buf[1024];
QDict *qdict = qdict_new();
+ assert(!monitor_is_qmp(mon));
+
/* parse the parameters */
typestr = cmd->args_type;
for(;;) {
@@ -2981,6 +2989,8 @@ static void cmd_completion(Monitor *mon, const char *name, const char *list)
char cmd[128];
int len;
+ assert(!monitor_is_qmp(mon));
+
p = list;
for(;;) {
pstart = p;
@@ -3010,6 +3020,8 @@ static void file_completion(Monitor *mon, const char *input)
int input_path_len;
const char *p;
+ assert(!monitor_is_qmp(mon));
+
p = strrchr(input, '/');
if (!p) {
input_path_len = 0;
@@ -3560,6 +3572,8 @@ static void monitor_find_completion_by_table(Monitor *mon,
const mon_cmd_t *cmd;
BlockBackend *blk = NULL;
+ assert(!monitor_is_qmp(mon));
+
if (nb_args <= 1) {
/* command completion */
if (nb_args == 0)
@@ -3641,6 +3655,8 @@ static void monitor_find_completion(void *opaque,
char *args[MAX_ARGS];
int nb_args, len;
+ assert(!monitor_is_qmp(mon));
+
/* 1. parse the cmdline */
if (parse_cmdline(cmdline, &nb_args, args) < 0) {
return;
@@ -3809,6 +3825,8 @@ static void monitor_qmp_read(void *opaque, const uint8_t *buf, int size)
cur_mon = opaque;
+ assert(monitor_is_qmp(cur_mon));
+
json_message_parser_feed(&cur_mon->qmp.parser, (const char *) buf, size);
cur_mon = old_mon;
@@ -3820,6 +3838,7 @@ static void monitor_read(void *opaque, const uint8_t *buf, int size)
int i;
cur_mon = opaque;
+ assert(monitor_is_qmp(cur_mon));
if (cur_mon->rs) {
for (i = 0; i < size; i++)
@@ -3839,6 +3858,8 @@ static void monitor_command_cb(void *opaque, const char *cmdline,
{
Monitor *mon = opaque;
+ assert(!monitor_is_qmp(mon));
+
monitor_suspend(mon);
handle_hmp_command(mon, cmdline);
monitor_resume(mon);
@@ -3846,6 +3867,8 @@ static void monitor_command_cb(void *opaque, const char *cmdline,
int monitor_suspend(Monitor *mon)
{
+ assert(!monitor_is_qmp(mon));
+
if (!mon->rs)
return -ENOTTY;
mon->suspend_cnt++;
@@ -3854,6 +3877,8 @@ int monitor_suspend(Monitor *mon)
void monitor_resume(Monitor *mon)
{
+ assert(!monitor_is_qmp(mon));
+
if (!mon->rs)
return;
if (--mon->suspend_cnt == 0)
@@ -3875,6 +3900,8 @@ static void monitor_qmp_event(void *opaque, int event)
QObject *data;
Monitor *mon = opaque;
+ assert(monitor_is_qmp(mon));
+
switch (event) {
case CHR_EVENT_OPENED:
mon->qmp.in_command_mode = false;
@@ -3898,6 +3925,8 @@ static void monitor_event(void *opaque, int event)
{
Monitor *mon = opaque;
+ assert(!monitor_is_qmp(mon));
+
switch (event) {
case CHR_EVENT_MUX_IN:
qemu_mutex_lock(&mon->out_lock);
@@ -3970,15 +3999,23 @@ static void sortcmdlist(void)
static void GCC_FMT_ATTR(2, 3) monitor_readline_printf(void *opaque,
const char *fmt, ...)
{
+ Monitor *mon = opaque;
va_list ap;
+
+ assert(!monitor_is_qmp(mon));
+
va_start(ap, fmt);
- monitor_vprintf(opaque, fmt, ap);
+ monitor_vprintf(mon, fmt, ap);
va_end(ap);
}
static void monitor_readline_flush(void *opaque)
{
- monitor_flush(opaque);
+ Monitor *mon = opaque;
+
+ assert(!monitor_is_qmp(mon));
+
+ monitor_flush(mon);
}
static void __attribute__((constructor)) monitor_lock_init(void)
@@ -4047,6 +4084,8 @@ static void bdrv_password_cb(void *opaque, const char *password,
int ret = 0;
Error *local_err = NULL;
+ assert(!monitor_is_qmp(mon));
+
bdrv_add_key(bs, password, &local_err);
if (local_err) {
error_report_err(local_err);
@@ -4064,6 +4103,8 @@ int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs,
{
int err;
+ assert(!monitor_is_qmp(mon));
+
monitor_printf(mon, "%s (%s) is encrypted.\n", bdrv_get_device_name(bs),
bdrv_get_encrypted_filename(bs));
@@ -4085,6 +4126,8 @@ int monitor_read_block_device_key(Monitor *mon, const char *device,
Error *err = NULL;
BlockBackend *blk;
+ assert(!monitor_is_qmp(mon));
+
blk = blk_by_name(device);
if (!blk) {
monitor_printf(mon, "Device not found %s\n", device);
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 15/24] monitor: suspend when running async and client has no async
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (13 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 14/24] monitor: add !qmp pre-conditions Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 16/24] qmp: update qmp-spec about async capability Marc-André Lureau
` (8 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
When the client doesn't support 'async' capability, let's suspend the
monitor until the on-going async command completes. That way, the client
will be able to use -async commands, but will get only in order
reply. (it won't be able to do concurrent commands)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
monitor.c | 32 +++++++++++++++++++++++++++++++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/monitor.c b/monitor.c
index 61abb86..0b47f83 100644
--- a/monitor.c
+++ b/monitor.c
@@ -174,6 +174,7 @@ typedef struct {
*/
bool in_command_mode; /* are we in command mode? */
bool has_async; /* the client has async capability */
+ QObject *suspended;
QmpClient client;
} MonitorQMP;
@@ -3683,9 +3684,10 @@ static int monitor_can_read(void *opaque)
{
Monitor *mon = opaque;
- return (mon->suspend_cnt == 0) ? 1 : 0;
+ return (mon->suspend_cnt == 0 && !mon->qmp.suspended) ? 1 : 0;
}
+
static bool invalid_qmp_mode(const Monitor *mon, const char *cmd,
Error **errp)
{
@@ -3762,11 +3764,33 @@ static QDict *qmp_check_input_obj(QObject *input_obj, Error **errp)
return input_dict;
}
+static void monitor_qmp_suspend(Monitor *mon, QObject *req)
+{
+ assert(monitor_is_qmp(mon));
+ assert(!mon->qmp.suspended);
+
+ qobject_incref(req);
+ mon->qmp.suspended = req;
+}
+
+static void monitor_qmp_resume(Monitor *mon)
+{
+ assert(monitor_is_qmp(mon));
+ assert(mon->qmp.suspended);
+
+ qobject_decref(mon->qmp.suspended);
+ mon->qmp.suspended = NULL;
+}
+
static void qmp_dispatch_return(QmpClient *client, QObject *rsp)
{
Monitor *mon = container_of(client, Monitor, qmp.client);
monitor_json_emitter(mon, rsp);
+
+ if (mon->qmp.suspended) {
+ monitor_qmp_resume(mon);
+ }
}
static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
@@ -3805,6 +3829,12 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
}
qmp_dispatch(&mon->qmp.client, req, rqdict);
+
+ /* suspend if the command is on-going and client doesn't support async */
+ if (!QLIST_EMPTY(&mon->qmp.client.pending) && !mon->qmp.has_async) {
+ monitor_qmp_suspend(mon, req);
+ }
+
qobject_decref(req);
return;
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 16/24] qmp: update qmp-spec about async capability
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (14 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 15/24] monitor: suspend when running async and client has no async Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 17/24] qtest: add qtest-timeout Marc-André Lureau
` (7 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
docs/qmp-spec.txt | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/docs/qmp-spec.txt b/docs/qmp-spec.txt
index f8b5356..a40706e 100644
--- a/docs/qmp-spec.txt
+++ b/docs/qmp-spec.txt
@@ -83,9 +83,32 @@ The greeting message format is:
2.2.1 Capabilities
------------------
-As of the date this document was last revised, no server or client
-capability strings have been defined.
+- "async"
+This capability indicates that the server can handle async commands.
+An async command is a regular message request with the "id" member
+mandatory (see 2.3), but the reply may be delayed.
+
+The client should match the incoming replies with the "id" member
+associated with the requests.
+
+If both the server and the client have the "async" capability, the
+client is allowed to make requests while previous async requests are
+pending. The responses may come out of order.
+
+If the client doesn't have the "async" capability, it may still call
+an async command, and make multiple outstanding calls. In this case,
+the commands are processed in order, so the replies will also be in
+order (pipelining).
+
+When a client is disconnected, the pending commands are not
+necessarily canceled. But the future clients will not get replies
+from commands they didn't make (they might receive side-effects
+events).
+
+Note that even without "async", a client may receive events from the
+server between a request and its reply, and must thus be prepared to
+handle it.
2.3 Issuing Commands
--------------------
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 17/24] qtest: add qtest-timeout
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (15 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 16/24] qmp: update qmp-spec about async capability Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 18/24] qtest: add qtest_init_qmp_caps() Marc-André Lureau
` (6 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
This command allows to test async behavior. It is only registered when
qtest is enabled. See the schema documentation for more details.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qtest.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
qapi-schema.json | 22 ++++++++++++++++++++++
2 files changed, 70 insertions(+)
diff --git a/qtest.c b/qtest.c
index 22482cc..eda171d 100644
--- a/qtest.c
+++ b/qtest.c
@@ -31,6 +31,8 @@
#ifdef TARGET_PPC64
#include "hw/ppc/spapr_rtas.h"
#endif
+#include "qapi/qmp/dispatch.h"
+#include "qapi/qmp-input-visitor.h"
#define MAX_IRQ 256
@@ -649,6 +651,49 @@ static void qtest_event(void *opaque, int event)
}
}
+static gboolean qtest_timeout_cb(void *data)
+{
+ QmpReturn *qret = data;
+
+ qmp_return(qret, NULL);
+
+ return FALSE;
+}
+
+static void qmp_qtest_timeout(QDict *args, QmpReturn *qret)
+{
+ Error *err = NULL;
+ Visitor *v;
+ int64_t duration = 0;
+
+ v = qmp_input_visitor_new(QOBJECT(args), true);
+ visit_start_struct(v, NULL, NULL, 0, &err);
+ if (err) {
+ goto out;
+ }
+
+ visit_type_int(v, "duration", &duration, &err);
+ if (!err) {
+ visit_check_struct(v, &err);
+ }
+ visit_end_struct(v, NULL);
+ if (err) {
+ goto out;
+ }
+
+ if (duration <= 0) {
+ qmp_return(qret, NULL);
+ } else {
+ g_timeout_add_seconds(duration, qtest_timeout_cb, qret);
+ }
+
+out:
+ if (err) {
+ qmp_return_error(qret, err);
+ }
+ visit_free(v);
+}
+
static int qtest_init_accel(MachineState *ms)
{
QemuOpts *opts = qemu_opts_create(qemu_find_opts("icount"), NULL, 0,
@@ -656,6 +701,9 @@ static int qtest_init_accel(MachineState *ms)
qemu_opt_set(opts, "shift", "0", &error_abort);
configure_icount(opts, &error_abort);
qemu_opts_del(opts);
+
+ qmp_register_async_command("qtest-timeout", qmp_qtest_timeout,
+ QCO_NO_OPTIONS);
return 0;
}
diff --git a/qapi-schema.json b/qapi-schema.json
index 95d3c72..e9618f6 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4658,3 +4658,25 @@
# Since: 2.7
##
{ 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
+
+##
+# @qtest-timeout:
+#
+# @duration: the time before timeout expires (in seconds)
+#
+# Test command that replies after @duration seconds.
+#
+# Example:
+#
+# -> { "execute": "qtest-timeout",
+# "arguments": { "duration": 5 } }
+# ... after 5s
+# <- { "return": {} }
+#
+# Note: this command is only available with a qtest machine
+#
+# Since: 2.8
+##
+{ 'command': 'qtest-timeout',
+ 'data': {'duration': 'int'},
+ 'gen': false } # so we can can register it manually when qtest is enabled
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 18/24] qtest: add qtest_init_qmp_caps()
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (16 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 17/24] qtest: add qtest-timeout Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 19/24] tests: add tests for async and non-async clients Marc-André Lureau
` (5 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
Add a function to specify the qmp capabilities content.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/libqtest.c | 13 +++++++++++--
tests/libqtest.h | 9 +++++++++
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 6f6bdf1..d672adb 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -146,7 +146,7 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data)
g_hook_prepend(&abrt_hooks, hook);
}
-QTestState *qtest_init(const char *extra_args)
+QTestState *qtest_init_qmp_caps(const char *extra_args, const char *qmp_caps)
{
QTestState *s;
int sock, qmpsock, i;
@@ -203,7 +203,11 @@ QTestState *qtest_init(const char *extra_args)
/* Read the QMP greeting and then do the handshake */
qtest_qmp_discard_response(s, "");
- qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }");
+ command = g_strdup_printf(
+ "{ 'execute': 'qmp_capabilities',"
+ " 'arguments': {'capabilities': [%s]}}", qmp_caps);
+ qtest_qmp_discard_response(s, command);
+ g_free(command);
if (getenv("QTEST_STOP")) {
kill(s->qemu_pid, SIGSTOP);
@@ -212,6 +216,11 @@ QTestState *qtest_init(const char *extra_args)
return s;
}
+QTestState *qtest_init(const char *extra_args)
+{
+ return qtest_init_qmp_caps(extra_args, "");
+}
+
void qtest_quit(QTestState *s)
{
qtest_instances = g_list_remove(qtest_instances, s);
diff --git a/tests/libqtest.h b/tests/libqtest.h
index f7402e0..bdec5de 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -31,6 +31,15 @@ extern QTestState *global_qtest;
*/
QTestState *qtest_init(const char *extra_args);
+/**
+ * qtest_init_qmp_caps:
+ * @extra_args: other arguments to pass to QEMU.
+ * @qmp_caps: qmp capabilities
+ *
+ * Returns: #QTestState instance.
+ */
+QTestState *qtest_init_qmp_caps(const char *extra_args, const char *qmp_caps);
+
/**
* qtest_quit:
* @s: #QTestState instance to operate on.
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 19/24] tests: add tests for async and non-async clients
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (17 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 18/24] qtest: add qtest_init_qmp_caps() Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 20/24] qapi: improve 'screendump' documentation Marc-André Lureau
` (4 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
Add two tests to check async and non-async client behaviour:
- an async client can see out of order replies
- an non-async client has commands processed in order
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/qmp-test.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 480ff28..f2ecc08 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -58,6 +58,61 @@ static void test_qom_set_without_value(void)
QDECREF(ret);
}
+static void test_qom_no_async(void)
+{
+ QDict *ret;
+ int64_t id;
+
+ /* check that only one async command is being processed */
+ qmp_async("{'execute': 'qtest-timeout', 'id': 42, "
+ " 'arguments': { 'duration': 1 } }");
+ qmp_async("{'execute': 'qtest-timeout', 'id': 43, "
+ " 'arguments': { 'duration': 0 } }");
+
+ /* check that the second command didn't execute immediately */
+ ret = qtest_qmp_receive(global_qtest);
+ g_assert_nonnull(ret);
+ id = qdict_get_try_int(ret, "id", -1);
+ g_assert_cmpint(id, ==, 42);
+ QDECREF(ret);
+
+ /* check that the second command executes after */
+ ret = qtest_qmp_receive(global_qtest);
+ g_assert_nonnull(ret);
+ id = qdict_get_try_int(ret, "id", -1);
+ g_assert_cmpint(id, ==, 43);
+ QDECREF(ret);
+}
+
+static void test_qom_async(void)
+{
+ QDict *ret;
+ int64_t id;
+ QTestState *qtest;
+
+ qtest = qtest_init_qmp_caps("-machine none", "'async'");
+
+ /* check that async are concurrent */
+ qtest_async_qmp(qtest, "{'execute': 'qtest-timeout', 'id': 42, "
+ " 'arguments': { 'duration': 1 } }");
+ qtest_async_qmp(qtest, "{'execute': 'qtest-timeout', 'id': 43, "
+ " 'arguments': { 'duration': 0 } }");
+
+ ret = qtest_qmp_receive(qtest);
+ g_assert_nonnull(ret);
+ id = qdict_get_try_int(ret, "id", -1);
+ g_assert_cmpint(id, ==, 43);
+ QDECREF(ret);
+
+ ret = qtest_qmp_receive(qtest);
+ g_assert_nonnull(ret);
+ id = qdict_get_try_int(ret, "id", -1);
+ g_assert_cmpint(id, ==, 42);
+ QDECREF(ret);
+
+ qtest_quit(qtest);
+}
+
int main(int argc, char **argv)
{
int ret;
@@ -70,6 +125,10 @@ int main(int argc, char **argv)
test_object_add_without_props);
qtest_add_func("/qemu-qmp/qom-set-without-value",
test_qom_set_without_value);
+ qtest_add_func("/qemu-qmp/no-async",
+ test_qom_no_async);
+ qtest_add_func("/qemu-qmp/async",
+ test_qom_async);
ret = g_test_run();
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 20/24] qapi: improve 'screendump' documentation
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (18 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 19/24] tests: add tests for async and non-async clients Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 21/24] console: graphic_hw_update return true if async Marc-André Lureau
` (3 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
The command dumps the console 0, and it doesn't have to be VGA.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qapi-schema.json | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index e9618f6..117ace8 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3564,7 +3564,7 @@
##
# @screendump:
#
-# Write a PPM of the VGA screen to a file.
+# Write a PPM of the first console to a file.
#
# @filename: the path of a new PPM file to store the image
#
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 21/24] console: graphic_hw_update return true if async
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (19 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 20/24] qapi: improve 'screendump' documentation Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 22/24] console: add graphic_hw_update_done() Marc-André Lureau
` (2 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
qxl_render_update() returns true if the update is deferred.
Let the caller know if the update is immediate or async.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/display/qxl-render.c | 5 +++--
hw/display/qxl.c | 8 ++++----
ui/console.c | 14 ++++++++++++--
hw/display/qxl.h | 2 +-
include/ui/console.h | 3 ++-
5 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index 9ad9d9e..50a0191 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -163,7 +163,7 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
* callbacks are called by spice_server thread, deferring to bh called from the
* io thread.
*/
-void qxl_render_update(PCIQXLDevice *qxl)
+bool qxl_render_update(PCIQXLDevice *qxl)
{
QXLCookie *cookie;
@@ -172,7 +172,7 @@ void qxl_render_update(PCIQXLDevice *qxl)
if (!runstate_is_running() || !qxl->guest_primary.commands) {
qxl_render_update_area_unlocked(qxl);
qemu_mutex_unlock(&qxl->ssd.lock);
- return;
+ return false;
}
qxl->guest_primary.commands = 0;
@@ -183,6 +183,7 @@ void qxl_render_update(PCIQXLDevice *qxl)
qxl_set_rect_to_surface(qxl, &cookie->u.render.area);
qxl_spice_update_area(qxl, 0, &cookie->u.render.area, NULL,
0, 1 /* clear_dirty_region */, QXL_ASYNC, cookie);
+ return true;
}
void qxl_render_update_area_bh(void *opaque)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 0e2682d..7dc08ca 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -134,7 +134,7 @@ static void qxl_reset_memslots(PCIQXLDevice *d);
static void qxl_reset_surfaces(PCIQXLDevice *d);
static void qxl_ring_set_dirty(PCIQXLDevice *qxl);
-static void qxl_hw_update(void *opaque);
+static bool qxl_hw_update_async(void *opaque);
void qxl_set_guest_bug(PCIQXLDevice *qxl, const char *msg, ...)
{
@@ -1091,7 +1091,7 @@ static const QXLInterface qxl_interface = {
};
static const GraphicHwOps qxl_ops = {
- .gfx_update = qxl_hw_update,
+ .gfx_update_async = qxl_hw_update_async,
};
static void qxl_enter_vga_mode(PCIQXLDevice *d)
@@ -1806,11 +1806,11 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
/* graphics console */
-static void qxl_hw_update(void *opaque)
+static bool qxl_hw_update_async(void *opaque)
{
PCIQXLDevice *qxl = opaque;
- qxl_render_update(qxl);
+ return qxl_render_update(qxl);
}
static void qxl_dirty_one_surface(PCIQXLDevice *qxl, QXLPHYSICAL pqxl,
diff --git a/ui/console.c b/ui/console.c
index fa3e658..392d7c7 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -253,14 +253,24 @@ static void gui_setup_refresh(DisplayState *ds)
ds->have_text = have_text;
}
-void graphic_hw_update(QemuConsole *con)
+bool graphic_hw_update(QemuConsole *con)
{
if (!con) {
con = active_console;
}
- if (con && con->hw_ops->gfx_update) {
+
+ if (!con) {
+ return false;
+ }
+
+ if (con->hw_ops->gfx_update_async) {
+ return con->hw_ops->gfx_update_async(con->hw);
+ } else if (con->hw_ops->gfx_update) {
con->hw_ops->gfx_update(con->hw);
+ return false;
}
+
+ return false;
}
void graphic_hw_gl_block(QemuConsole *con, bool block)
diff --git a/hw/display/qxl.h b/hw/display/qxl.h
index d2d49dd..7ac31fc 100644
--- a/hw/display/qxl.h
+++ b/hw/display/qxl.h
@@ -167,7 +167,7 @@ int qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext);
/* qxl-render.c */
void qxl_render_resize(PCIQXLDevice *qxl);
-void qxl_render_update(PCIQXLDevice *qxl);
+bool qxl_render_update(PCIQXLDevice *qxl);
int qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext);
void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie);
void qxl_render_update_area_bh(void *opaque);
diff --git a/include/ui/console.h b/include/ui/console.h
index e2589e2..772b9db 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -362,6 +362,7 @@ static inline void console_write_ch(console_ch_t *dest, uint32_t ch)
typedef struct GraphicHwOps {
void (*invalidate)(void *opaque);
void (*gfx_update)(void *opaque);
+ bool (*gfx_update_async)(void *opaque);
void (*text_update)(void *opaque, console_ch_t *text);
void (*update_interval)(void *opaque, uint64_t interval);
int (*ui_info)(void *opaque, uint32_t head, QemuUIInfo *info);
@@ -375,7 +376,7 @@ void graphic_console_set_hwops(QemuConsole *con,
const GraphicHwOps *hw_ops,
void *opaque);
-void graphic_hw_update(QemuConsole *con);
+bool graphic_hw_update(QemuConsole *con);
void graphic_hw_invalidate(QemuConsole *con);
void graphic_hw_text_update(QemuConsole *con, console_ch_t *chardata);
void graphic_hw_gl_block(QemuConsole *con, bool block);
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 22/24] console: add graphic_hw_update_done()
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (20 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 21/24] console: graphic_hw_update return true if async Marc-André Lureau
@ 2016-10-10 9:22 ` Marc-André Lureau
2016-10-10 9:23 ` [Qemu-devel] [PATCH 23/24] console: make screendump async Marc-André Lureau
2016-10-10 9:23 ` [Qemu-devel] [PATCH 24/24] qmp: move json-message-parser to QmpClient Marc-André Lureau
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
Add a function to be called when an async graphic update is done.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/display/qxl-render.c | 9 +++++++--
ui/console.c | 4 ++++
include/ui/console.h | 1 +
3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index 50a0191..8341f58 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -106,7 +106,7 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
qxl->guest_primary.surface.mem,
MEMSLOT_GROUP_GUEST);
if (!qxl->guest_primary.data) {
- return;
+ goto end;
}
qxl_set_rect_to_surface(qxl, &qxl->dirty[0]);
qxl->num_dirty_rects = 1;
@@ -134,7 +134,7 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
}
if (!qxl->guest_primary.data) {
- return;
+ goto end;
}
for (i = 0; i < qxl->num_dirty_rects; i++) {
if (qemu_spice_rect_is_empty(qxl->dirty+i)) {
@@ -155,6 +155,11 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
qxl->dirty[i].bottom - qxl->dirty[i].top);
}
qxl->num_dirty_rects = 0;
+
+end:
+ if (qxl->render_update_cookie_num == 0) {
+ graphic_hw_update_done(qxl->ssd.dcl.con);
+ }
}
/*
diff --git a/ui/console.c b/ui/console.c
index 392d7c7..7ca5d19 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -253,6 +253,10 @@ static void gui_setup_refresh(DisplayState *ds)
ds->have_text = have_text;
}
+void graphic_hw_update_done(QemuConsole *con)
+{
+}
+
bool graphic_hw_update(QemuConsole *con)
{
if (!con) {
diff --git a/include/ui/console.h b/include/ui/console.h
index 772b9db..fd9f648 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -377,6 +377,7 @@ void graphic_console_set_hwops(QemuConsole *con,
void *opaque);
bool graphic_hw_update(QemuConsole *con);
+void graphic_hw_update_done(QemuConsole *con);
void graphic_hw_invalidate(QemuConsole *con);
void graphic_hw_text_update(QemuConsole *con, console_ch_t *chardata);
void graphic_hw_gl_block(QemuConsole *con, bool block);
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 23/24] console: make screendump async
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (21 preceding siblings ...)
2016-10-10 9:22 ` [Qemu-devel] [PATCH 22/24] console: add graphic_hw_update_done() Marc-André Lureau
@ 2016-10-10 9:23 ` Marc-André Lureau
2016-10-10 9:23 ` [Qemu-devel] [PATCH 24/24] qmp: move json-message-parser to QmpClient Marc-André Lureau
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:23 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
Make screendump async to provide correct screendumps.
HMP doesn't have async support, so it has to remain sync to avoid
potential races.
Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1230527
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hmp.c | 2 +-
ui/console.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++-
include/ui/console.h | 1 +
qapi-schema.json | 11 ++++++++-
4 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/hmp.c b/hmp.c
index 336e7bf..b161453 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1809,7 +1809,7 @@ void hmp_screendump(Monitor *mon, const QDict *qdict)
const char *filename = qdict_get_str(qdict, "filename");
Error *err = NULL;
- qmp_screendump(filename, &err);
+ hmp_screendump_sync(filename, &err);
hmp_handle_error(mon, &err);
}
diff --git a/ui/console.c b/ui/console.c
index 7ca5d19..85f099c 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -114,6 +114,12 @@ typedef enum {
TEXT_CONSOLE_FIXED_SIZE
} console_type_t;
+struct qmp_screendump {
+ gchar *filename;
+ QmpReturn *ret;
+ QLIST_ENTRY(qmp_screendump) link;
+};
+
struct QemuConsole {
Object parent;
@@ -162,6 +168,8 @@ struct QemuConsole {
QEMUFIFO out_fifo;
uint8_t out_fifo_buf[16];
QEMUTimer *kbd_timer;
+
+ QLIST_HEAD(, qmp_screendump) qmp_screendumps;
};
struct DisplayState {
@@ -187,6 +195,8 @@ static void dpy_refresh(DisplayState *s);
static DisplayState *get_alloc_displaystate(void);
static void text_console_update_cursor_timer(void);
static void text_console_update_cursor(void *opaque);
+static void ppm_save(const char *filename, DisplaySurface *ds,
+ Error **errp);
static void gui_update(void *opaque)
{
@@ -253,8 +263,39 @@ static void gui_setup_refresh(DisplayState *ds)
ds->have_text = have_text;
}
+static void qmp_screendump_finish(QemuConsole *con, const char *filename,
+ QmpReturn *ret)
+{
+ Error *err = NULL;
+ DisplaySurface *surface;
+
+ if (qmp_return_is_cancelled(ret)) {
+ return;
+ }
+
+ surface = qemu_console_surface(con);
+
+ /* FIXME: async save with coroutine? it would have to copy or lock
+ * the surface. */
+ ppm_save(filename, surface, &err);
+
+ if (err) {
+ qmp_return_error(ret, err);
+ } else {
+ qmp_screendump_return(ret);
+ }
+}
+
void graphic_hw_update_done(QemuConsole *con)
{
+ struct qmp_screendump *dump, *next;
+
+ QLIST_FOREACH_SAFE(dump, &con->qmp_screendumps, link, next) {
+ qmp_screendump_finish(con, dump->filename, dump->ret);
+ g_free(dump->filename);
+ QLIST_REMOVE(dump, link);
+ g_free(dump);
+ }
}
bool graphic_hw_update(QemuConsole *con)
@@ -345,7 +386,8 @@ write_err:
goto out;
}
-void qmp_screendump(const char *filename, Error **errp)
+/* this sync screendump may produce outdated dumps: use qmp instead */
+void hmp_screendump_sync(const char *filename, Error **errp)
{
QemuConsole *con = qemu_console_lookup_by_index(0);
DisplaySurface *surface;
@@ -360,6 +402,29 @@ void qmp_screendump(const char *filename, Error **errp)
ppm_save(filename, surface, errp);
}
+void qmp_screendump(const char *filename, QmpReturn *qret)
+{
+ QemuConsole *con = qemu_console_lookup_by_index(0);
+ bool async;
+ Error *err = NULL;
+
+ if (con == NULL) {
+ error_setg(&err, "There is no QemuConsole I can screendump from.");
+ qmp_return_error(qret, err);
+ return;
+ }
+
+ async = graphic_hw_update(con);
+ if (async) {
+ struct qmp_screendump *dump = g_new(struct qmp_screendump, 1);
+ dump->filename = g_strdup(filename);
+ dump->ret = qret;
+ QLIST_INSERT_HEAD(&con->qmp_screendumps, dump, link);
+ } else {
+ qmp_screendump_finish(con, filename, qret);
+ }
+}
+
void graphic_hw_text_update(QemuConsole *con, console_ch_t *chardata)
{
if (!con) {
@@ -1235,6 +1300,7 @@ static QemuConsole *new_console(DisplayState *ds, console_type_t console_type,
obj = object_new(TYPE_QEMU_CONSOLE);
s = QEMU_CONSOLE(obj);
s->head = head;
+ QLIST_INIT(&s->qmp_screendumps);
object_property_add_link(obj, "device", TYPE_DEVICE,
(Object **)&s->device,
object_property_allow_set_link,
diff --git a/include/ui/console.h b/include/ui/console.h
index fd9f648..7c4f35b 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -75,6 +75,7 @@ struct MouseTransformInfo {
};
void hmp_mouse_set(Monitor *mon, const QDict *qdict);
+void hmp_screendump_sync(const char *filename, Error **errp);
/* keysym is a unicode code except for special keys (see QEMU_KEY_xxx
constants) */
diff --git a/qapi-schema.json b/qapi-schema.json
index 117ace8..80e9aeb 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3570,9 +3570,18 @@
#
# Returns: Nothing on success
#
+# Note: the operation can be cancelled and the file not written if the
+# client disconnects before completion.
+#
# Since: 0.14.0
+#
+# Example:
+#
+# -> { "execute": "screendump", "arguments": { "filename": "/tmp/image" }, id: X }
+# <- { "return": {}, id: X }
+#
##
-{ 'command': 'screendump', 'data': {'filename': 'str'} }
+{ 'command': 'screendump', 'data': {'filename': 'str'}, 'async': true }
##
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH 24/24] qmp: move json-message-parser to QmpClient
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
` (22 preceding siblings ...)
2016-10-10 9:23 ` [Qemu-devel] [PATCH 23/24] console: make screendump async Marc-André Lureau
@ 2016-10-10 9:23 ` Marc-André Lureau
23 siblings, 0 replies; 28+ messages in thread
From: Marc-André Lureau @ 2016-10-10 9:23 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, berrange, armbru, Marc-André Lureau
Clean up qmp_dispatch usage to have consistant checks between qga &
qemu, and simplify QmpClient/parser_feed usage.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
monitor.c | 142 ++++++++------------------------------------
qapi/qmp-dispatch.c | 125 +++++++++++++++++++++++++++++++++++++-
qga/main.c | 59 +-----------------
qobject/json-lexer.c | 4 +-
tests/test-qmp-commands.c | 10 ++--
include/qapi/qmp/dispatch.h | 13 +++-
6 files changed, 172 insertions(+), 181 deletions(-)
diff --git a/monitor.c b/monitor.c
index 0b47f83..2b60885 100644
--- a/monitor.c
+++ b/monitor.c
@@ -56,7 +56,6 @@
#include "qapi/qmp/qerror.h"
#include "qapi/qmp/types.h"
#include "qapi/qmp/qjson.h"
-#include "qapi/qmp/json-streamer.h"
#include "qapi/qmp/json-parser.h"
#include "qom/object_interfaces.h"
#include "cpu.h"
@@ -166,7 +165,6 @@ struct MonFdset {
};
typedef struct {
- JSONMessageParser parser;
/*
* When a client connects, we're in capabilities negotiation mode.
* When command qmp_capabilities succeeds, we go into command
@@ -610,9 +608,6 @@ static void monitor_data_destroy(Monitor *mon)
if (mon->chr) {
qemu_chr_add_handlers(mon->chr, NULL, NULL, NULL, NULL);
}
- if (monitor_is_qmp(mon)) {
- json_message_parser_destroy(&mon->qmp.parser);
- }
qmp_client_destroy(&mon->qmp.client);
g_free(mon->rs);
QDECREF(mon->outbuf);
@@ -3708,62 +3703,6 @@ static bool invalid_qmp_mode(const Monitor *mon, const char *cmd,
return false;
}
-/*
- * Input object checking rules
- *
- * 1. Input object must be a dict
- * 2. The "execute" key must exist
- * 3. The "execute" key must be a string
- * 4. If the "arguments" key exists, it must be a dict
- * 5. If the "id" key exists, it can be anything (ie. json-value)
- * 6. Any argument not listed above is considered invalid
- */
-static QDict *qmp_check_input_obj(QObject *input_obj, Error **errp)
-{
- const QDictEntry *ent;
- int has_exec_key = 0;
- QDict *input_dict;
-
- if (qobject_type(input_obj) != QTYPE_QDICT) {
- error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT, "object");
- return NULL;
- }
-
- input_dict = qobject_to_qdict(input_obj);
-
- for (ent = qdict_first(input_dict); ent; ent = qdict_next(input_dict, ent)){
- const char *arg_name = qdict_entry_key(ent);
- const QObject *arg_obj = qdict_entry_value(ent);
-
- if (!strcmp(arg_name, "execute")) {
- if (qobject_type(arg_obj) != QTYPE_QSTRING) {
- error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT_MEMBER,
- "execute", "string");
- return NULL;
- }
- has_exec_key = 1;
- } else if (!strcmp(arg_name, "arguments")) {
- if (qobject_type(arg_obj) != QTYPE_QDICT) {
- error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT_MEMBER,
- "arguments", "object");
- return NULL;
- }
- } else if (!strcmp(arg_name, "id")) {
- /* Any string is acceptable as "id", so nothing to check */
- } else {
- error_setg(errp, QERR_QMP_EXTRA_MEMBER, arg_name);
- return NULL;
- }
- }
-
- if (!has_exec_key) {
- error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT, "execute");
- return NULL;
- }
-
- return input_dict;
-}
-
static void monitor_qmp_suspend(Monitor *mon, QObject *req)
{
assert(monitor_is_qmp(mon));
@@ -3782,71 +3721,42 @@ static void monitor_qmp_resume(Monitor *mon)
mon->qmp.suspended = NULL;
}
-static void qmp_dispatch_return(QmpClient *client, QObject *rsp)
+static bool qmp_pre_dispatch(QmpClient *client, QObject *req, Error **errp)
{
Monitor *mon = container_of(client, Monitor, qmp.client);
+ QDict *qdict = qobject_to_qdict(req);
+ const char *cmd_name = qdict_get_str(qdict, "execute");
- monitor_json_emitter(mon, rsp);
+ trace_handle_qmp_command(mon, cmd_name);
- if (mon->qmp.suspended) {
- monitor_qmp_resume(mon);
+ if (invalid_qmp_mode(mon, cmd_name, errp)) {
+ return false;
}
+
+ return true;
}
-static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
+static bool qmp_post_dispatch(QmpClient *client, QObject *req, Error **errp)
{
- QObject *req, *id = NULL;
- QDict *qdict, *rqdict = qdict_new();
- const char *cmd_name;
- Monitor *mon = cur_mon;
- Error *err = NULL;
-
- req = json_parser_parse_err(tokens, NULL, &err);
- if (err || !req || qobject_type(req) != QTYPE_QDICT) {
- if (!err) {
- error_setg(&err, QERR_JSON_PARSING);
- }
- goto err_out;
- }
-
- qdict = qmp_check_input_obj(req, &err);
- if (!qdict) {
- goto err_out;
- }
-
- id = qdict_get(qdict, "id");
- if (id) {
- qobject_incref(id);
- qdict_del(qdict, "id");
- qdict_put_obj(rqdict, "id", id);
- }
-
- cmd_name = qdict_get_str(qdict, "execute");
- trace_handle_qmp_command(mon, cmd_name);
-
- if (invalid_qmp_mode(mon, cmd_name, &err)) {
- goto err_out;
- }
-
- qmp_dispatch(&mon->qmp.client, req, rqdict);
+ Monitor *mon = container_of(client, Monitor, qmp.client);
/* suspend if the command is on-going and client doesn't support async */
if (!QLIST_EMPTY(&mon->qmp.client.pending) && !mon->qmp.has_async) {
monitor_qmp_suspend(mon, req);
}
- qobject_decref(req);
- return;
+ return true;
+}
-err_out:
- if (err) {
- qdict_put_obj(rqdict, "error", qmp_build_error_object(err));
- error_free(err);
- monitor_json_emitter(mon, QOBJECT(rqdict));
- }
+static void qmp_dispatch_return(QmpClient *client, QObject *rsp)
+{
+ Monitor *mon = container_of(client, Monitor, qmp.client);
+
+ monitor_json_emitter(mon, rsp);
- QDECREF(rqdict);
- qobject_decref(req);
+ if (mon->qmp.suspended) {
+ monitor_qmp_resume(mon);
+ }
}
static void monitor_qmp_read(void *opaque, const uint8_t *buf, int size)
@@ -3857,7 +3767,8 @@ static void monitor_qmp_read(void *opaque, const uint8_t *buf, int size)
assert(monitor_is_qmp(cur_mon));
- json_message_parser_feed(&cur_mon->qmp.parser, (const char *) buf, size);
+ qmp_client_feed(&cur_mon->qmp.client,
+ (const char *) buf, size);
cur_mon = old_mon;
}
@@ -3934,6 +3845,10 @@ static void monitor_qmp_event(void *opaque, int event)
switch (event) {
case CHR_EVENT_OPENED:
+ qmp_client_init(&mon->qmp.client,
+ qmp_pre_dispatch,
+ qmp_post_dispatch,
+ qmp_dispatch_return);
mon->qmp.in_command_mode = false;
data = get_qmp_greeting();
monitor_json_emitter(mon, data);
@@ -3941,10 +3856,7 @@ static void monitor_qmp_event(void *opaque, int event)
mon_refcount++;
break;
case CHR_EVENT_CLOSED:
- json_message_parser_destroy(&mon->qmp.parser);
- json_message_parser_init(&mon->qmp.parser, handle_qmp_command);
qmp_client_destroy(&mon->qmp.client);
- qmp_client_init(&mon->qmp.client, qmp_dispatch_return);
mon_refcount--;
monitor_fdsets_cleanup();
break;
@@ -4081,8 +3993,6 @@ void monitor_init(CharDriverState *chr, int flags)
qemu_chr_add_handlers(chr, monitor_can_read, monitor_qmp_read,
monitor_qmp_event, mon);
qemu_chr_fe_set_echo(chr, true);
- json_message_parser_init(&mon->qmp.parser, handle_qmp_command);
- qmp_client_init(&mon->qmp.client, qmp_dispatch_return);
} else {
qemu_chr_add_handlers(chr, monitor_can_read, monitor_read,
monitor_event, mon);
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index b6a1feb..d972b4c 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -20,6 +20,12 @@
#include "qapi-types.h"
#include "qapi/qmp/qerror.h"
+void qmp_client_feed(QmpClient *client,
+ const char *buffer, size_t size)
+{
+ json_message_parser_feed(&client->parser, buffer, size);
+}
+
static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp)
{
const QDictEntry *ent;
@@ -179,8 +185,123 @@ bool qmp_return_is_cancelled(QmpReturn *qret)
return false;
}
-void qmp_client_init(QmpClient *client, QmpDispatchReturn *return_cb)
+/*
+ * Input object checking rules
+ *
+ * 1. Input object must be a dict
+ * 2. The "execute" key must exist
+ * 3. The "execute" key must be a string
+ * 4. If the "arguments" key exists, it must be a dict
+ * 5. If the "id" key exists, it can be anything (ie. json-value)
+ * 6. Any argument not listed above is considered invalid
+ */
+static QDict *qmp_check_input_obj(QObject *input_obj, Error **errp)
+{
+ const QDictEntry *ent;
+ int has_exec_key = 0;
+ QDict *dict;
+
+ if (qobject_type(input_obj) != QTYPE_QDICT) {
+ error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT, "object");
+ return NULL;
+ }
+
+ dict = qobject_to_qdict(input_obj);
+
+ for (ent = qdict_first(dict); ent; ent = qdict_next(dict, ent)) {
+ const char *arg_name = qdict_entry_key(ent);
+ const QObject *arg_obj = qdict_entry_value(ent);
+
+ if (!strcmp(arg_name, "execute")) {
+ if (qobject_type(arg_obj) != QTYPE_QSTRING) {
+ error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT_MEMBER,
+ "execute", "string");
+ return NULL;
+ }
+ has_exec_key = 1;
+ } else if (!strcmp(arg_name, "arguments")) {
+ if (qobject_type(arg_obj) != QTYPE_QDICT) {
+ error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT_MEMBER,
+ "arguments", "object");
+ return NULL;
+ }
+ } else if (!strcmp(arg_name, "id")) {
+ /* Any string is acceptable as "id", so nothing to check */
+ } else {
+ error_setg(errp, QERR_QMP_EXTRA_MEMBER, arg_name);
+ return NULL;
+ }
+ }
+
+ if (!has_exec_key) {
+ error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT, "execute");
+ return NULL;
+ }
+
+ return dict;
+}
+
+static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
{
+ QmpClient *client = container_of(parser, QmpClient, parser);
+ QObject *req, *id = NULL;
+ QDict *qdict, *rqdict = qdict_new();
+ Error *err = NULL;
+
+ req = json_parser_parse_err(tokens, NULL, &err);
+ if (err || !req || qobject_type(req) != QTYPE_QDICT) {
+ if (!err) {
+ error_setg(&err, QERR_JSON_PARSING);
+ }
+ goto err_out;
+ }
+
+ qdict = qmp_check_input_obj(req, &err);
+ if (!qdict) {
+ goto err_out;
+ }
+
+ id = qdict_get(qdict, "id");
+ if (id) {
+ qobject_incref(id);
+ qdict_del(qdict, "id");
+ qdict_put_obj(rqdict, "id", id);
+ }
+
+ if (client->pre_dispatch_cb &&
+ !client->pre_dispatch_cb(client, QOBJECT(qdict), &err)) {
+ goto err_out;
+ }
+
+ qmp_dispatch(client, req, rqdict);
+
+ if (client->post_dispatch_cb &&
+ !client->post_dispatch_cb(client, QOBJECT(qdict), &err)) {
+ goto err_out;
+ }
+
+ qobject_decref(req);
+ return;
+
+err_out:
+ if (err) {
+ qdict_put_obj(rqdict, "error", qmp_build_error_object(err));
+ error_free(err);
+ client->return_cb(client, QOBJECT(rqdict));
+ }
+
+ QDECREF(rqdict);
+ qobject_decref(req);
+}
+
+void qmp_client_init(QmpClient *client,
+ QmpPreDispatch *pre_dispatch_cb,
+ QmpPostDispatch *post_dispatch_cb,
+ QmpDispatchReturn *return_cb)
+{
+ json_message_parser_init(&client->parser, handle_qmp_command);
+ client->pre_dispatch_cb = pre_dispatch_cb;
+ client->post_dispatch_cb = post_dispatch_cb;
client->return_cb = return_cb;
QLIST_INIT(&client->pending);
}
@@ -189,6 +310,8 @@ void qmp_client_destroy(QmpClient *client)
{
QmpReturn *ret, *next;
+ json_message_parser_destroy(&client->parser);
+
/* Remove the weak references to the pending returns. The
* dispatched function is the owner of QmpReturn, and will have to
* qmp_return(). (it might be interesting to have a way to notify
diff --git a/qga/main.c b/qga/main.c
index 267e3bb..8f6a6e7 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -65,7 +65,6 @@ typedef struct GAPersistentState {
} GAPersistentState;
struct GAState {
- JSONMessageParser parser;
GMainLoop *main_loop;
GAChannel *channel;
bool virtio; /* fastpath to check for virtio to deal with poll() quirks */
@@ -559,59 +558,7 @@ static void dispatch_return_cb(QmpClient *client, QObject *rsp)
}
}
-static void process_command(GAState *s, QDict *req)
-{
- g_assert(req);
- g_debug("processing command");
- qmp_dispatch(&ga_state->client, QOBJECT(req), NULL);
-}
-
/* handle requests/control events coming in over the channel */
-static void process_event(JSONMessageParser *parser, GQueue *tokens)
-{
- GAState *s = container_of(parser, GAState, parser);
- QDict *qdict;
- Error *err = NULL;
- int ret;
-
- g_assert(s && parser);
-
- g_debug("process_event: called");
- qdict = qobject_to_qdict(json_parser_parse_err(tokens, NULL, &err));
- if (err || !qdict) {
- QDECREF(qdict);
- qdict = qdict_new();
- if (!err) {
- g_warning("failed to parse event: unknown error");
- error_setg(&err, QERR_JSON_PARSING);
- } else {
- g_warning("failed to parse event: %s", error_get_pretty(err));
- }
- qdict_put_obj(qdict, "error", qmp_build_error_object(err));
- error_free(err);
- }
-
- /* handle host->guest commands */
- if (qdict_haskey(qdict, "execute")) {
- process_command(s, qdict);
- } else {
- if (!qdict_haskey(qdict, "error")) {
- QDECREF(qdict);
- qdict = qdict_new();
- g_warning("unrecognized payload format");
- error_setg(&err, QERR_UNSUPPORTED);
- qdict_put_obj(qdict, "error", qmp_build_error_object(err));
- error_free(err);
- }
- ret = send_response(s, QOBJECT(qdict));
- if (ret < 0) {
- g_warning("error sending error response: %s", strerror(-ret));
- }
- }
-
- QDECREF(qdict);
-}
-
/* false return signals GAChannel to close the current client connection */
static gboolean channel_event_cb(GIOCondition condition, gpointer data)
{
@@ -626,7 +573,7 @@ static gboolean channel_event_cb(GIOCondition condition, gpointer data)
case G_IO_STATUS_NORMAL:
buf[count] = 0;
g_debug("read data, count: %d, data: %s", (int)count, buf);
- json_message_parser_feed(&s->parser, (char *)buf, (int)count);
+ qmp_client_feed(&s->client, (char *)buf, (int)count);
break;
case G_IO_STATUS_EOF:
g_debug("received EOF");
@@ -1283,9 +1230,8 @@ static int run_agent(GAState *s, GAConfig *config)
s->command_state = ga_command_state_new();
ga_command_state_init(s, s->command_state);
ga_command_state_init_all(s->command_state);
- json_message_parser_init(&s->parser, process_event);
ga_state = s;
- qmp_client_init(&s->client, dispatch_return_cb);
+ qmp_client_init(&s->client, NULL, NULL, dispatch_return_cb);
#ifndef _WIN32
if (!register_signal_handlers()) {
g_critical("failed to register signal handlers");
@@ -1374,7 +1320,6 @@ end:
if (s->command_state) {
ga_command_state_cleanup_all(s->command_state);
ga_command_state_free(s->command_state);
- json_message_parser_destroy(&s->parser);
}
if (s->channel) {
ga_channel_free(s->channel);
diff --git a/qobject/json-lexer.c b/qobject/json-lexer.c
index af4a75e..94f0db3 100644
--- a/qobject/json-lexer.c
+++ b/qobject/json-lexer.c
@@ -382,5 +382,7 @@ int json_lexer_flush(JSONLexer *lexer)
void json_lexer_destroy(JSONLexer *lexer)
{
- g_string_free(lexer->token, true);
+ if (lexer->token) {
+ g_string_free(lexer->token, true);
+ }
}
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index cca5555..2965dc1 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -127,7 +127,7 @@ static void test_dispatch_cmd(void)
QmpClient client;
QDict *req = qdict_new();
- qmp_client_init(&client, dispatch_cmd_return);
+ qmp_client_init(&client, NULL, NULL, dispatch_cmd_return);
qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd")));
@@ -150,7 +150,7 @@ static void test_dispatch_cmd_failure(void)
QDict *req = qdict_new();
QDict *args = qdict_new();
- qmp_client_init(&client, dispatch_cmd_error_return);
+ qmp_client_init(&client, NULL, NULL, dispatch_cmd_error_return);
qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd2")));
@@ -192,7 +192,7 @@ static QObject *test_qmp_dispatch(QDict *req)
QObject *ret;
QmpClient client;
- qmp_client_init(&client, qmp_dispatch_return);
+ qmp_client_init(&client, NULL, NULL, qmp_dispatch_return);
qmp_dispatch(&client, QOBJECT(req), NULL);
qmp_client_destroy(&client);
@@ -258,7 +258,7 @@ static void test_dispatch_cmd_async(void)
QDict *args = qdict_new();
loop = g_main_loop_new(NULL, FALSE);
- qmp_client_init(&client, qmp_dispatch_return);
+ qmp_client_init(&client, NULL, NULL, qmp_dispatch_return);
qdict_put(args, "a", qint_from_int(99));
qdict_put(req, "arguments", args);
@@ -290,7 +290,7 @@ static void test_destroy_pending_async(void)
int npending = 0;
loop = g_main_loop_new(NULL, FALSE);
- qmp_client_init(&client, qmp_dispatch_return);
+ qmp_client_init(&client, NULL, NULL, qmp_dispatch_return);
qdict_put(args, "a", qint_from_int(99));
qdict_put(req, "arguments", args);
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index e13e381..ed9c062 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -16,9 +16,12 @@
#include "qapi/qmp/qobject.h"
#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/json-streamer.h"
typedef struct QmpClient QmpClient;
+typedef bool (QmpPreDispatch) (QmpClient *client, QObject *rsp, Error **err);
+typedef bool (QmpPostDispatch) (QmpClient *client, QObject *rsp, Error **err);
typedef void (QmpDispatchReturn) (QmpClient *client, QObject *rsp);
typedef struct QmpReturn {
@@ -29,6 +32,9 @@ typedef struct QmpReturn {
} QmpReturn;
struct QmpClient {
+ JSONMessageParser parser;
+ QmpPreDispatch *pre_dispatch_cb;
+ QmpPostDispatch *post_dispatch_cb;
QmpDispatchReturn *return_cb;
QLIST_HEAD(, QmpReturn) pending;
@@ -67,8 +73,13 @@ void qmp_register_async_command(const char *name, QmpCommandFuncAsync *fn,
QmpCommandOptions options);
void qmp_unregister_command(const char *name);
QmpCommand *qmp_find_command(const char *name);
-void qmp_client_init(QmpClient *client, QmpDispatchReturn *return_cb);
+void qmp_client_init(QmpClient *client,
+ QmpPreDispatch *pre_dispatch_cb,
+ QmpPostDispatch *post_dispatch_cb,
+ QmpDispatchReturn *return_cb);
void qmp_client_destroy(QmpClient *client);
+void qmp_client_feed(QmpClient *client, const char *buffer, size_t size);
+
void qmp_dispatch(QmpClient *client, QObject *request, QDict *rsp);
void qmp_disable_command(const char *name);
void qmp_enable_command(const char *name);
--
2.10.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH 01/24] tests: start generic qemu-qmp tests
2016-10-10 9:22 ` [Qemu-devel] [PATCH 01/24] tests: start generic qemu-qmp tests Marc-André Lureau
@ 2016-10-10 20:09 ` Eric Blake
0 siblings, 0 replies; 28+ messages in thread
From: Eric Blake @ 2016-10-10 20:09 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: berrange, armbru
[-- Attachment #1: Type: text/plain, Size: 1402 bytes --]
On 10/10/2016 04:22 AM, Marc-André Lureau wrote:
> These 2 tests exhibit two qmp bugs fixed by the previous patches.
It looks like this is a respin because it was removed from an earlier
pull request; now that the previous patches mentioned have landed and
this is no longer immediately adjacent to those patches, is it worth
tweaking the commit message to call out commit ids?
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Message-Id: <20160922203927.28241-4-marcandre.lureau@redhat.com>
> [Rename tests/test-qemu-qmp.c to tests/qmp-test.c, cover it in
> MAINTAINERS, add a file comment]
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Since you reworked the patch to get it to compile, you may want to drop
the [] parenthetical inserted by Markus, as this is now a new revision
of your patch (although crediting him for contributions is still
appropriate).
> +++ b/tests/qmp-test.c
> @@ -0,0 +1,79 @@
> +/*
> + * QTest testcase for QMP
> + *
> + * Copyright (c) 2016 Red Hat, Inc.
We aren't very consistent on (C) vs. (c). I don't know if a lawyer would
complain.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH 02/24] tests: change /0.15/* tests to /qmp/*
2016-10-10 9:22 ` [Qemu-devel] [PATCH 02/24] tests: change /0.15/* tests to /qmp/* Marc-André Lureau
@ 2016-10-10 20:10 ` Eric Blake
0 siblings, 0 replies; 28+ messages in thread
From: Eric Blake @ 2016-10-10 20:10 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: berrange, armbru
[-- Attachment #1: Type: text/plain, Size: 1639 bytes --]
On 10/10/2016 04:22 AM, Marc-André Lureau wrote:
> Presumably 0.15 was the version it was first introduced, but
> qmp keeps evolving. There is no point in having that version
> as test prefix, 'qmp' makes more sense here.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> tests/test-qmp-commands.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
>
> diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
> index 81cbe54..5ff2f9b 100644
> --- a/tests/test-qmp-commands.c
> +++ b/tests/test-qmp-commands.c
> @@ -267,11 +267,11 @@ int main(int argc, char **argv)
> {
> g_test_init(&argc, &argv, NULL);
>
> - g_test_add_func("/0.15/dispatch_cmd", test_dispatch_cmd);
> - g_test_add_func("/0.15/dispatch_cmd_failure", test_dispatch_cmd_failure);
> - g_test_add_func("/0.15/dispatch_cmd_io", test_dispatch_cmd_io);
> - g_test_add_func("/0.15/dealloc_types", test_dealloc_types);
> - g_test_add_func("/0.15/dealloc_partial", test_dealloc_partial);
> + g_test_add_func("/qmp/dispatch_cmd", test_dispatch_cmd);
> + g_test_add_func("/qmp/dispatch_cmd_failure", test_dispatch_cmd_failure);
> + g_test_add_func("/qmp/dispatch_cmd_io", test_dispatch_cmd_io);
> + g_test_add_func("/qmp/dealloc_types", test_dealloc_types);
> + g_test_add_func("/qmp/dealloc_partial", test_dealloc_partial);
>
> module_call_init(MODULE_INIT_QAPI);
> g_test_run();
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH 03/24] qmp: teach qmp_dispatch() to take a pre-filled QDict
2016-10-10 9:22 ` [Qemu-devel] [PATCH 03/24] qmp: teach qmp_dispatch() to take a pre-filled QDict Marc-André Lureau
@ 2016-10-10 20:20 ` Eric Blake
0 siblings, 0 replies; 28+ messages in thread
From: Eric Blake @ 2016-10-10 20:20 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: berrange, armbru
[-- Attachment #1: Type: text/plain, Size: 797 bytes --]
On 10/10/2016 04:22 AM, Marc-André Lureau wrote:
> Give an optionnal qdict for the dispatch call to be used for the
s/optionnal/optional/
> reply. The qemu monitor has the request "id" pre-filled, simplifying a
> bit the code.
simplifying the code a bit
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> monitor.c | 26 +++++++++++---------------
> qapi/qmp-dispatch.c | 5 ++---
> qga/main.c | 2 +-
> tests/test-qmp-commands.c | 8 ++++----
> include/qapi/qmp/dispatch.h | 2 +-
> 5 files changed, 19 insertions(+), 24 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2016-10-10 20:21 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-10 9:22 [Qemu-devel] [PATCH 00/24] qapi: add async command type Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 01/24] tests: start generic qemu-qmp tests Marc-André Lureau
2016-10-10 20:09 ` Eric Blake
2016-10-10 9:22 ` [Qemu-devel] [PATCH 02/24] tests: change /0.15/* tests to /qmp/* Marc-André Lureau
2016-10-10 20:10 ` Eric Blake
2016-10-10 9:22 ` [Qemu-devel] [PATCH 03/24] qmp: teach qmp_dispatch() to take a pre-filled QDict Marc-André Lureau
2016-10-10 20:20 ` Eric Blake
2016-10-10 9:22 ` [Qemu-devel] [PATCH 04/24] qmp: use a return callback for the command reply Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 05/24] qmp: add QmpClient Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 06/24] qmp: add qmp_return_is_cancelled() Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 07/24] qmp: introduce async command type Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 08/24] qapi: ignore top-level 'id' field Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 09/24] qmp: take 'id' from request Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 10/24] qmp: check that async command have an 'id' Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 11/24] scripts: learn 'async' qapi commands Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 12/24] tests: add dispatch async tests Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 13/24] monitor: add 'async' capability Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 14/24] monitor: add !qmp pre-conditions Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 15/24] monitor: suspend when running async and client has no async Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 16/24] qmp: update qmp-spec about async capability Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 17/24] qtest: add qtest-timeout Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 18/24] qtest: add qtest_init_qmp_caps() Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 19/24] tests: add tests for async and non-async clients Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 20/24] qapi: improve 'screendump' documentation Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 21/24] console: graphic_hw_update return true if async Marc-André Lureau
2016-10-10 9:22 ` [Qemu-devel] [PATCH 22/24] console: add graphic_hw_update_done() Marc-André Lureau
2016-10-10 9:23 ` [Qemu-devel] [PATCH 23/24] console: make screendump async Marc-André Lureau
2016-10-10 9:23 ` [Qemu-devel] [PATCH 24/24] qmp: move json-message-parser to QmpClient Marc-André Lureau
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.