From e6c42b96b9a0fa58cf49bb85cdf473d87fabbeb6 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Oct 2019 09:43:44 +0200 Subject: qapi: Split up scripts/qapi/common.py The QAPI code generator clocks in at some 3100 SLOC in 8 source files. Almost 60% of the code is in qapi/common.py. Split it into more focused modules: * Move QAPISchemaPragma and QAPISourceInfo to qapi/source.py. * Move QAPIError and its sub-classes to qapi/error.py. * Move QAPISchemaParser and QAPIDoc to parser.py. Use the opportunity to put QAPISchemaParser first. * Move check_expr() & friends to qapi/expr.py. Use the opportunity to put the code into a more sensible order. * Move QAPISchema & friends to qapi/schema.py * Move QAPIGen and its sub-classes, ifcontext, QAPISchemaModularCVisitor, and QAPISchemaModularCVisitor to qapi/gen.py * Delete camel_case(), it's unused since commit e98859a9b9 "qapi: Clean up after recent conversions to QAPISchemaVisitor" A number of helper functions remain in qapi/common.py. I considered moving the code generator helpers to qapi/gen.py, but decided not to. Perhaps we should rewrite them as methods of QAPIGen some day. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20191018074345.24034-7-armbru@redhat.com> [Add "# -*- coding: utf-8 -*-" lines] --- scripts/qapi/expr.py | 378 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 378 insertions(+) create mode 100644 scripts/qapi/expr.py (limited to 'scripts/qapi/expr.py') diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py new file mode 100644 index 0000000000..67cb2c2b6c --- /dev/null +++ b/scripts/qapi/expr.py @@ -0,0 +1,378 @@ +# -*- coding: utf-8 -*- +# +# Check (context-free) QAPI schema expression structure +# +# Copyright IBM, Corp. 2011 +# Copyright (c) 2013-2019 Red Hat Inc. +# +# Authors: +# Anthony Liguori +# Markus Armbruster +# Eric Blake +# Marc-André Lureau +# +# This work is licensed under the terms of the GNU GPL, version 2. +# See the COPYING file in the top-level directory. + +import re +from collections import OrderedDict +from qapi.common import c_name +from qapi.error import QAPISemError + + +# Names must be letters, numbers, -, and _. They must start with letter, +# except for downstream extensions which must start with __RFQDN_. +# Dots are only valid in the downstream extension prefix. +valid_name = re.compile(r'^(__[a-zA-Z0-9.-]+_)?' + '[a-zA-Z][a-zA-Z0-9_-]*$') + + +def check_name_is_str(name, info, source): + if not isinstance(name, str): + raise QAPISemError(info, "%s requires a string name" % source) + + +def check_name_str(name, info, source, + allow_optional=False, enum_member=False, + permit_upper=False): + global valid_name + membername = name + + if allow_optional and name.startswith('*'): + membername = name[1:] + # Enum members can start with a digit, because the generated C + # code always prefixes it with the enum name + if enum_member and membername[0].isdigit(): + membername = 'D' + membername + # Reserve the entire 'q_' namespace for c_name(), and for 'q_empty' + # and 'q_obj_*' implicit type names. + if not valid_name.match(membername) or \ + c_name(membername, False).startswith('q_'): + raise QAPISemError(info, "%s has an invalid name" % source) + if not permit_upper and name.lower() != name: + raise QAPISemError( + info, "%s uses uppercase in name" % source) + assert not membername.startswith('*') + + +def check_defn_name_str(name, info, meta): + check_name_str(name, info, meta, permit_upper=True) + if name.endswith('Kind') or name.endswith('List'): + raise QAPISemError( + info, "%s name should not end in '%s'" % (meta, name[-4:])) + + +def check_keys(value, info, source, required, optional): + + def pprint(elems): + return ', '.join("'" + e + "'" for e in sorted(elems)) + + missing = set(required) - set(value) + if missing: + raise QAPISemError( + info, + "%s misses key%s %s" + % (source, 's' if len(missing) > 1 else '', + pprint(missing))) + allowed = set(required + optional) + unknown = set(value) - allowed + if unknown: + raise QAPISemError( + info, + "%s has unknown key%s %s\nValid keys are %s." + % (source, 's' if len(unknown) > 1 else '', + pprint(unknown), pprint(allowed))) + + +def check_flags(expr, info): + for key in ['gen', 'success-response']: + 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']: + if key in expr and expr[key] is not True: + raise QAPISemError( + info, "flag '%s' may only use true value" % key) + + +def normalize_if(expr): + ifcond = expr.get('if') + if isinstance(ifcond, str): + expr['if'] = [ifcond] + + +def check_if(expr, info, source): + + def check_if_str(ifcond, info): + if not isinstance(ifcond, str): + raise QAPISemError( + info, + "'if' condition of %s must be a string or a list of strings" + % source) + if ifcond.strip() == '': + raise QAPISemError( + info, + "'if' condition '%s' of %s makes no sense" + % (ifcond, source)) + + ifcond = expr.get('if') + if ifcond is None: + return + if isinstance(ifcond, list): + if ifcond == []: + raise QAPISemError( + info, "'if' condition [] of %s is useless" % source) + for elt in ifcond: + check_if_str(elt, info) + else: + check_if_str(ifcond, info) + + +def normalize_members(members): + if isinstance(members, OrderedDict): + for key, arg in members.items(): + if isinstance(arg, dict): + continue + members[key] = {'type': arg} + + +def check_type(value, info, source, + allow_array=False, allow_dict=False): + if value is None: + return + + # Array type + if isinstance(value, list): + if not allow_array: + raise QAPISemError(info, "%s cannot be an array" % source) + if len(value) != 1 or not isinstance(value[0], str): + raise QAPISemError(info, + "%s: array type must contain single type name" % + source) + return + + # Type name + if isinstance(value, str): + return + + # Anonymous type + + if not allow_dict: + raise QAPISemError(info, "%s should be a type name" % source) + + if not isinstance(value, OrderedDict): + raise QAPISemError(info, + "%s should be an object or type name" % source) + + permit_upper = allow_dict in info.pragma.name_case_whitelist + + # value is a dictionary, check that each member is okay + for (key, arg) in value.items(): + key_source = "%s member '%s'" % (source, key) + check_name_str(key, info, key_source, + allow_optional=True, permit_upper=permit_upper) + if c_name(key, False) == 'u' or c_name(key, False).startswith('has_'): + raise QAPISemError(info, "%s uses reserved name" % key_source) + check_keys(arg, info, key_source, ['type'], ['if']) + check_if(arg, info, key_source) + normalize_if(arg) + check_type(arg['type'], info, key_source, allow_array=True) + + +def normalize_features(features): + if isinstance(features, list): + features[:] = [f if isinstance(f, dict) else {'name': f} + for f in features] + + +def normalize_enum(expr): + if isinstance(expr['data'], list): + expr['data'] = [m if isinstance(m, dict) else {'name': m} + for m in expr['data']] + + +def check_enum(expr, info): + name = expr['enum'] + members = expr['data'] + prefix = expr.get('prefix') + + if not isinstance(members, list): + raise QAPISemError(info, "'data' must be an array") + if prefix is not None and not isinstance(prefix, str): + raise QAPISemError(info, "'prefix' must be a string") + + permit_upper = name in info.pragma.name_case_whitelist + + for member in members: + source = "'data' member" + check_keys(member, info, source, ['name'], ['if']) + check_name_is_str(member['name'], info, source) + source = "%s '%s'" % (source, member['name']) + check_name_str(member['name'], info, source, + enum_member=True, permit_upper=permit_upper) + check_if(member, info, source) + normalize_if(member) + + +def check_struct(expr, info): + name = expr['struct'] + members = expr['data'] + features = expr.get('features') + + check_type(members, info, "'data'", allow_dict=name) + check_type(expr.get('base'), info, "'base'") + + 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_union(expr, info): + name = expr['union'] + base = expr.get('base') + discriminator = expr.get('discriminator') + members = expr['data'] + + if discriminator is None: # simple union + if base is not None: + raise QAPISemError(info, "'base' requires 'discriminator'") + else: # flat union + check_type(base, info, "'base'", allow_dict=name) + if not base: + raise QAPISemError(info, "'discriminator' requires 'base'") + check_name_is_str(discriminator, info, "'discriminator'") + + for (key, value) in members.items(): + source = "'data' member '%s'" % key + check_name_str(key, info, source) + check_keys(value, info, source, ['type'], ['if']) + check_if(value, info, source) + normalize_if(value) + check_type(value['type'], info, source, allow_array=not base) + + +def check_alternate(expr, info): + members = expr['data'] + + if len(members) == 0: + raise QAPISemError(info, "'data' must not be empty") + for (key, value) in members.items(): + source = "'data' member '%s'" % key + check_name_str(key, info, source) + check_keys(value, info, source, ['type'], ['if']) + check_if(value, info, source) + normalize_if(value) + check_type(value['type'], info, source) + + +def check_command(expr, info): + args = expr.get('data') + rets = expr.get('returns') + boxed = expr.get('boxed', False) + + 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) + + +def check_event(expr, info): + args = expr.get('data') + boxed = expr.get('boxed', False) + + if boxed and args is None: + raise QAPISemError(info, "'boxed': true requires 'data'") + check_type(args, info, "'data'", allow_dict=not boxed) + + +def check_exprs(exprs): + for expr_elem in exprs: + expr = expr_elem['expr'] + info = expr_elem['info'] + doc = expr_elem.get('doc') + + if 'include' in expr: + continue + + if 'enum' in expr: + meta = 'enum' + elif 'union' in expr: + meta = 'union' + elif 'alternate' in expr: + meta = 'alternate' + elif 'struct' in expr: + meta = 'struct' + elif 'command' in expr: + meta = 'command' + elif 'event' in expr: + meta = 'event' + else: + raise QAPISemError(info, "expression is missing metatype") + + name = expr[meta] + check_name_is_str(name, info, "'%s'" % meta) + info.set_defn(meta, name) + check_defn_name_str(name, info, meta) + + if doc: + if doc.symbol != name: + raise QAPISemError( + info, "documentation comment is for '%s'" % doc.symbol) + doc.check_expr(expr) + elif info.pragma.doc_required: + raise QAPISemError(info, + "documentation comment required") + + if meta == 'enum': + check_keys(expr, info, meta, + ['enum', 'data'], ['if', 'prefix']) + normalize_enum(expr) + check_enum(expr, info) + elif meta == 'union': + check_keys(expr, info, meta, + ['union', 'data'], + ['base', 'discriminator', 'if']) + normalize_members(expr.get('base')) + normalize_members(expr['data']) + check_union(expr, info) + elif meta == 'alternate': + check_keys(expr, info, meta, + ['alternate', 'data'], ['if']) + normalize_members(expr['data']) + check_alternate(expr, info) + elif meta == 'struct': + check_keys(expr, info, meta, + ['struct', 'data'], ['base', 'if', 'features']) + normalize_members(expr['data']) + normalize_features(expr.get('features')) + check_struct(expr, info) + elif meta == 'command': + check_keys(expr, info, meta, + ['command'], + ['data', 'returns', 'boxed', 'if', + 'gen', 'success-response', 'allow-oob', + 'allow-preconfig']) + normalize_members(expr.get('data')) + check_command(expr, info) + elif meta == 'event': + check_keys(expr, info, meta, + ['event'], ['data', 'boxed', 'if']) + normalize_members(expr.get('data')) + check_event(expr, info) + else: + assert False, 'unexpected meta type' + + normalize_if(expr) + check_if(expr, info, meta) + check_flags(expr, info) + + return exprs -- cgit 1.4.1 From 23394b4c393c832aa3891533587ff97e04c70883 Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Fri, 18 Oct 2019 10:14:51 +0200 Subject: qapi: Add feature flags to commands 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 Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster Message-Id: <20191018081454.21369-3-armbru@redhat.com> --- docs/devel/qapi-code-gen.txt | 10 ++++++---- qapi/introspect.json | 6 +++++- scripts/qapi/commands.py | 3 ++- scripts/qapi/doc.py | 4 +++- scripts/qapi/expr.py | 35 ++++++++++++++++++++--------------- scripts/qapi/introspect.py | 7 ++++++- scripts/qapi/schema.py | 22 ++++++++++++++++++---- tests/qapi-schema/test-qapi.py | 3 ++- 8 files changed, 62 insertions(+), 28 deletions(-) (limited to 'scripts/qapi/expr.py') diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt index 64d9e4c6a9..45c93a43cc 100644 --- a/docs/devel/qapi-code-gen.txt +++ b/docs/devel/qapi-code-gen.txt @@ -457,7 +457,8 @@ Syntax: '*gen': false, '*allow-oob': true, '*allow-preconfig': true, - '*if': COND } + '*if': COND, + '*features': FEATURES } Member 'command' names the command. @@ -640,9 +641,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..6d5726cf6e 100644 --- a/scripts/qapi/doc.py +++ b/scripts/qapi/doc.py @@ -249,12 +249,14 @@ 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) body += ('\n@b{Arguments:} the members of @code{%s}\n' % arg_type.name) + body += texi_features(doc) body += texi_sections(doc, ifcond) else: body = texi_entity(doc, 'Arguments', ifcond) diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index 67cb2c2b6c..7c7394f835 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -185,6 +185,22 @@ def normalize_features(features): for f in features] +def check_features(features, info): + if features is None: + return + 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 normalize_enum(expr): if isinstance(expr['data'], list): expr['data'] = [m if isinstance(m, dict) else {'name': m} @@ -217,23 +233,10 @@ def check_enum(expr, info): def check_struct(expr, info): name = expr['struct'] members = expr['data'] - features = expr.get('features') check_type(members, info, "'data'", allow_dict=name) check_type(expr.get('base'), info, "'base'") - - 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) + check_features(expr.get('features'), info) def check_union(expr, info): @@ -283,6 +286,7 @@ def check_command(expr, info): raise QAPISemError(info, "'boxed': true requires 'data'") check_type(args, info, "'data'", allow_dict=not boxed) check_type(rets, info, "'returns'", allow_array=True) + check_features(expr.get('features'), info) def check_event(expr, info): @@ -358,10 +362,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 4f257591de..b3a463dd8b 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -211,13 +211,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 2913a0fef0..f7d68a35f4 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -110,7 +110,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): @@ -659,10 +660,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 @@ -672,6 +677,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) @@ -701,13 +707,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): @@ -984,6 +996,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)) @@ -992,7 +1005,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 29d9435bf7..d31ac4bbb7 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)) -- cgit 1.4.1