Hi,
HMP and QMP commands are handled synchronously in qemu today. But
there are benefits allowing the command handler to re-enter the main
loop if the command cannot be handled synchronously, or if it is
long-lasting. Some bugs such as rhbz#1230527 are difficult to solve
without it.
The common solution is to use a pair of command+event in this case.
But this approach has a number of issues:
- you can't "fix" an existing command: you need a new API, and ad-hoc
documentation for that command+signal association, and old/broken
command deprecation
- since the reply event is broadcasted and 'id' is used for matching the
request, it may conflict with other clients request 'id' space
- it is arguably less efficient and elegant (weird API, useless return
in most cases, broadcast reply, no cancelling on disconnect etc)
The following series implements an internal async command solution
instead. By introducing a session context and a command return
handler, QMP handlers can:
- defer the return, allowing the mainloop to reenter
- return only to the caller (instead of the broadcast event reply)
- optionnally allow cancellation when the client is gone
- track on-going qapi command(s) per session
This does not introduce new QMP APIs or client visible changes, the
command are handled in order, and the reply still come in order (even
when handlers finish out of order).
Existing qemu commands can be gradually replaced by async:true
variants when needed, while 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.
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). It could be further improved to do asynchronous
IO writes as well.
v6:
- includes PPM save code improvements (will be sent seperatly,
included for convenience for now)
- remove qmp_return_get_monitor() need, which is problematic with the
QMP session over HMP monitor ("monitor: teach HMP about asynchronous
commands")
- rebased
v5:
- rebased
v4:
- rebased, mostly adapting to new OOB code
(there was not much feedback in v3 for the async command part,
but preliminary patches got merged!)
- drop the RFC status
v3:
- complete rework, dropping the asynchronous commands visibility from
the protocol side entirely (until there is a real need for it)
- rebased, with a few preliminary cleanup patches
- teach asynchronous commands to HMP
v2:
- documentation fixes and improvements
- fix calling async commands sync without id
- fix bad hmp monitor assert
- add a few extra asserts
- add async with no-id failure and screendump test
Marc-André Lureau (25):
qmp: constify QmpCommand and list
json-lexer: make it safe to call destroy multiple times
qmp: add QmpSession
QmpSession: add a return callback
QmpSession: add json parser and use it in qga
monitor: use qmp session to parse json feed
qga: simplify dispatch_return_cb
QmpSession: introduce QmpReturn
qmp: simplify qmp_return_error()
QmpSession: keep a queue of pending commands
QmpSession: return orderly
qmp: introduce asynchronous command type
scripts: learn 'async' qapi commands
qmp: add qmp_return_is_cancelled()
console: add graphic_hw_update_done()
ppm-save: pass opened fd
ui: add pixman image g_autoptr support
object: add g_autoptr support
screendump: replace FILE with QIOChannel and fix close()/qemu_close()
osdep: add qemu_unlink()
screendump: use qemu_unlink()
console: make screendump asynchronous
monitor: start making qmp_human_monitor_command() asynchronous
monitor: teach HMP about asynchronous commands
hmp: call the asynchronous QMP screendump to fix outdated/glitches
hmp-commands.hx | 3 +-
hw/display/qxl-render.c | 9 +-
hw/display/qxl.c | 1 +
include/qapi/qmp/dispatch.h | 89 +++++++++-
include/qapi/qmp/json-parser.h | 7 +-
include/qemu/osdep.h | 1 +
include/qom/object.h | 3 +
include/ui/console.h | 4 +
include/ui/qemu-pixman.h | 2 +
monitor/hmp-cmds.c | 6 +-
monitor/hmp.c | 110 +++++++++++-
monitor/misc.c | 46 +----
monitor/monitor-internal.h | 12 +-
monitor/monitor.c | 2 +-
monitor/qmp.c | 64 +++----
qapi/misc.json | 3 +-
qapi/qmp-dispatch.c | 214 +++++++++++++++++++-----
qapi/qmp-registry.c | 33 +++-
qapi/ui.json | 3 +-
qga/commands.c | 2 +-
qga/main.c | 51 ++----
qobject/json-lexer.c | 5 +-
qobject/json-streamer.c | 3 +-
scripts/qapi/commands.py | 149 ++++++++++++++---
scripts/qapi/doc.py | 2 +-
scripts/qapi/expr.py | 4 +-
scripts/qapi/introspect.py | 2 +-
scripts/qapi/schema.py | 11 +-
tests/qapi-schema/qapi-schema-test.json | 5 +
tests/qapi-schema/qapi-schema-test.out | 8 +
tests/qapi-schema/test-qapi.py | 7 +-
tests/test-qmp-cmds.c | 206 +++++++++++++++++++----
ui/console.c | 165 ++++++++++++------
ui/trace-events | 2 +-
util/osdep.c | 15 ++
35 files changed, 937 insertions(+), 312 deletions(-)
--
2.24.0
