Re: [PATCH v7 08/13] qapi: Add a 'coroutine' flag for commands

[Markus Armbruster]

Additional nitpick detail on Kevin's request.

Kevin Wolf <kwolf@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 <kwolf@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
[...]
> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> index 2942520399..928cd1eb5c 100644
> --- a/scripts/qapi/expr.py
> +++ b/scripts/qapi/expr.py
> @@ -88,10 +88,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.
> +        raise QAPISemError(info, "flags 'allow-oob' and 'coroutine' "
> +                                 "are incompatible")

PEP 8: For flowing long blocks of text with fewer structural
restrictions (docstrings or comments), the line length should be limited
to 72 characters.

PEP 8: You should use two spaces after a sentence-ending period in
multi- sentence comments, except after the final sentence.

>  
>  
>  def check_if(expr, info, source):
[...]