Kevin Wolf <kw...@redhat.com> writes:
> This patch adds a new 'coroutine' flag to QMP command definitions that
> tells the QMP dispatcher that the command handler is safe to be run in a
> coroutine.
>
> The documentation of the new flag pretends that this flag is already
> used as intended, which it isn't yet after this patch. We'll implement
> this in another patch in this series.
>
> Signed-off-by: Kevin Wolf <kw...@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> Reviewed-by: Markus Armbruster <arm...@redhat.com>
> ---
> docs/devel/qapi-code-gen.txt | 11 +++++++++++
> include/qapi/qmp/dispatch.h | 1 +
> tests/test-qmp-cmds.c | 4 ++++
> scripts/qapi/commands.py | 10 +++++++---
> scripts/qapi/doc.py | 2 +-
> scripts/qapi/expr.py | 10 ++++++++--
> scripts/qapi/introspect.py | 2 +-
> scripts/qapi/schema.py | 9 ++++++---
> tests/qapi-schema/test-qapi.py | 7 ++++---
> tests/Makefile.include | 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 ++
> 15 files changed, 51 insertions(+), 13 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
>
> diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
> index 59d6973e1e..df01bd852b 100644
> --- a/docs/devel/qapi-code-gen.txt
> +++ b/docs/devel/qapi-code-gen.txt
> @@ -457,6 +457,7 @@ Syntax:
> '*gen': false,
> '*allow-oob': true,
> '*allow-preconfig': true,
> + '*coroutine': true,
> '*if': COND,
> '*features': FEATURES }
>
> @@ -581,6 +582,16 @@ before the machine is built. It defaults to false. For
> example:
> QMP is available before the machine is built only when QEMU was
> started with --preconfig.
>
> +Member 'coroutine' tells the QMP dispatcher whether the command handler
> +is safe to be run in a coroutine. It defaults to false. If it is true,
> +the command handler is called from coroutine context and may yield while
> +waiting for an external event (such as I/O completion) in order to avoid
> +blocking the guest and other background operations.
> +
> +It is an error to specify both 'coroutine': true and 'allow-oob': true
> +for a command. (We don't currently have a use case for both together and
> +without a use case, it's not entirely clear what the semantics should be.)
The parenthesis is new since v4. I like its contents. I'm not fond of
putting complete sentences in parenthesis. I'll drop them, if you don't
mind.
> +
> The optional 'if' member specifies a conditional. See "Configuring
> the schema" below for more on this.
>
[...]
> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> index d7a289eded..95cc006cae 100644
> --- a/scripts/qapi/expr.py
> +++ b/scripts/qapi/expr.py
> @@ -89,10 +89,16 @@ def check_flags(expr, info):
> if key in expr and expr[key] is not False:
> raise QAPISemError(
> info, "flag '%s' may only use false value" % key)
> - for key in ['boxed', 'allow-oob', 'allow-preconfig']:
> + for key in ['boxed', 'allow-oob', 'allow-preconfig', 'coroutine']:
> if key in expr and expr[key] is not True:
> raise QAPISemError(
> info, "flag '%s' may only use true value" % key)
> + if 'allow-oob' in expr and 'coroutine' in expr:
> + # This is not necessarily a fundamental incompatibility, but we don't
> + # have a use case and the desired semantics isn't obvious. The
> simplest
> + # solution is to forbid it until we get a use case for it.
Appreciated. I'll word-wrap, if you don't mind.
> + raise QAPISemError(info, "flags 'allow-oob' and 'coroutine' "
> + "are incompatible")
>
>
> def check_if(expr, info, source):
> @@ -344,7 +350,7 @@ def check_exprs(exprs):
> ['command'],
> ['data', 'returns', 'boxed', 'if', 'features',
> 'gen', 'success-response', 'allow-oob',
> - 'allow-preconfig'])
> + 'allow-preconfig', 'coroutine'])
> normalize_members(expr.get('data'))
> check_command(expr, info)
> elif meta == 'event':
[...]
R-by stands.
