From 6a8c0b51025314cdb1a8b4be24d45e690f1217dd Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 6 Jun 2019 17:37:57 +0200 Subject: [PATCH] qapi: Add feature flags to struct types Sometimes, the behaviour of QEMU changes without a 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 they can rely on the changed behavior. Let's add feature flags to the QAPI schema language, so that we can make such changes visible with schema introspection. An example for a schema definition using feature flags looks like this: { 'struct': 'TestType', 'data': { 'number': 'int' }, 'features': [ 'allow-negative-numbers' ] } Introspection information then looks like this: { "name": "TestType", "meta-type": "object", "members": [ { "name": "number", "type": "int" } ], "features": [ "allow-negative-numbers" ] } This patch implements feature flags only for struct types. We'll implement them more widely as needed. Signed-off-by: Kevin Wolf Message-Id: <20190606153803.5278-2-armbru@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- docs/devel/qapi-code-gen.txt | 38 +++++++++++++++ qapi/introspect.json | 6 ++- scripts/qapi/common.py | 66 ++++++++++++++++++++++---- scripts/qapi/doc.py | 3 +- scripts/qapi/introspect.py | 6 ++- scripts/qapi/types.py | 3 +- scripts/qapi/visit.py | 3 +- tests/qapi-schema/double-type.err | 2 +- tests/qapi-schema/test-qapi.py | 3 +- tests/qapi-schema/unknown-expr-key.err | 2 +- 10 files changed, 114 insertions(+), 18 deletions(-) diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt index b517b0cfbf..e8ec8ac1de 100644 --- a/docs/devel/qapi-code-gen.txt +++ b/docs/devel/qapi-code-gen.txt @@ -719,6 +719,34 @@ any non-empty complex type (struct, union, or alternate), and a pointer to that QAPI type is passed as a single argument. +=== Features === + +Sometimes, the behaviour of QEMU changes compatibly, but without a +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. + +In the schema, features can be specified as simple strings, for example: + +{ 'struct': 'TestType', + 'data': { 'number': 'int' }, + 'features': [ 'allow-negative-numbers' ] } + +Another option is to specify features as dictionaries, where the key +'name' specifies the feature string to be exposed to clients: + +{ 'struct': 'TestType', + 'data': { 'number': 'int' }, + 'features': [ { 'name': 'allow-negative-numbers' } ] } + +This expanded form is necessary if you want to make the feature +conditional (see below in "Configuring the schema"). + + === Downstream extensions === QAPI schema names that are externally visible, say in the Client JSON @@ -771,6 +799,16 @@ Example: a conditional 'bar' enum member. [ 'foo', { 'name' : 'bar', 'if': 'defined(IFCOND)' } ] } +Similarly, features can be specified as a dictionary with a 'name' and +an 'if' key. + +Example: a conditional 'allow-negative-numbers' feature + +{ 'struct': 'TestType', + 'data': { 'number': 'int' }, + 'features': [ { 'name': 'allow-negative-numbers', + 'if' 'defined(IFCOND)' } ] } + Please note that you are responsible to ensure that the C code will compile with an arbitrary combination of conditions, since the generators are unable to check it at this point. diff --git a/qapi/introspect.json b/qapi/introspect.json index 3d22166b2b..1843c1cb17 100644 --- a/qapi/introspect.json +++ b/qapi/introspect.json @@ -174,6 +174,9 @@ # and may even differ from the order of the values of the # enum type of the @tag. # +# @features: names of features associated with the type, in no particular +# order. (since: 4.1) +# # Values of this type are JSON object on the wire. # # Since: 2.5 @@ -181,7 +184,8 @@ { 'struct': 'SchemaInfoObject', 'data': { 'members': [ 'SchemaInfoObjectMember' ], '*tag': 'str', - '*variants': [ 'SchemaInfoObjectVariant' ] } } + '*variants': [ 'SchemaInfoObjectVariant' ], + '*features': [ 'str' ] } } ## # @SchemaInfoObjectMember: diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py index f07869ec73..9e4b6c00b5 100644 --- a/scripts/qapi/common.py +++ b/scripts/qapi/common.py @@ -886,12 +886,26 @@ def check_enum(expr, info): def check_struct(expr, info): name = expr['struct'] members = expr['data'] + features = expr.get('features') check_type(info, "'data' for struct '%s'" % name, members, allow_dict=True, allow_optional=True) check_type(info, "'base' for struct '%s'" % name, expr.get('base'), allow_metas=['struct']) + if features: + if not isinstance(features, list): + raise QAPISemError(info, + "Struct '%s' requires an array for 'features'" % + name) + for f in features: + assert isinstance(f, dict) + check_known_keys(info, "feature of struct %s" % name, f, + ['name'], ['if']) + + check_if(f, info) + check_name(info, "Feature of struct %s" % name, f['name']) + def check_known_keys(info, source, keys, required, optional): @@ -948,6 +962,12 @@ def normalize_members(members): members[key] = {'type': arg} +def normalize_features(features): + if isinstance(features, list): + features[:] = [f if isinstance(f, dict) else {'name': f} + for f in features] + + def check_exprs(exprs): global all_names @@ -986,8 +1006,10 @@ def check_exprs(exprs): normalize_members(expr['data']) elif 'struct' in expr: meta = 'struct' - check_keys(expr_elem, 'struct', ['data'], ['base', 'if']) + check_keys(expr_elem, 'struct', ['data'], + ['base', 'if', 'features']) normalize_members(expr['data']) + normalize_features(expr.get('features')) struct_types[expr[meta]] = expr elif 'command' in expr: meta = 'command' @@ -1126,10 +1148,12 @@ class QAPISchemaVisitor(object): def visit_array_type(self, name, info, ifcond, element_type): pass - def visit_object_type(self, name, info, ifcond, base, members, variants): + def visit_object_type(self, name, info, ifcond, base, members, variants, + features): pass - def visit_object_type_flat(self, name, info, ifcond, members, variants): + def visit_object_type_flat(self, name, info, ifcond, members, variants, + features): pass def visit_alternate_type(self, name, info, ifcond, variants): @@ -1290,7 +1314,7 @@ class QAPISchemaArrayType(QAPISchemaType): class QAPISchemaObjectType(QAPISchemaType): def __init__(self, name, info, doc, ifcond, - base, local_members, variants): + base, local_members, variants, features): # struct has local_members, optional base, and no variants # flat union has base, variants, and no local_members # simple union has local_members, variants, and no base @@ -1302,11 +1326,15 @@ class QAPISchemaObjectType(QAPISchemaType): if variants is not None: assert isinstance(variants, QAPISchemaObjectTypeVariants) variants.set_owner(name) + for f in features: + assert isinstance(f, QAPISchemaFeature) + f.set_owner(name) self._base_name = base self.base = None self.local_members = local_members self.variants = variants self.members = None + self.features = features def check(self, schema): QAPISchemaType.check(self, schema) @@ -1332,6 +1360,12 @@ class QAPISchemaObjectType(QAPISchemaType): self.variants.check(schema, seen) assert self.variants.tag_member in self.members self.variants.check_clash(self.info, seen) + + # Features are in a name space separate from members + seen = {} + for f in self.features: + f.check_clash(self.info, seen) + if self.doc: self.doc.check() @@ -1368,12 +1402,15 @@ class QAPISchemaObjectType(QAPISchemaType): def visit(self, visitor): visitor.visit_object_type(self.name, self.info, self.ifcond, - self.base, self.local_members, self.variants) + self.base, self.local_members, self.variants, + self.features) visitor.visit_object_type_flat(self.name, self.info, self.ifcond, - self.members, self.variants) + self.members, self.variants, + self.features) class QAPISchemaMember(object): + """ Represents object members, enum members and features """ role = 'member' def __init__(self, name, ifcond=None): @@ -1419,6 +1456,10 @@ class QAPISchemaMember(object): return "'%s' %s" % (self.name, self._pretty_owner()) +class QAPISchemaFeature(QAPISchemaMember): + role = 'feature' + + class QAPISchemaObjectTypeMember(QAPISchemaMember): def __init__(self, name, typ, optional, ifcond=None): QAPISchemaMember.__init__(self, name, ifcond) @@ -1675,7 +1716,7 @@ class QAPISchema(object): ('null', 'null', 'QNull' + pointer_suffix)]: self._def_builtin_type(*t) self.the_empty_object_type = QAPISchemaObjectType( - 'q_empty', None, None, None, None, [], None) + 'q_empty', None, None, None, None, [], None, []) self._def_entity(self.the_empty_object_type) qtypes = ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', @@ -1685,6 +1726,9 @@ class QAPISchema(object): self._def_entity(QAPISchemaEnumType('QType', None, None, None, qtype_values, 'QTYPE')) + def _make_features(self, features): + return [QAPISchemaFeature(f['name'], f.get('if')) for f in features] + def _make_enum_members(self, values): return [QAPISchemaMember(v['name'], v.get('if')) for v in values] @@ -1721,7 +1765,7 @@ class QAPISchema(object): assert ifcond == typ._ifcond # pylint: disable=protected-access else: self._def_entity(QAPISchemaObjectType(name, info, doc, ifcond, - None, members, None)) + None, members, None, [])) return name def _def_enum_type(self, expr, info, doc): @@ -1752,9 +1796,11 @@ class QAPISchema(object): base = expr.get('base') data = expr['data'] ifcond = expr.get('if') + features = expr.get('features', []) self._def_entity(QAPISchemaObjectType(name, info, doc, ifcond, base, self._make_members(data, info), - None)) + None, + self._make_features(features))) def _make_variant(self, case, typ, ifcond): return QAPISchemaObjectTypeVariant(case, typ, ifcond) @@ -1795,7 +1841,7 @@ class QAPISchema(object): QAPISchemaObjectType(name, info, doc, ifcond, base, members, QAPISchemaObjectTypeVariants(tag_name, tag_member, - variants))) + variants), [])) def _def_alternate_type(self, expr, info, doc): name = expr['alternate'] diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py index 5c8c136899..433e9fcbfb 100755 --- a/scripts/qapi/doc.py +++ b/scripts/qapi/doc.py @@ -220,7 +220,8 @@ class QAPISchemaGenDocVisitor(qapi.common.QAPISchemaVisitor): body=texi_entity(doc, 'Values', ifcond, member_func=texi_enum_value))) - def visit_object_type(self, name, info, ifcond, base, members, variants): + def visit_object_type(self, name, info, ifcond, base, members, variants, + features): doc = self.cur_doc if base and base.is_implicit(): base = None diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index f7f2ca07e4..f62cf0a2e1 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -188,11 +188,15 @@ const QLitObject %(c_name)s = %(c_string)s; self._gen_qlit('[' + element + ']', 'array', {'element-type': element}, ifcond) - def visit_object_type_flat(self, name, info, ifcond, members, variants): + def visit_object_type_flat(self, name, info, ifcond, members, variants, + features): obj = {'members': [self._gen_member(m) for m in members]} if variants: obj.update(self._gen_variants(variants.tag_member.name, variants.variants)) + if features: + obj['features'] = [(f.name, {'if': f.ifcond}) for f in features] + self._gen_qlit(name, 'object', obj, ifcond) def visit_alternate_type(self, name, info, ifcond, variants): diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py index 2bd6fcd44f..3edd9374aa 100644 --- a/scripts/qapi/types.py +++ b/scripts/qapi/types.py @@ -227,7 +227,8 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor): self._genh.add(gen_array(name, element_type)) self._gen_type_cleanup(name) - def visit_object_type(self, name, info, ifcond, base, members, variants): + def visit_object_type(self, name, info, ifcond, base, members, variants, + features): # Nothing to do for the special empty builtin if name == 'q_empty': return diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index fd356151d2..484ebb66ad 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -324,7 +324,8 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor): self._genh.add(gen_visit_decl(name)) self._genc.add(gen_visit_list(name, element_type)) - def visit_object_type(self, name, info, ifcond, base, members, variants): + def visit_object_type(self, name, info, ifcond, base, members, variants, + features): # Nothing to do for the special empty builtin if name == 'q_empty': return diff --git a/tests/qapi-schema/double-type.err b/tests/qapi-schema/double-type.err index 799193dba1..69457173a7 100644 --- a/tests/qapi-schema/double-type.err +++ b/tests/qapi-schema/double-type.err @@ -1,2 +1,2 @@ tests/qapi-schema/double-type.json:2: Unknown key 'command' in struct 'bar' -Valid keys are 'base', 'data', 'if', 'struct'. +Valid keys are 'base', 'data', 'features', 'if', 'struct'. diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py index d21fca01fc..f2d6815c86 100644 --- a/tests/qapi-schema/test-qapi.py +++ b/tests/qapi-schema/test-qapi.py @@ -38,7 +38,8 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor): print('array %s %s' % (name, element_type.name)) self._print_if(ifcond) - def visit_object_type(self, name, info, ifcond, base, members, variants): + def visit_object_type(self, name, info, ifcond, base, members, variants, + features): print('object %s' % name) if base: print(' base %s' % base.name) diff --git a/tests/qapi-schema/unknown-expr-key.err b/tests/qapi-schema/unknown-expr-key.err index 6ff8bb99c5..4340eaf894 100644 --- a/tests/qapi-schema/unknown-expr-key.err +++ b/tests/qapi-schema/unknown-expr-key.err @@ -1,2 +1,2 @@ tests/qapi-schema/unknown-expr-key.json:2: Unknown keys 'bogus', 'phony' in struct 'bar' -Valid keys are 'base', 'data', 'if', 'struct'. +Valid keys are 'base', 'data', 'features', 'if', 'struct'.