[v3,1/3] qapi: Add feature flags to commands in qapi

Similarly to features for struct types introduce the feature flags also
for commands. This will allow notifying management layers of fixes and
compatible changes in the behaviour of a command which may not be
detectable any other way.

The changes were heavily inspired by commit 6a8c0b51025.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 docs/devel/qapi-code-gen.txt   |  7 ++++---
 qapi/introspect.json           |  6 +++++-
 scripts/qapi/commands.py       |  3 ++-
 scripts/qapi/doc.py            |  3 ++-
 scripts/qapi/expr.py           | 17 ++++++++++++++++-
 scripts/qapi/introspect.py     |  7 ++++++-
 scripts/qapi/schema.py         | 22 ++++++++++++++++++----
 tests/qapi-schema/test-qapi.py |  3 ++-
 8 files changed, 55 insertions(+), 13 deletions(-)

Peter Krempa <pkrempa@redhat.com> writes:

> Similarly to features for struct types introduce the feature flags also
> for commands. This will allow notifying management layers of fixes and
> compatible changes in the behaviour of a command which may not be
> detectable any other way.
>
> The changes were heavily inspired by commit 6a8c0b51025.
>
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  docs/devel/qapi-code-gen.txt   |  7 ++++---
>  qapi/introspect.json           |  6 +++++-
>  scripts/qapi/commands.py       |  3 ++-
>  scripts/qapi/doc.py            |  3 ++-
>  scripts/qapi/expr.py           | 17 ++++++++++++++++-
>  scripts/qapi/introspect.py     |  7 ++++++-
>  scripts/qapi/schema.py         | 22 ++++++++++++++++++----
>  tests/qapi-schema/test-qapi.py |  3 ++-
>  8 files changed, 55 insertions(+), 13 deletions(-)
>
> diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
> index 64d9e4c6a9..637fa49977 100644
> --- a/docs/devel/qapi-code-gen.txt
> +++ b/docs/devel/qapi-code-gen.txt
> @@ -640,9 +640,10 @@ change in the QMP syntax (usually by allowing values or operations
>  that previously resulted in an error).  QMP clients may still need to
>  know whether the extension is available.
>
> -For this purpose, a list of features can be specified for a struct type.
> -This is exposed to the client as a list of string, where each string
> -signals that this build of QEMU shows a certain behaviour.
> +For this purpose, a list of features can be specified for a command or
> +struct type.  This is exposed to the client as a list of strings,
> +where each string signals that this build of QEMU shows a certain
> +behaviour.
>
>  Each member of the 'features' array defines a feature.  It can either
>  be { 'name': STRING, '*if': COND }, or STRING, which is shorthand for
> diff --git a/qapi/introspect.json b/qapi/introspect.json
> index 1843c1cb17..031a954fa9 100644
> --- a/qapi/introspect.json
> +++ b/qapi/introspect.json
> @@ -266,13 +266,17 @@
>  # @allow-oob: whether the command allows out-of-band execution,
>  #             defaults to false (Since: 2.12)
>  #
> +# @features: names of features associated with the command, in no particular
> +#            order. (since 4.2)
> +#
>  # TODO: @success-response (currently irrelevant, because it's QGA, not QMP)
>  #
>  # Since: 2.5
>  ##
>  { 'struct': 'SchemaInfoCommand',
>    'data': { 'arg-type': 'str', 'ret-type': 'str',
> -            '*allow-oob': 'bool' } }
> +            '*allow-oob': 'bool',
> +            '*features': [ 'str' ] } }
>
>  ##
>  # @SchemaInfoEvent:
> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
> index 898516b086..ab98e504f3 100644
> --- a/scripts/qapi/commands.py
> +++ b/scripts/qapi/commands.py
> @@ -277,7 +277,8 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
>          genc.add(gen_registry(self._regy.get_content(), self._prefix))
>
>      def visit_command(self, name, info, ifcond, arg_type, ret_type, gen,
> -                      success_response, boxed, allow_oob, allow_preconfig):
> +                      success_response, boxed, allow_oob, allow_preconfig,
> +                      features):
>          if not gen:
>              return
>          # FIXME: If T is a user-defined type, the user is responsible
> diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
> index dc8919bab7..8278ccbc43 100644
> --- a/scripts/qapi/doc.py
> +++ b/scripts/qapi/doc.py
> @@ -249,7 +249,8 @@ class QAPISchemaGenDocVisitor(QAPISchemaVisitor):
>                                 body=texi_entity(doc, 'Members', ifcond)))
>
>      def visit_command(self, name, info, ifcond, arg_type, ret_type, gen,
> -                      success_response, boxed, allow_oob, allow_preconfig):
> +                      success_response, boxed, allow_oob, allow_preconfig,
> +                      features):
>          doc = self.cur_doc
>          if boxed:
>              body = texi_body(doc)
> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> index da23063f57..129a4075f0 100644
> --- a/scripts/qapi/expr.py
> +++ b/scripts/qapi/expr.py
> @@ -277,12 +277,26 @@ def check_command(expr, info):
>      args = expr.get('data')
>      rets = expr.get('returns')
>      boxed = expr.get('boxed', False)
> +    features = expr.get('features')
>
>      if boxed and args is None:
>          raise QAPISemError(info, "'boxed': true requires 'data'")
>      check_type(args, info, "'data'", allow_dict=not boxed)
>      check_type(rets, info, "'returns'", allow_array=True)
>
> +    if features:
> +        if not isinstance(features, list):
> +            raise QAPISemError(info, "'features' must be an array")
> +        for f in features:
> +            source = "'features' member"
> +            assert isinstance(f, dict)
> +            check_keys(f, info, source, ['name'], ['if'])
> +            check_name_is_str(f['name'], info, source)
> +            source = "%s '%s'" % (source, f['name'])
> +            check_name_str(f['name'], info, source)
> +            check_if(f, info, source)
> +            normalize_if(f)
> +

Copied from check_struct().  Let's factor it into a helper function
check_features(features, info).

>
>  def check_event(expr, info):
>      args = expr.get('data')
> @@ -357,10 +371,11 @@ def check_exprs(exprs):
>          elif meta == 'command':
>              check_keys(expr, info, meta,
>                         ['command'],
> -                       ['data', 'returns', 'boxed', 'if',
> +                       ['data', 'returns', 'boxed', 'if', 'features',
>                          'gen', 'success-response', 'allow-oob',
>                          'allow-preconfig'])
>              normalize_members(expr.get('data'))
> +            normalize_features(expr.get('features'))
>              check_command(expr, info)
>          elif meta == 'event':
>              check_keys(expr, info, meta,
> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
> index d1c1ad346d..739b35ae8f 100644
> --- a/scripts/qapi/introspect.py
> +++ b/scripts/qapi/introspect.py
> @@ -209,13 +209,18 @@ const QLitObject %(c_name)s = %(c_string)s;
>                             for m in variants.variants]}, ifcond)
>
>      def visit_command(self, name, info, ifcond, arg_type, ret_type, gen,
> -                      success_response, boxed, allow_oob, allow_preconfig):
> +                      success_response, boxed, allow_oob, allow_preconfig,
> +                      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),
>                 'ret-type': self._use_type(ret_type)}
>          if allow_oob:
>              obj['allow-oob'] = allow_oob
> +
> +        if features:
> +            obj['features'] = [(f.name, {'if': f.ifcond}) for f in features]
> +

Copied from visit_object_type_flat().  Acceptable for now.

>          self._gen_qlit(name, 'command', obj, ifcond)
>
>      def visit_event(self, name, info, ifcond, arg_type, boxed):
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 38041098bd..8a48231766 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -109,7 +109,8 @@ class QAPISchemaVisitor(object):
>          pass
>
>      def visit_command(self, name, info, ifcond, arg_type, ret_type, gen,
> -                      success_response, boxed, allow_oob, allow_preconfig):
> +                      success_response, boxed, allow_oob, allow_preconfig,
> +                      features):
>          pass
>
>      def visit_event(self, name, info, ifcond, arg_type, boxed):
> @@ -658,10 +659,14 @@ class QAPISchemaCommand(QAPISchemaEntity):
>      meta = 'command'
>
>      def __init__(self, name, info, doc, ifcond, arg_type, ret_type,
> -                 gen, success_response, boxed, allow_oob, allow_preconfig):
> +                 gen, success_response, boxed, allow_oob, allow_preconfig,
> +                 features):
>          QAPISchemaEntity.__init__(self, name, info, doc, ifcond)
>          assert not arg_type or isinstance(arg_type, str)
>          assert not ret_type or isinstance(ret_type, str)
> +        for f in features:
> +            assert isinstance(f, QAPISchemaFeature)
> +            f.set_defined_in(name)

Copied from QAPISchemaObjectType.__init__().  Acceptable for now.

>          self._arg_type_name = arg_type
>          self.arg_type = None
>          self._ret_type_name = ret_type
> @@ -671,6 +676,7 @@ class QAPISchemaCommand(QAPISchemaEntity):
>          self.boxed = boxed
>          self.allow_oob = allow_oob
>          self.allow_preconfig = allow_preconfig
> +        self.features = features
>
>      def check(self, schema):
>          QAPISchemaEntity.check(self, schema)
> @@ -700,13 +706,19 @@ class QAPISchemaCommand(QAPISchemaEntity):
>                          "command's 'returns' cannot take %s"
>                          % self.ret_type.describe())
>
> +        # Features are in a name space separate from members
> +        seen = {}
> +        for f in self.features:
> +            f.check_clash(self.info, seen)
> +

Copied from QAPISchemaObjectType.check().  Acceptable for now.

>      def visit(self, visitor):
>          QAPISchemaEntity.visit(self, visitor)
>          visitor.visit_command(self.name, self.info, self.ifcond,
>                                self.arg_type, self.ret_type,
>                                self.gen, self.success_response,
>                                self.boxed, self.allow_oob,
> -                              self.allow_preconfig)
> +                              self.allow_preconfig,
> +                              self.features)
>
>
>  class QAPISchemaEvent(QAPISchemaEntity):
> @@ -983,6 +995,7 @@ class QAPISchema(object):
>          allow_oob = expr.get('allow-oob', False)
>          allow_preconfig = expr.get('allow-preconfig', False)
>          ifcond = expr.get('if')
> +        features = expr.get('features', [])
>          if isinstance(data, OrderedDict):
>              data = self._make_implicit_object_type(
>                  name, info, doc, ifcond, 'arg', self._make_members(data, info))
> @@ -991,7 +1004,8 @@ class QAPISchema(object):
>              rets = self._make_array_type(rets[0], info)
>          self._def_entity(QAPISchemaCommand(name, info, doc, ifcond, data, rets,
>                                             gen, success_response,
> -                                           boxed, allow_oob, allow_preconfig))
> +                                           boxed, allow_oob, allow_preconfig,
> +                                           self._make_features(features, info)))
>
>      def _def_event(self, expr, info, doc):
>          name = expr['event']
> diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
> index 664254618a..e13c2e8671 100755
> --- a/tests/qapi-schema/test-qapi.py
> +++ b/tests/qapi-schema/test-qapi.py
> @@ -72,7 +72,8 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
>          self._print_if(ifcond)
>
>      def visit_command(self, name, info, ifcond, arg_type, ret_type, gen,
> -                      success_response, boxed, allow_oob, allow_preconfig):
> +                      success_response, boxed, allow_oob, allow_preconfig,
> +                      features):
>          print('command %s %s -> %s'
>                % (name, arg_type and arg_type.name,
>                   ret_type and ret_type.name))

v2 had:

           print('   gen=%s success_response=%s boxed=%s oob=%s preconfig=%s'
                 % (gen, success_response, boxed, allow_oob, allow_preconfig))
           self._print_if(ifcond)
  +        if features:
  +            for f in features:
  +                print('    feature %s' % f.name)
  +                self._print_if(f.ifcond, 8)

       def visit_event(self, name, info, ifcond, arg_type, boxed):
           print('event %s %s' % (name, arg_type and arg_type.name))

I see this is now in PATCH 2.  Okay.