From patchwork Thu Sep 10 04:06:26 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 516116 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 92B05140271 for ; Thu, 10 Sep 2015 14:25:31 +1000 (AEST) Received: from localhost ([::1]:46902 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZZtQP-00077K-Ci for incoming@patchwork.ozlabs.org; Thu, 10 Sep 2015 00:25:29 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34244) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZZt8V-0003Fr-9u for qemu-devel@nongnu.org; Thu, 10 Sep 2015 00:07:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZZt8T-00055k-M2 for qemu-devel@nongnu.org; Thu, 10 Sep 2015 00:06:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43527) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZZt8T-00055Y-DF for qemu-devel@nongnu.org; Thu, 10 Sep 2015 00:06:57 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 09CB496C7; Thu, 10 Sep 2015 04:06:57 +0000 (UTC) Received: from red.redhat.com ([10.3.113.15]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t8A46Zhl031165; Thu, 10 Sep 2015 00:06:56 -0400 From: Eric Blake To: qemu-devel@nongnu.org Date: Wed, 9 Sep 2015 22:06:26 -0600 Message-Id: <1441857991-7309-25-git-send-email-eblake@redhat.com> In-Reply-To: <1441857991-7309-1-git-send-email-eblake@redhat.com> References: <1441857991-7309-1-git-send-email-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Michael Roth , marcandre.lureau@redhat.com, armbru@redhat.com, DirtY.iCE.hu@gmail.com Subject: [Qemu-devel] [PATCH RFC v4 24/29] qapi: Implement boxed structs for commands/events X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 --- docs/qapi-code-gen.txt | 24 ++++++++++++++++++++++-- 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, 62 insertions(+), 10 deletions(-) diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index ff57010..a12a7f6 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -396,7 +396,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, @@ -447,6 +447,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 @@ -470,7 +481,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 @@ -491,6 +503,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 ed4b38f..2a98cca 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -41,7 +41,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: @@ -91,7 +91,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: @@ -140,7 +143,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 c333d05..89d2e8d 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, &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 b407779..92e9e74 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -679,6 +679,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, @@ -709,10 +713,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'], @@ -1485,7 +1489,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 a6321ec..92fc178 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -94,6 +94,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: @@ -121,6 +122,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 6f775e5..734cd76 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 @@ -178,6 +180,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,