From d708cdbe8792a55f53e90c1c787e871d527e8d4b Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 4 May 2015 09:05:19 -0600 Subject: qapi: Unify type bypass and add tests For a few QMP commands, we are forced to pass an arbitrary type without tracking it properly in QAPI. Among the existing clients, this unnamed type was spelled 'dict', 'visitor', and '**'; this patch standardizes on '**', matching the documentation changes earlier in the series. Meanwhile, for the 'gen' key, we have been ignoring the value, although the schema consistently used "'no'" ('success-response' was hard-coded to checking for 'no'). But now that we can support a literal "false" in the schema, we might as well use that rather than ignoring the value or special-casing a random string. Note that these are one-way switches (use of 'gen':true is not the same as omitting 'gen'). Also, the use of '**' requires 'gen':false, but the use of 'gen':false does not mandate the use of '**'. There is no difference to the generated code. Add some tests on what we'd like to guarantee, although it will take later patches to clean up test results and actually enforce the use of a bool parameter. Signed-off-by: Eric Blake Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- scripts/qapi-commands.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'scripts/qapi-commands.py') diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 053ba85b5f..cb786825ee 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -2,7 +2,7 @@ # QAPI command marshaller generator # # Copyright IBM, Corp. 2011 -# Copyright (C) 2014 Red Hat, Inc. +# Copyright (C) 2014-2015 Red Hat, Inc. # # Authors: # Anthony Liguori @@ -293,17 +293,12 @@ out: return ret -def option_value_matches(opt, val, cmd): - if opt in cmd and cmd[opt] == val: - return True - return False - def gen_registry(commands): registry="" push_indent() for cmd in commands: options = 'QCO_NO_OPTIONS' - if option_value_matches('success-response', 'no', cmd): + if not cmd.get('success-response', True): options = 'QCO_NO_SUCCESS_RESP' registry += mcgen(''' -- cgit 1.4.1 From 6b5abc7df7ef9aadb3ff0eba6ccf4f1f0181e2e1 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 4 May 2015 09:05:33 -0600 Subject: qapi: Drop support for inline nested types A future patch will be using a 'name':{dictionary} entry in the QAPI schema to specify a default value for an optional argument (see previous commit messages for more details why); but existing use of inline nested structs conflicts with that goal. Now that all commands have been changed to avoid inline nested structs, nuke support for them, and turn it into a hard error. Update the testsuite to reflect tighter parsing rules. Signed-off-by: Eric Blake Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- scripts/qapi-commands.py | 8 +++--- scripts/qapi-event.py | 4 +-- scripts/qapi-types.py | 9 ++----- scripts/qapi-visit.py | 37 ++++------------------------ scripts/qapi.py | 20 ++++++--------- tests/qapi-schema/event-nest-struct.err | 2 +- tests/qapi-schema/nested-struct-data.err | 1 + tests/qapi-schema/nested-struct-data.exit | 2 +- tests/qapi-schema/nested-struct-data.json | 2 +- tests/qapi-schema/nested-struct-data.out | 3 --- tests/qapi-schema/nested-struct-returns.err | 1 + tests/qapi-schema/nested-struct-returns.exit | 2 +- tests/qapi-schema/nested-struct-returns.json | 2 +- tests/qapi-schema/nested-struct-returns.out | 3 --- 14 files changed, 27 insertions(+), 69 deletions(-) (limited to 'scripts/qapi-commands.py') diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index cb786825ee..93e43f0e48 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -28,7 +28,7 @@ def type_visitor(name): def generate_command_decl(name, args, ret_type): arglist="" - for argname, argtype, optional, structured in parse_args(args): + for argname, argtype, optional in parse_args(args): argtype = c_type(argtype, is_param=True) if optional: arglist += "bool has_%s, " % c_var(argname) @@ -53,7 +53,7 @@ def gen_sync_call(name, args, ret_type, indent=0): retval="" if ret_type: retval = "retval = " - for argname, argtype, optional, structured in parse_args(args): + for argname, argtype, optional in parse_args(args): if optional: arglist += "has_%s, " % c_var(argname) arglist += "%s, " % (c_var(argname)) @@ -96,7 +96,7 @@ Visitor *v; def gen_visitor_input_vars_decl(args): ret = "" push_indent() - for argname, argtype, optional, structured in parse_args(args): + for argname, argtype, optional in parse_args(args): if optional: ret += mcgen(''' bool has_%(argname)s = false; @@ -139,7 +139,7 @@ v = qapi_dealloc_get_visitor(md); v = qmp_input_get_visitor(mi); ''') - for argname, argtype, optional, structured in parse_args(args): + for argname, argtype, optional in parse_args(args): if optional: ret += mcgen(''' visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s); diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index 601e3076ab..47dc041805 100644 --- a/scripts/qapi-event.py +++ b/scripts/qapi-event.py @@ -21,7 +21,7 @@ def _generate_event_api_name(event_name, params): l = len(api_name) if params: - for argname, argentry, optional, structured in parse_args(params): + for argname, argentry, optional in parse_args(params): if optional: api_name += "bool has_%s,\n" % c_var(argname) api_name += "".ljust(l) @@ -93,7 +93,7 @@ def generate_event_implement(api_name, event_name, params): """, event_name = event_name) - for argname, argentry, optional, structured in parse_args(params): + for argname, argentry, optional in parse_args(params): if optional: ret += mcgen(""" if (has_%(var)s) { diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index a429d9ec05..2bf8145076 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -63,18 +63,13 @@ typedef struct %(name)sList def generate_struct_fields(members): ret = '' - for argname, argentry, optional, structured in parse_args(members): + for argname, argentry, optional in parse_args(members): if optional: ret += mcgen(''' bool has_%(c_name)s; ''', c_name=c_var(argname)) - if structured: - push_indent() - ret += generate_struct({ "field": argname, "data": argentry}) - pop_indent() - else: - ret += mcgen(''' + ret += mcgen(''' %(c_type)s %(c_name)s; ''', c_type=c_type(argentry), c_name=c_var(argname)) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index c739a95a87..6156162500 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -51,27 +51,6 @@ def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base = else: full_name = "%s_%s" % (name, fn_prefix) - for argname, argentry, optional, structured in parse_args(members): - if structured: - if not fn_prefix: - nested_fn_prefix = argname - else: - nested_fn_prefix = "%s_%s" % (fn_prefix, argname) - - nested_field_prefix = "%s%s." % (field_prefix, argname) - ret += generate_visit_struct_fields(name, nested_field_prefix, - nested_fn_prefix, argentry) - ret += mcgen(''' - -static void visit_type_%(full_name)s_field_%(c_name)s(Visitor *m, %(name)s **obj, Error **errp) -{ -''', - name=name, full_name=full_name, c_name=c_var(argname)) - ret += generate_visit_struct_body(full_name, argname, argentry) - ret += mcgen(''' -} -''') - if base: ret += generate_visit_implicit_struct(base) @@ -94,7 +73,7 @@ if (err) { c_prefix=c_var(field_prefix), type=type_name(base), c_name=c_var('base')) - for argname, argentry, optional, structured in parse_args(members): + for argname, argentry, optional in parse_args(members): if optional: ret += mcgen(''' visit_optional(m, &(*obj)->%(c_prefix)shas_%(c_name)s, "%(name)s", &err); @@ -104,18 +83,12 @@ if (!err && (*obj)->%(prefix)shas_%(c_name)s) { c_name=c_var(argname), name=argname) push_indent() - if structured: - ret += mcgen(''' -visit_type_%(full_name)s_field_%(c_name)s(m, obj, &err); -''', - full_name=full_name, c_name=c_var(argname)) - else: - ret += mcgen(''' + ret += mcgen(''' visit_type_%(type)s(m, &(*obj)->%(c_prefix)s%(c_name)s, "%(name)s", &err); ''', - c_prefix=c_var(field_prefix), prefix=field_prefix, - type=type_name(argentry), c_name=c_var(argname), - name=argname) + c_prefix=c_var(field_prefix), prefix=field_prefix, + type=type_name(argentry), c_name=c_var(argname), + name=argname) if optional: pop_indent() diff --git a/scripts/qapi.py b/scripts/qapi.py index 333f59ab5b..44898b082a 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -373,10 +373,12 @@ def check_type(expr_info, source, value, allow_array = False, for (key, arg) in value.items(): check_name(expr_info, "Member of %s" % source, key, allow_optional=allow_optional) + # Todo: allow dictionaries to represent default values of + # an optional argument. check_type(expr_info, "Member '%s' of %s" % (key, source), arg, - allow_array=True, allow_dict=True, allow_optional=True, + allow_array=True, allow_star=allow_star, allow_metas=['built-in', 'union', 'alternate', 'struct', - 'enum'], allow_star=allow_star) + 'enum']) def check_command(expr, expr_info): name = expr['command'] @@ -404,13 +406,6 @@ def check_event(expr, expr_info): check_type(expr_info, "'data' for event '%s'" % name, expr.get('data'), allow_dict=True, allow_optional=True, allow_metas=['union', 'struct']) - if params: - for argname, argentry, optional, structured in parse_args(params): - if structured: - raise QAPIExprError(expr_info, - "Nested structure define in event is not " - "supported, event '%s', argname '%s'" - % (expr['event'], argname)) def check_union(expr, expr_info): name = expr['union'] @@ -671,13 +666,12 @@ def parse_args(typeinfo): argname = member argentry = typeinfo[member] optional = False - structured = False if member.startswith('*'): argname = member[1:] optional = True - if isinstance(argentry, OrderedDict): - structured = True - yield (argname, argentry, optional, structured) + # Todo: allow argentry to be OrderedDict, for providing the + # value of an optional argument. + yield (argname, argentry, optional) def de_camel_case(name): new_name = '' diff --git a/tests/qapi-schema/event-nest-struct.err b/tests/qapi-schema/event-nest-struct.err index 91bde1c967..5a42701b8f 100644 --- a/tests/qapi-schema/event-nest-struct.err +++ b/tests/qapi-schema/event-nest-struct.err @@ -1 +1 @@ -tests/qapi-schema/event-nest-struct.json:1: Nested structure define in event is not supported, event 'EVENT_A', argname 'a' +tests/qapi-schema/event-nest-struct.json:1: Member 'a' of 'data' for event 'EVENT_A' should be a type name diff --git a/tests/qapi-schema/nested-struct-data.err b/tests/qapi-schema/nested-struct-data.err index e69de29bb2..da767bade2 100644 --- a/tests/qapi-schema/nested-struct-data.err +++ b/tests/qapi-schema/nested-struct-data.err @@ -0,0 +1 @@ +tests/qapi-schema/nested-struct-data.json:2: Member 'a' of 'data' for command 'foo' should be a type name diff --git a/tests/qapi-schema/nested-struct-data.exit b/tests/qapi-schema/nested-struct-data.exit index 573541ac97..d00491fd7e 100644 --- a/tests/qapi-schema/nested-struct-data.exit +++ b/tests/qapi-schema/nested-struct-data.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/nested-struct-data.json b/tests/qapi-schema/nested-struct-data.json index 0247c8c82c..3d52d2b398 100644 --- a/tests/qapi-schema/nested-struct-data.json +++ b/tests/qapi-schema/nested-struct-data.json @@ -1,4 +1,4 @@ -# FIXME: inline subtypes collide with our desired future use of defaults +# inline subtypes collide with our desired future use of defaults { 'command': 'foo', 'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' }, 'returns': {} } diff --git a/tests/qapi-schema/nested-struct-data.out b/tests/qapi-schema/nested-struct-data.out index 999cbb8eef..e69de29bb2 100644 --- a/tests/qapi-schema/nested-struct-data.out +++ b/tests/qapi-schema/nested-struct-data.out @@ -1,3 +0,0 @@ -[OrderedDict([('command', 'foo'), ('data', OrderedDict([('a', OrderedDict([('string', 'str'), ('integer', 'int')])), ('b', 'str')])), ('returns', OrderedDict())])] -[] -[] diff --git a/tests/qapi-schema/nested-struct-returns.err b/tests/qapi-schema/nested-struct-returns.err index e69de29bb2..5238d075b7 100644 --- a/tests/qapi-schema/nested-struct-returns.err +++ b/tests/qapi-schema/nested-struct-returns.err @@ -0,0 +1 @@ +tests/qapi-schema/nested-struct-returns.json:2: Member 'a' of 'returns' for command 'foo' should be a type name diff --git a/tests/qapi-schema/nested-struct-returns.exit b/tests/qapi-schema/nested-struct-returns.exit index 573541ac97..d00491fd7e 100644 --- a/tests/qapi-schema/nested-struct-returns.exit +++ b/tests/qapi-schema/nested-struct-returns.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/nested-struct-returns.json b/tests/qapi-schema/nested-struct-returns.json index 5a46840b71..d2cd047f0d 100644 --- a/tests/qapi-schema/nested-struct-returns.json +++ b/tests/qapi-schema/nested-struct-returns.json @@ -1,3 +1,3 @@ -# FIXME: inline subtypes collide with our desired future use of defaults +# inline subtypes collide with our desired future use of defaults { 'command': 'foo', 'returns': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } } diff --git a/tests/qapi-schema/nested-struct-returns.out b/tests/qapi-schema/nested-struct-returns.out index c53d23b3a8..e69de29bb2 100644 --- a/tests/qapi-schema/nested-struct-returns.out +++ b/tests/qapi-schema/nested-struct-returns.out @@ -1,3 +0,0 @@ -[OrderedDict([('command', 'foo'), ('returns', OrderedDict([('a', OrderedDict([('string', 'str'), ('integer', 'int')])), ('b', 'str')]))])] -[] -[] -- cgit 1.4.1