Turn on the ability to pass command and event arguments in a single boxed parameter. This patch merely tests the use of the feature on structs. With this patch, we still reject union types, and crash on { 'command':'foo', 'data': { anonymous...}, 'box':true }; that will be addressed in the next patch.
While this does not alter any QMP commands, it opens the door for clients to turn on boxing to receive a single pointer parameter of the overall struct instead of a series of parameters for each member of the struct; which will be useful for commands whose argument struct has lots of members. The generated code is slightly reorganized for convenience. Signed-off-by: Eric Blake <ebl...@redhat.com> --- docs/qapi-code-gen.txt | 24 ++++++++++++++++++++++-- scripts/qapi-commands.py | 16 +++++++++++++--- scripts/qapi-event.py | 13 +++++++++++-- scripts/qapi.py | 11 ++++++++--- tests/qapi-schema/qapi-schema-test.json | 2 ++ tests/qapi-schema/qapi-schema-test.out | 4 ++++ tests/test-qmp-commands.c | 4 ++++ 7 files changed, 64 insertions(+), 10 deletions(-) diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index f0fb881..49e3586 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -404,7 +404,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, @@ -455,6 +455,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 @@ -478,7 +489,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 @@ -499,6 +511,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 d003e3c..0256c27 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -30,7 +30,7 @@ def gen_call(name, arg_type, box, ret_type): argstr = '' if box: - assert False # not implemented + argstr = 'arg, ' else: if arg_type: for memb in arg_type.members: @@ -80,7 +80,10 @@ Visitor *v; ''') 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: @@ -115,6 +118,7 @@ def gen_marshal_input_visit(arg_type, box, dealloc=False): if dealloc: errarg = None + errparg = 'NULL' ret += mcgen(''' qmp_input_visitor_cleanup(mi); md = qapi_dealloc_visitor_new(); @@ -122,12 +126,18 @@ v = qapi_dealloc_get_visitor(md); ''') else: errarg = 'err' + errparg = '&err' ret += mcgen(''' v = qmp_input_get_visitor(mi); ''') if box: - assert False # not implemented + ret += mcgen(''' +visit_type_%(c_name)s(v, &arg, NULL, %(errp)s); + +''', + c_name=arg_type.c_name(), errp=errparg) + ret += gen_err_check(errarg) else: ret += gen_visit_fields(arg_type.members, '', False, errarg) diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index f732001..b46989c 100644 --- a/scripts/qapi-event.py +++ b/scripts/qapi-event.py @@ -40,10 +40,13 @@ def gen_event_send(name, arg_type, box): proto=gen_event_send_proto(name, arg_type, box)) if arg_type and arg_type.members: + if not box: + ret += mcgen(''' + QObject *obj; +''') ret += mcgen(''' QmpOutputVisitor *qov; Visitor *v; - QObject *obj; ''') @@ -69,7 +72,13 @@ def gen_event_send(name, arg_type, box): ''') if box: - assert False # not implemented + ret += mcgen(''' + visit_type_%(c_name)s(v, &arg, NULL, &err); + if (err) { + goto out; + } +''', + c_name=arg_type.c_name(), name=arg_type.name) else: ret += mcgen(''' /* Fake visit, as if all members are under a structure */ diff --git a/scripts/qapi.py b/scripts/qapi.py index fb941c9..a54ff64 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -689,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, @@ -720,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'], @@ -1564,7 +1568,8 @@ def gen_params(arg_type, box, extra): ret = '' sep = '' if box: - assert False # not implemented + ret += '%s arg' % arg_type.c_type(is_param=True) + sep = ', ' else: assert not arg_type.variants for memb in arg_type.members: diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index 97830a3..f0f5ad2 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -103,6 +103,7 @@ 'returns': 'int' } # note: command name 'guest-sync' chosen to avoid "cannot use built-in" error { 'command': 'guest-sync', 'data': { 'arg': 'any' }, 'returns': 'any' } +{ 'command': 'boxed', 'box': true, 'data': 'UserDefZero' } # For testing integer range flattening in opts-visitor. The following schema # corresponds to the option format: @@ -130,6 +131,7 @@ '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 { 'enum': '__org.qemu_x-Enum', 'data': [ '__org.qemu_x-value' ] } diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 6f39a92..fb58d54 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -79,6 +79,8 @@ event EVENT_C :obj-EVENT_C-arg box=False event EVENT_D :obj-EVENT_D-arg box=False +event EVENT_E UserDefZero + box=True enum EnumOne ['value1', 'value2', 'value3'] object EventStructOne member struct1: UserDefOne optional=False @@ -185,6 +187,8 @@ 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 UserDefZero -> None + gen=True success_response=True box=True command guest-sync :obj-guest-sync-arg -> any gen=True success_response=True box=False command user_def_cmd None -> None diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c index 89a3b47..a190f31 100644 --- a/tests/test-qmp-commands.c +++ b/tests/test-qmp-commands.c @@ -54,6 +54,10 @@ QObject *qmp_guest_sync(QObject *arg, Error **errp) return arg; } +void qmp_boxed(UserDefZero *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