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