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 clients, 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. Signed-off-by: Eric Blake <ebl...@redhat.com> --- docs/qapi-code-gen.txt | 31 +++++++++++++++++++++++++------ scripts/qapi-commands.py | 14 +++++++++++--- 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, 65 insertions(+), 14 deletions(-) diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index 688a60c..668d2fc 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -397,7 +397,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, @@ -408,10 +408,9 @@ Client JSON Protocol command exchange. The 'data' argument maps to the "arguments" dictionary passed in as part of a Client JSON Protocol command. The 'data' member is optional and defaults to {} (an empty dictionary). If present, it must be the -string name of a complex type, a one-element array containing the name -of a complex type, or a dictionary that declares an anonymous type -with the same semantics as a 'struct' expression, with one exception -noted below when 'gen' is used. +string name of a complex type, or a dictionary that declares an +anonymous type with the same semantics as a 'struct' expression, with +one exception noted below when 'gen' is used. The 'returns' member describes what will appear in the "return" field of a Client JSON Protocol reply on successful completion of a command. @@ -449,6 +448,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, where the +user's function protocol 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 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 @@ -472,7 +482,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 @@ -493,6 +504,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. == Code generation == diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index e2b8f7a..d75a399 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -38,7 +38,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: @@ -87,7 +87,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: @@ -135,7 +138,12 @@ 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: for memb in arg_type.members: if memb.optional: diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index 278cd7f..17f0c12 100644 --- a/scripts/qapi-event.py +++ b/scripts/qapi-event.py @@ -37,10 +37,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; ''') @@ -66,7 +69,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, &local_err); + if (local_err) { + goto clean; + } +''', + 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 313eaef..b907878 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -686,6 +686,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 != True: + raise QAPIExprError(info, + "'%s' of %s '%s' should only use true value" + % (key, meta, name)) for key in required: if not expr.has_key(key): raise QAPIExprError(info, @@ -716,10 +720,10 @@ def check_exprs(exprs): add_struct(expr, info) elif expr.has_key('command'): check_keys(expr_elem, 'command', [], - ['data', 'returns', 'gen', 'success-response']) + ['data', 'returns', 'gen', 'success-response', 'box']) add_name(expr['command'], info, 'command') elif expr.has_key('event'): - 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'], @@ -1426,7 +1430,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 45143e3..4acb57a 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -93,6 +93,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: @@ -120,6 +121,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 1a66627..8b120ee 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -77,6 +77,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 @@ -173,6 +175,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