Turn on the ability to pass command and event arguments in a single boxed parameter. For structs, it makes it possible to pass a single qapi type instead of a breakout of all struct members; for unions, it is now possible to use a union as the data for a command or event.
Generated code is unchanged, as long as no client uses the new feature. Signed-off-by: Eric Blake <ebl...@redhat.com> --- v6: retitle, rebase, and merge v5 40/46 and 41/46 into one patch --- docs/qapi-code-gen.txt | 24 ++++++++++++++++++++++-- scripts/qapi-commands.py | 18 +++++++++++++++--- scripts/qapi-event.py | 11 +++++++++-- scripts/qapi.py | 27 ++++++++++++++++++++------- tests/Makefile | 2 ++ tests/qapi-schema/args-bad-box.err | 1 + tests/qapi-schema/args-bad-box.exit | 1 + tests/qapi-schema/args-bad-box.json | 2 ++ tests/qapi-schema/args-bad-box.out | 0 tests/qapi-schema/args-box-anon.err | 1 + tests/qapi-schema/args-box-anon.exit | 1 + tests/qapi-schema/args-box-anon.json | 2 ++ tests/qapi-schema/args-box-anon.out | 0 tests/qapi-schema/args-union.err | 2 +- tests/qapi-schema/args-union.json | 3 +-- tests/qapi-schema/qapi-schema-test.json | 6 +++++- tests/qapi-schema/qapi-schema-test.out | 10 +++++++++- tests/test-qmp-commands.c | 12 ++++++++++++ 18 files changed, 104 insertions(+), 19 deletions(-) create mode 100644 tests/qapi-schema/args-bad-box.err create mode 100644 tests/qapi-schema/args-bad-box.exit create mode 100644 tests/qapi-schema/args-bad-box.json create mode 100644 tests/qapi-schema/args-bad-box.out create mode 100644 tests/qapi-schema/args-box-anon.err create mode 100644 tests/qapi-schema/args-box-anon.exit create mode 100644 tests/qapi-schema/args-box-anon.json create mode 100644 tests/qapi-schema/args-box-anon.out diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index 999f3b9..bd437c8 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -409,7 +409,7 @@ following example objects: === Commands === Usage: { 'command': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT, - '*returns': TYPE-NAME, + '*returns': TYPE-NAME, '*box': true, '*gen': false, '*success-response': false } Commands are defined by using a dictionary containing several members, @@ -460,6 +460,17 @@ which would validate this Client JSON Protocol transaction: => { "execute": "my-second-command" } <= { "return": [ { "value": "one" }, { } ] } +By default, the generator creates a marshalling function that converts +an input QDict into a function call implemented by the user, and +declares a prototype for the user's function which has a parameter for +each member of the argument struct, including boolean arguments that +describe whether optional arguments were provided. But if the QAPI +description includes the key 'box' with the boolean value true, the +user call prototype will have only a single parameter for the overall +generated C structure. The 'box' key is required in order to use a +union as an input argument, since it is not possible to list all +members of the union as separate parameters. + In rare cases, QAPI cannot express a type-safe representation of a corresponding Client JSON Protocol command. You then have to suppress generation of a marshalling function by including a key 'gen' with @@ -483,7 +494,8 @@ use of this field. === Events === -Usage: { 'event': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT } +Usage: { 'event': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT, + '*box': true } Events are defined with the keyword 'event'. It is not allowed to name an event 'MAX', since the generator also produces a C enumeration @@ -504,6 +516,14 @@ Resulting in this JSON object: "data": { "b": "test string" }, "timestamp": { "seconds": 1267020223, "microseconds": 435656 } } +By default, the generator creates a C function that takes as +parameters each member of the argument struct and turns it into the +appropriate JSON Client event. But if the QAPI description includes +the key 'box' with the boolean value true, the event function will +have only a single parameter for the overall generated C structure. +The 'box' key is required in order to use a union as the data key, +since it is not possible to list all members of the union as separate +parameters. == Client JSON Protocol introspection == diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index efdeb70..d47ee10 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -31,7 +31,7 @@ def gen_call(name, arg_type, box, ret_type): argstr = '' assert arg_type if box and not arg_type.is_empty(): - assert False # not implemented + argstr = 'arg, ' else: for memb in arg_type.members: if memb.optional: @@ -77,7 +77,10 @@ def gen_marshal_vars(arg_type, box, ret_type): ''') if box: - assert False # not implemented + ret += mcgen(''' + %(c_type)s arg = %(c_null)s; +''', + c_type=arg_type.c_type(), c_null=arg_type.c_null()) else: for memb in arg_type.members: if memb.optional: @@ -119,7 +122,16 @@ def gen_marshal_input_visit(arg_type, box, dealloc=False): ''') if box: - assert False # not implemented + if dealloc: + errp = 'NULL' + else: + errp = '&err' + ret += mcgen(''' + visit_type_%(c_name)s(v, NULL, &arg, %(errp)s); + +''', + c_name=arg_type.c_name(), errp=errp) + ret += gen_err_check(skiperr=dealloc) else: ret += gen_visit_fields(arg_type.members, skiperr=dealloc) diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index 451d077..bae200d 100644 --- a/scripts/qapi-event.py +++ b/scripts/qapi-event.py @@ -41,8 +41,11 @@ def gen_event_send(name, arg_type, box): assert arg_type if not arg_type.is_empty(): - ret += mcgen(''' + if not box: + ret += mcgen(''' QObject *obj; +''') + ret += mcgen(''' QmpOutputVisitor *qov; Visitor *v; @@ -67,7 +70,11 @@ def gen_event_send(name, arg_type, box): ''') if box: - assert False # not implemented + ret += mcgen(''' + visit_type_%(c_name)s(v, NULL, &arg, &err); +''', + c_name=arg_type.c_name(), name=arg_type.name) + ret += gen_err_check() else: ret += mcgen(''' visit_start_struct(v, "%(name)s", NULL, 0, &err); diff --git a/scripts/qapi.py b/scripts/qapi.py index b408a82..84c0bd0 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -521,10 +521,14 @@ def check_type(expr_info, source, value, allow_array=False, def check_command(expr, expr_info): name = expr['command'] + box = expr.get('box', False) + args_meta = ['struct'] + if box: + args_meta += ['union'] check_type(expr_info, "'data' for command '%s'" % name, - expr.get('data'), allow_dict=True, allow_optional=True, - allow_metas=['struct']) + expr.get('data'), allow_dict=not box, allow_optional=True, + allow_metas=args_meta) returns_meta = ['union', 'struct'] if name in returns_whitelist: returns_meta += ['built-in', 'alternate', 'enum'] @@ -536,11 +540,15 @@ def check_command(expr, expr_info): def check_event(expr, expr_info): global events name = expr['event'] + box = expr.get('box', False) + meta = ['struct'] + if box: + meta += ['union'] events.append(name) check_type(expr_info, "'data' for event '%s'" % name, - expr.get('data'), allow_dict=True, allow_optional=True, - allow_metas=['struct']) + expr.get('data'), allow_dict=not box, allow_optional=True, + allow_metas=meta) def check_union(expr, expr_info): @@ -681,6 +689,10 @@ def check_keys(expr_elem, meta, required, optional=[]): raise QAPIExprError(info, "'%s' of %s '%s' should only use false value" % (key, meta, name)) + if key == 'box' and value is not True: + raise QAPIExprError(info, + "'%s' of %s '%s' should only use true value" + % (key, meta, name)) for key in required: if key not in expr: raise QAPIExprError(info, @@ -712,10 +724,10 @@ def check_exprs(exprs): add_struct(expr, info) elif 'command' in expr: check_keys(expr_elem, 'command', [], - ['data', 'returns', 'gen', 'success-response']) + ['data', 'returns', 'gen', 'success-response', 'box']) add_name(expr['command'], info, 'command') elif 'event' in expr: - check_keys(expr_elem, 'event', [], ['data']) + check_keys(expr_elem, 'event', [], ['data', 'box']) add_name(expr['event'], info, 'event') else: raise QAPIExprError(expr_elem['info'], @@ -1631,7 +1643,8 @@ def gen_params(arg_type, box, extra): ret = '' sep = '' if box and not arg_type.is_empty(): - assert False # not implemented + ret += '%s arg' % arg_type.c_type(is_param=True) + sep = ', ' else: for memb in arg_type.members: ret += sep diff --git a/tests/Makefile b/tests/Makefile index 84caf98..3e5d17d 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -253,6 +253,8 @@ qapi-schema += args-alternate.json qapi-schema += args-any.json qapi-schema += args-array-empty.json qapi-schema += args-array-unknown.json +qapi-schema += args-bad-box.json +qapi-schema += args-box-anon.json qapi-schema += args-int.json qapi-schema += args-invalid.json qapi-schema += args-member-array-bad.json diff --git a/tests/qapi-schema/args-bad-box.err b/tests/qapi-schema/args-bad-box.err new file mode 100644 index 0000000..16afe3c --- /dev/null +++ b/tests/qapi-schema/args-bad-box.err @@ -0,0 +1 @@ +tests/qapi-schema/args-bad-box.json:2: 'box' of command 'foo' should only use true value diff --git a/tests/qapi-schema/args-bad-box.exit b/tests/qapi-schema/args-bad-box.exit new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/tests/qapi-schema/args-bad-box.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/args-bad-box.json b/tests/qapi-schema/args-bad-box.json new file mode 100644 index 0000000..8d5737a --- /dev/null +++ b/tests/qapi-schema/args-bad-box.json @@ -0,0 +1,2 @@ +# 'box' should only appear with value true +{ 'command': 'foo', 'box': false } diff --git a/tests/qapi-schema/args-bad-box.out b/tests/qapi-schema/args-bad-box.out new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/args-box-anon.err b/tests/qapi-schema/args-box-anon.err new file mode 100644 index 0000000..11eaefc --- /dev/null +++ b/tests/qapi-schema/args-box-anon.err @@ -0,0 +1 @@ +tests/qapi-schema/args-box-anon.json:2: 'data' for command 'foo' should be a type name diff --git a/tests/qapi-schema/args-box-anon.exit b/tests/qapi-schema/args-box-anon.exit new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/tests/qapi-schema/args-box-anon.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/args-box-anon.json b/tests/qapi-schema/args-box-anon.json new file mode 100644 index 0000000..947e3c6 --- /dev/null +++ b/tests/qapi-schema/args-box-anon.json @@ -0,0 +1,2 @@ +# 'box' can only be used with named types +{ 'command': 'foo', 'box': true, 'data': { 'string': 'str' } } diff --git a/tests/qapi-schema/args-box-anon.out b/tests/qapi-schema/args-box-anon.out new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/args-union.err b/tests/qapi-schema/args-union.err index 1d693d7..f8ad223 100644 --- a/tests/qapi-schema/args-union.err +++ b/tests/qapi-schema/args-union.err @@ -1 +1 @@ -tests/qapi-schema/args-union.json:4: 'data' for command 'oops' cannot use union type 'Uni' +tests/qapi-schema/args-union.json:3: 'data' for command 'oops' cannot use union type 'Uni' diff --git a/tests/qapi-schema/args-union.json b/tests/qapi-schema/args-union.json index 7bdcbb7..c0ce091 100644 --- a/tests/qapi-schema/args-union.json +++ b/tests/qapi-schema/args-union.json @@ -1,4 +1,3 @@ -# we do not allow union arguments -# TODO should we support this? +# use of union arguments requires 'box':true { 'union': 'Uni', 'data': { 'case1': 'int', 'case2': 'str' } } { 'command': 'oops', 'data': 'Uni' } diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index 44c1965..d0bc3d2 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -126,6 +126,9 @@ { 'command': 'guest-get-time', 'data': {'a': 'int', '*b': 'int' }, 'returns': 'int' } { 'command': 'guest-sync', 'data': { 'arg': 'any' }, 'returns': 'any' } +{ 'command': 'boxed-empty', 'box': true } +{ 'command': 'boxed-struct', 'box': true, 'data': 'UserDefZero' } +{ 'command': 'boxed-union', 'data': 'UserDefNativeListUnion', 'box': true } # For testing integer range flattening in opts-visitor. The following schema # corresponds to the option format: @@ -146,13 +149,14 @@ { 'struct': 'EventStructOne', 'data': { 'struct1': 'UserDefOne', 'string': 'str', '*enum2': 'EnumOne' } } -{ 'event': 'EVENT_A' } +{ 'event': 'EVENT_A', 'box': true } { 'event': 'EVENT_B', 'data': { } } { 'event': 'EVENT_C', 'data': { '*a': 'int', '*b': 'UserDefOne', 'c': 'str' } } { 'event': 'EVENT_D', 'data': { 'a' : 'EventStructOne', 'b' : 'str', '*c': 'str', '*enum3': 'EnumOne' } } +{ 'event': 'EVENT_E', 'box': true, 'data': 'UserDefZero' } # test that we correctly compile downstream extensions, as well as munge # ticklish names diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 8731caa..3dc133e 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -100,13 +100,15 @@ alternate AltStrNum case s: str case n: number event EVENT_A :empty - box=False + box=True event EVENT_B :empty box=False event EVENT_C :obj-EVENT_C-arg box=False event EVENT_D :obj-EVENT_D-arg box=False +event EVENT_E UserDefZero + box=True object Empty1 base :empty object Empty2 @@ -246,6 +248,12 @@ object __org.qemu_x-Union2 case __org.qemu_x-value: __org.qemu_x-Struct2 command __org.qemu_x-command :obj-__org.qemu_x-command-arg -> __org.qemu_x-Union1 gen=True success_response=True box=False +command boxed-empty :empty -> None + gen=True success_response=True box=True +command boxed-struct UserDefZero -> None + gen=True success_response=True box=True +command boxed-union UserDefNativeListUnion -> None + gen=True success_response=True box=True command guest-get-time :obj-guest-get-time-arg -> int gen=True success_response=True box=False command guest-sync :obj-guest-sync-arg -> any diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c index d3466a4..a58a425 100644 --- a/tests/test-qmp-commands.c +++ b/tests/test-qmp-commands.c @@ -59,6 +59,18 @@ QObject *qmp_guest_sync(QObject *arg, Error **errp) return arg; } +void qmp_boxed_empty(Error **errp) +{ +} + +void qmp_boxed_struct(UserDefZero *arg, Error **errp) +{ +} + +void qmp_boxed_union(UserDefNativeListUnion *arg, Error **errp) +{ +} + __org_qemu_x_Union1 *qmp___org_qemu_x_command(__org_qemu_x_EnumList *a, __org_qemu_x_StructList *b, __org_qemu_x_Union2 *c, -- 2.4.3