[Qemu-devel] [PATCH v5 40/46] qapi: Implement boxed structs for commands/events

[Eric Blake <eblake@redhat.com>]

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 <eblake@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