Re: [Qemu-devel] [PATCH v3 00/38] RFC: monitor: add asynchronous command type

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> Hi,

Without having gone too deep here, it looks like you've got a mix
of cleanups and adding the async stuff.
You might want to split it into the set of simple cleanups first.

Dave

> One of initial design goals of QMP was to have "asynchronous command
> completion" (http://wiki.qemu.org/Features/QAPI). Unfortunately, that
> goal was not fully achieved, and some broken bits left were removed
> progressively until commit 65207c59d that removed async command
> support.
> 
> Yet there are clear benefits allowing 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 difficult to solve.  Furthermore, many QMP
> commands do IO and could be considered 'slow' and blocking the main
> loop today. The unwritten solution so far is to use a pair of
> command+event. But this approach has a number of issues, in particular
> to fix existing commands, and inadequacy since the event is
> broadcasted and may thus have command 'id' conflict, beside being
> rather inefficient and incorrect.
> 
> The following series implements an async command solution instead. By
> introducing a session context and a command return handler, it can:
> - defer the return, allowing the mainloop to reenter
> - return only to the caller (no broadcasted event)
> - optionnally allow cancellation when the client is gone
> - track on-going qapi command(s) per client/session
> 
> 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, which allows for step-by-step conversion.
> 
> 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 write as well.
> 
> 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 (38):
>   HACK: add back OOB
>   qmp-shell: learn to send commands with quoted arguments
>   Revert "qmp: isolate responses into io thread"
>   monitor: no need to save need_resume
>   monitor: further simplify previous patch
>   monitor: no need to remove desc before replacing it
>   json-parser: always set an error if return NULL
>   json-lexer: make it safe to call multiple times
>   json: remove useless return value from lexer/parser
>   tests: add a few qemu-qmp tests
>   tests: change /0.15/* tests to /qmp/*
>   tests: add a qmp success-response test
>   qga: process_event() simplification
>   monitor: simplify monitor_qmp_respond()
>   qmp: pass and return a QDict to qmp_dispatch()
>   qmp: move 'id' copy to qmp_dispatch()
>   qmp: constify qmp_is_oob()
>   qmp: add QmpSession
>   QmpSession: add a return_cb
>   QmpSession: add json parser and use it in qga
>   QmpSession: add a dispatch callback
>   monitor: use QmpSession parsing and common dispatch code
>   QmpSession: introduce QmpReturn
>   qmp: remove qmp_build_error_object()
>   qmp: remove need for qobject_from_jsonf()
>   qmp: fold do_qmp_dispatch() in qmp_dispatch()
>   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()
>   monitor: add qmp_return_get_monitor()
>   console: graphic_hw_update return true if async
>   console: add graphic_hw_update_done()
>   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
> 
>  qapi/misc.json                          |   3 +-
>  qapi/ui.json                            |   3 +-
>  scripts/qapi/commands.py                | 149 +++++++--
>  scripts/qapi/common.py                  |  12 +-
>  scripts/qapi/doc.py                     |   2 +-
>  scripts/qapi/introspect.py              |   2 +-
>  hmp.h                                   |   3 +-
>  hw/display/qxl.h                        |   2 +-
>  include/monitor/monitor.h               |   3 +
>  include/qapi/qmp/dispatch.h             |  85 ++++-
>  include/qapi/qmp/json-lexer.h           |   4 +-
>  include/qapi/qmp/json-streamer.h        |   4 +-
>  include/ui/console.h                    |   7 +-
>  hmp.c                                   |   6 +-
>  hw/display/qxl-render.c                 |  14 +-
>  hw/display/qxl.c                        |   8 +-
>  migration/postcopy-ram.c                |   1 +
>  monitor.c                               | 393 +++++++++---------------
>  qapi/qmp-dispatch.c                     | 234 +++++++++++---
>  qapi/qmp-registry.c                     |  27 +-
>  qga/main.c                              |  73 +----
>  qobject/json-lexer.c                    |  28 +-
>  qobject/json-parser.c                   |   7 +-
>  qobject/json-streamer.c                 |   8 +-
>  tests/qmp-test.c                        | 146 ++++++++-
>  tests/test-qmp-cmds.c                   | 206 +++++++++++--
>  ui/console.c                            | 106 ++++++-
>  hmp-commands.hx                         |   3 +-
>  scripts/qmp/qmp-shell                   |   3 +-
>  tests/Makefile.include                  |   6 +-
>  tests/qapi-schema/qapi-schema-test.json |   7 +
>  tests/qapi-schema/qapi-schema-test.out  |  10 +
>  tests/qapi-schema/test-qapi.py          |   7 +-
>  33 files changed, 1078 insertions(+), 494 deletions(-)
>  mode change 100644 => 100755 scripts/qapi/doc.py
> 
> -- 
> 2.17.0.rc1.1.g4c4f2b46a3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK