Re: [PATCH 1/4] qapi: Add a 'coroutine' flag for commands

[Eric Blake <eblake@redhat.com>]

On 1/9/20 12:35 PM, Kevin Wolf wrote:
> This patch adds a new 'coroutine' flag to QMP command definitions that
> tells the QMP dispatcher than the command handler is safe to be run in a

s/than/that/

> coroutine.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   tests/qapi-schema/qapi-schema-test.json |  1 +
>   docs/devel/qapi-code-gen.txt            |  4 ++++
>   include/qapi/qmp/dispatch.h             |  1 +
>   tests/test-qmp-cmds.c                   |  4 ++++
>   scripts/qapi/commands.py                | 17 +++++++++++------
>   scripts/qapi/doc.py                     |  2 +-
>   scripts/qapi/expr.py                    |  4 ++--
>   scripts/qapi/introspect.py              |  2 +-
>   scripts/qapi/schema.py                  |  9 ++++++---
>   tests/qapi-schema/qapi-schema-test.out  |  2 ++
>   tests/qapi-schema/test-qapi.py          |  7 ++++---
>   11 files changed, 37 insertions(+), 16 deletions(-)
> 
> diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> index 9abf175fe0..55f596dbaa 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -147,6 +147,7 @@
>     'returns': 'UserDefTwo' }
>   
>   { 'command': 'cmd-success-response', 'data': {}, 'success-response': false }
> +{ 'command': 'coroutine_cmd', 'data': {}, 'coroutine': true }

Not user-visible (it's the testsuite), but why not follow our naming 
convention of 'coroutine-cmd'?


> +++ 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,9 @@ 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.
> +
>   The optional 'if' member specifies a conditional.  See "Configuring

Maybe "The optional 'coroutine' member tells..." for symmetry with the 
next paragraph.

> +++ b/scripts/qapi/commands.py
> @@ -15,6 +15,7 @@ See the COPYING file in the top-level directory.
>   
>   from qapi.common import *
>   from qapi.gen import QAPIGenCCode, QAPISchemaModularCVisitor, ifcontext
> +from typing import List
>   
>   
>   def gen_command_decl(name, arg_type, boxed, ret_type):
> @@ -194,8 +195,9 @@ out:
>       return ret
>   
>   
> -def gen_register_command(name, success_response, allow_oob, allow_preconfig):
> -    options = []
> +def gen_register_command(name: str, success_response: bool, allow_oob: bool,
> +                         allow_preconfig: bool, coroutine: bool) -> str:
> +    options = [] # type: List[str]

Aha - now that we require python 3, you're going to exploit it ;)


> +++ b/scripts/qapi/introspect.py
> @@ -212,7 +212,7 @@ const QLitObject %(c_name)s = %(c_string)s;
>   
>       def visit_command(self, name, info, ifcond, arg_type, ret_type, gen,
>                         success_response, boxed, allow_oob, allow_preconfig,
> -                      features):
> +                      coroutine, features):
>           arg_type = arg_type or self._schema.the_empty_object_type
>           ret_type = ret_type or self._schema.the_empty_object_type
>           obj = {'arg-type': self._use_type(arg_type),

I'm assuming the new flag is internal only, and doesn't affect behavior 
seen by the user, and thus nothing to change in the introspection output.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org