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

[Markus Armbruster]

Hit send too fast.

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/schema.py b/scripts/qapi/schema.py
> index 78309a00f0..c44d391c3f 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -128,7 +128,7 @@ class QAPISchemaVisitor:
>  
>      def visit_command(self, name, info, ifcond, features,
>                        arg_type, ret_type, gen, success_response, boxed,
> -                      allow_oob, allow_preconfig):
> +                      allow_oob, allow_preconfig, coroutine):
>          pass
>  
>      def visit_event(self, name, info, ifcond, features, arg_type, boxed):
> @@ -713,7 +713,8 @@ class QAPISchemaCommand(QAPISchemaEntity):
>  
>      def __init__(self, name, info, doc, ifcond, features,
>                   arg_type, ret_type,
> -                 gen, success_response, boxed, allow_oob, allow_preconfig):
> +                 gen, success_response, boxed, allow_oob, allow_preconfig,
> +                 coroutine):
>          super().__init__(name, info, doc, ifcond, features)
>          assert not arg_type or isinstance(arg_type, str)
>          assert not ret_type or isinstance(ret_type, str)
> @@ -726,6 +727,7 @@ class QAPISchemaCommand(QAPISchemaEntity):
>          self.boxed = boxed
>          self.allow_oob = allow_oob
>          self.allow_preconfig = allow_preconfig
> +        self.coroutine = coroutine
>  
>      def check(self, schema):
>          super().check(schema)
> @@ -768,7 +770,7 @@ class QAPISchemaCommand(QAPISchemaEntity):
>          visitor.visit_command(
>              self.name, self.info, self.ifcond, self.features,
>              self.arg_type, self.ret_type, self.gen, self.success_response,
> -            self.boxed, self.allow_oob, self.allow_preconfig)
> +            self.boxed, self.allow_oob, self.allow_preconfig, self.coroutine)

Recommend to break the line after preconfig, like you do elsewhere.

>  
>  
>  class QAPISchemaEvent(QAPISchemaEntity):
> @@ -1074,6 +1076,7 @@ class QAPISchema:
>          boxed = expr.get('boxed', False)
>          allow_oob = expr.get('allow-oob', False)
>          allow_preconfig = expr.get('allow-preconfig', False)
> +        coroutine = expr.get('coroutine', False)
>          ifcond = expr.get('if')
>          features = self._make_features(expr.get('features'), info)
>          if isinstance(data, OrderedDict):
> @@ -1086,7 +1089,8 @@ class QAPISchema:
>          self._def_entity(QAPISchemaCommand(name, info, doc, ifcond, features,
>                                             data, rets,
>                                             gen, success_response,
> -                                           boxed, allow_oob, 
> allow_preconfig))
> +                                           boxed, allow_oob, allow_preconfig,
> +                                           coroutine))

Preexisting: the arguments are kind of squeezed onto the right margin.
Hanging indent would avoid that.  Feel free to ignore.

>  
>      def _def_event(self, expr, info, doc):
>          name = expr['event']
[...]