[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v8 00/14] monitor: Optionally run handlers in coroutines
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH v8 00/14] monitor: Optionally run handlers in coroutines |
Date: |
Thu, 8 Oct 2020 16:53:13 +0100 |
On Mon, Oct 05, 2020 at 05:58:41PM +0200, Kevin Wolf wrote:
> Some QMP command handlers can block the main loop for a relatively long
> time, for example because they perform some I/O. This is quite nasty.
> Allowing such handlers to run in a coroutine where they can yield (and
> therefore release the BQL) while waiting for an event such as I/O
> completion solves the problem.
>
> This series adds the infrastructure to allow this and switches
> block_resize to run in a coroutine as a first example.
>
> This is an alternative solution to Marc-André's "monitor: add
> asynchronous command type" series.
>
> v8:
> - Replaced bdrv_co_move_to_aio_context() with bdrv_co_enter()/leave(),
> added bdrv_co_lock()/unlock() and fixed the iothread support in
> qmp_block_resize() with the new functions [Stefan]
> - More detailed documentation of 'coroutine' flag for commands [Markus]
> - Improved comment for dropping out of coroutine context in
> qmp_dispatch() [Markus]
> - Coding style changes [Markus]
> - Resolve some more merge conflicts on rebase
>
> v7:
> - Added patch 2 to add a Monitor parameter to monitor_get_cpu_index(),
> too [Markus]
> - Let monitor_set_cur() return the old monitor [Markus]
> - Fixed comment about linking stub objects in test-util-sockets [Markus]
> - More detailed commit message for per-coroutine current monitor and
> document that monitor_set_cur(NULL) must be called eventually [Markus]
> - Resolve some merge conflicts on rebase
>
> v6:
> - Fixed cur_mon behaviour: It should always point to the Monitor while
> we're in the handler coroutine, but be NULL while the handler
> coroutines has yielded. [Markus]
> - Give HMP handlers the coroutine option, too, because they will call
> QMP handlers, and life is easier when we know whether we are in
> coroutine context or not.
> - Fixed block_resize for block devices in iothreads and for HMP
> - Resolved some merge conflict with QAPI generator and monitor
> refactorings that were merged in the meantime
>
> v5:
> - Improved comments and documentation [Markus]
>
> v4:
> - Forbid 'oob': true, 'coroutine': true [Markus]
> - Removed Python type hints [Markus]
> - Introduced separate bool qmp_dispatcher_co_shutdown to make it clearer
> how a shutdown request is signalled to the dispatcher [Markus]
> - Allow using aio_poll() with iohandler_ctx and use that instead of
> aio_bh_poll() [Markus]
> - Removed coroutine_fn from qmp_block_resize() again because at least
> one caller (HMP) calls it outside of coroutine context [Markus]
> - Tried to make the synchronisation between dispatcher and the monitor
> thread clearer, and fixed a race condition
> - Improved documentation and comments
>
> v3:
> - Fix race between monitor thread and dispatcher that could schedule the
> dispatcher coroutine twice if a second requests comes in before the
> dispatcher can wake up [Patchew]
>
> v2:
> - Fix typo in a commit message [Eric]
> - Use hyphen instead of underscore for the test command [Eric]
> - Mark qmp_block_resize() as coroutine_fn [Stefan]
>
>
> Kevin Wolf (14):
> monitor: Add Monitor parameter to monitor_set_cpu()
> monitor: Add Monitor parameter to monitor_get_cpu_index()
> monitor: Use getter/setter functions for cur_mon
> hmp: Update current monitor only in handle_hmp_command()
> qmp: Assert that no other monitor is active
> qmp: Call monitor_set_cur() only in qmp_dispatch()
> monitor: Make current monitor a per-coroutine property
> qapi: Add a 'coroutine' flag for commands
> qmp: Move dispatcher to a coroutine
> hmp: Add support for coroutine command handlers
> util/async: Add aio_co_reschedule_self()
> block: Add bdrv_co_enter()/leave()
> block: Add bdrv_lock()/unlock()
> block: Convert 'block_resize' to coroutine
>
> qapi/block-core.json | 3 +-
> docs/devel/qapi-code-gen.txt | 29 ++++++
> docs/sphinx/qapidoc.py | 2 +-
> include/block/aio.h | 10 ++
> include/block/block.h | 31 ++++++
> include/monitor/monitor.h | 7 +-
> include/qapi/qmp/dispatch.h | 5 +-
> monitor/monitor-internal.h | 7 +-
> audio/wavcapture.c | 8 +-
> block.c | 50 +++++++++
> blockdev.c | 16 +--
> dump/dump.c | 2 +-
> hw/core/machine-hmp-cmds.c | 2 +-
> hw/scsi/vhost-scsi.c | 2 +-
> hw/virtio/vhost-vsock.c | 2 +-
> migration/fd.c | 4 +-
> monitor/hmp-cmds.c | 4 +-
> monitor/hmp.c | 44 ++++++--
> monitor/misc.c | 38 +++----
> monitor/monitor.c | 101 ++++++++++++++++--
> monitor/qmp-cmds-control.c | 2 +
> monitor/qmp-cmds.c | 2 +-
> monitor/qmp.c | 131 +++++++++++++++++-------
> net/socket.c | 2 +-
> net/tap.c | 6 +-
> qapi/qmp-dispatch.c | 65 +++++++++++-
> qapi/qmp-registry.c | 3 +
> qga/main.c | 2 +-
> softmmu/cpus.c | 2 +-
> stubs/monitor-core.c | 10 +-
> tests/test-qmp-cmds.c | 10 +-
> tests/test-util-sockets.c | 12 +--
> trace/control.c | 2 +-
> util/aio-posix.c | 8 +-
> util/async.c | 30 ++++++
> util/qemu-error.c | 6 +-
> util/qemu-print.c | 3 +-
> util/qemu-sockets.c | 1 +
> scripts/qapi/commands.py | 10 +-
> scripts/qapi/expr.py | 11 +-
> scripts/qapi/introspect.py | 2 +-
> scripts/qapi/schema.py | 13 ++-
> tests/qapi-schema/test-qapi.py | 7 +-
> hmp-commands.hx | 1 +
> tests/qapi-schema/meson.build | 1 +
> tests/qapi-schema/oob-coroutine.err | 2 +
> tests/qapi-schema/oob-coroutine.json | 2 +
> tests/qapi-schema/oob-coroutine.out | 0
> tests/qapi-schema/qapi-schema-test.json | 1 +
> tests/qapi-schema/qapi-schema-test.out | 2 +
> 50 files changed, 573 insertions(+), 143 deletions(-)
> create mode 100644 tests/qapi-schema/oob-coroutine.err
> create mode 100644 tests/qapi-schema/oob-coroutine.json
> create mode 100644 tests/qapi-schema/oob-coroutine.out
>
> --
> 2.25.4
>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
signature.asc
Description: PGP signature
- [PATCH v8 06/14] qmp: Call monitor_set_cur() only in qmp_dispatch(), (continued)
- [PATCH v8 06/14] qmp: Call monitor_set_cur() only in qmp_dispatch(), Kevin Wolf, 2020/10/05
- [PATCH v8 07/14] monitor: Make current monitor a per-coroutine property, Kevin Wolf, 2020/10/05
- [PATCH v8 08/14] qapi: Add a 'coroutine' flag for commands, Kevin Wolf, 2020/10/05
- [PATCH v8 09/14] qmp: Move dispatcher to a coroutine, Kevin Wolf, 2020/10/05
- [PATCH v8 12/14] block: Add bdrv_co_enter()/leave(), Kevin Wolf, 2020/10/05
- [PATCH v8 11/14] util/async: Add aio_co_reschedule_self(), Kevin Wolf, 2020/10/05
- [PATCH v8 10/14] hmp: Add support for coroutine command handlers, Kevin Wolf, 2020/10/05
- [PATCH v8 13/14] block: Add bdrv_lock()/unlock(), Kevin Wolf, 2020/10/05
- [PATCH v8 14/14] block: Convert 'block_resize' to coroutine, Kevin Wolf, 2020/10/05
- Re: [PATCH v8 00/14] monitor: Optionally run handlers in coroutines, Markus Armbruster, 2020/10/06
- Re: [PATCH v8 00/14] monitor: Optionally run handlers in coroutines,
Stefan Hajnoczi <=