| Message ID | b8559eac94436f327aa2e418c0bf8fc1fe032807.1570705279.git.pkrempa@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qapi: Add detection for the 'savevm' fix for blockdev | expand |
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.
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) + 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] + 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) 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) + 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))
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(-)