qapi: Implement boxed types for commands/events

Turn on the ability to pass command and event arguments in
a single boxed parameter, which must name a non-empty type
(although the type can be a struct with all optional members).
For structs, it makes it possible to pass a single qapi type
instead of a breakout of all struct members (useful if the
arguments are already in a struct or if the number of members
is large); for other complex types, it is now possible to use
a union or alternate as the data for a command or event.

The empty type may be technically feasible if needed down the
road, but it's easier to forbid it now and relax things to allow
it later, than it is to allow it now and have to special case
how the generated 'q_empty' type is handled (see commit 7ce106a9
for reasons why nothing is generated for the empty type).  An
alternate type is never considered empty, but now that a boxed
type can be either an object or an alternate, we have to provide
a trivial QAPISchemaAlternateType.is_empty().  The new call to
arg_type.is_empty() during QAPISchemaCommand.check() requires
that we first check the type in question; but there is no chance
of introducing a cycle since objects do not refer back to commands.

We still have a split in syntax checking between ad-hoc parsing
up front (merely validates that 'boxed' has a sane value) and
during .check() methods (if 'boxed' is set, then 'data' must name
a non-empty user-defined type).

Generated code is unchanged, as long as no client uses the
new feature.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1468468228-27827-10-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Test files renamed to *-boxed-*]
Signed-off-by: Markus Armbruster <armbru@redhat.com>

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 48b0b31f2e72f1d6fedacd83ab9c50f373481697..de298dcaecfffdacc39220b9dd7625934325f253 100644 (file)
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -410,7 +410,7 @@ following example objects:
 === Commands ===
 
 Usage: { 'command': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT,
-         '*returns': TYPE-NAME,
+         '*returns': TYPE-NAME, '*boxed': true,
          '*gen': false, '*success-response': false }
 
 Commands are defined by using a dictionary containing several members,
@@ -461,6 +461,20 @@ which would validate this Client JSON Protocol transaction:
  => { "execute": "my-second-command" }
  <= { "return": [ { "value": "one" }, { } ] }
 
+The generator emits a prototype for the user's function implementing
+the command.  Normally, 'data' is a dictionary for an anonymous type,
+or names a struct type (possibly empty, but not a union), and its
+members are passed as separate arguments to this function.  If the
+command definition includes a key 'boxed' with the boolean value true,
+then 'data' is instead the name of any non-empty complex type
+(struct, union, or alternate), and a pointer to that QAPI type is
+passed as a single argument.
+
+The generator also emits a marshalling function that extracts
+arguments for the user's function out of an input QDict, calls the
+user's function, and if it succeeded, builds an output QObject from
+its return value.
+
 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
@@ -484,7 +498,8 @@ use of this member.
 
 === Events ===
 
-Usage: { 'event': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT }
+Usage: { 'event': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT,
+         '*boxed': 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
@@ -505,6 +520,14 @@ Resulting in this JSON object:
   "data": { "b": "test string" },
   "timestamp": { "seconds": 1267020223, "microseconds": 435656 } }
 
+The generator emits a function to send the event.  Normally, 'data' is
+a dictionary for an anonymous type, or names a struct type (possibly
+empty, but not a union), and its members are passed as separate
+arguments to this function.  If the event definition includes a key
+'boxed' with the boolean value true, then 'data' is instead the name of
+any non-empty complex type (struct, union, or alternate), and a
+pointer to that QAPI type is passed as a single argument.
+
 
 == Client JSON Protocol introspection ==
 

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 0090fb4f21e14de7c1b397ab18e0b7a00d28a0f8..a06a2c4f9b1af843490cfcbe8228b47b8da29435 100644 (file)
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -30,7 +30,8 @@ def gen_call(name, arg_type, boxed, ret_type):
 
     argstr = ''
     if boxed:
-        assert False    # not implemented
+        assert arg_type and not arg_type.is_empty()
+        argstr = '&arg, '
     elif arg_type:
         assert not arg_type.variants
         for memb in arg_type.members:

diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 53410c26d5f91ba7aa67d500fe506b56f6d39a00..38d82111ad30abc3f5d4ae8ca884a30bc86cf780 100644 (file)
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -79,7 +79,10 @@ def gen_event_send(name, arg_type, boxed):
     QObject *obj;
     Visitor *v;
 ''')
-        ret += gen_param_var(arg_type)
+        if not boxed:
+            ret += gen_param_var(arg_type)
+    else:
+        assert not boxed
 
     ret += mcgen('''
 

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 0c6159c400ccebf6540f60c8e5a71f6e81071085..21bc32fda3d1e1d59f76d7b22c126deea236ea8c 100644 (file)
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -522,10 +522,14 @@ def check_type(expr_info, source, value, allow_array=False,
 
 def check_command(expr, expr_info):
     name = expr['command']
+    boxed = expr.get('boxed', False)
 
+    args_meta = ['struct']
+    if boxed:
+        args_meta += ['union', 'alternate']
     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 boxed, allow_optional=True,
+               allow_metas=args_meta)
     returns_meta = ['union', 'struct']
     if name in returns_whitelist:
         returns_meta += ['built-in', 'alternate', 'enum']
@@ -537,11 +541,15 @@ def check_command(expr, expr_info):
 def check_event(expr, expr_info):
     global events
     name = expr['event']
+    boxed = expr.get('boxed', False)
 
+    meta = ['struct']
+    if boxed:
+        meta += ['union', 'alternate']
     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 boxed, allow_optional=True,
+               allow_metas=meta)
 
 
 def check_union(expr, expr_info):
@@ -694,6 +702,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 == 'boxed' 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,
@@ -725,10 +737,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', 'boxed'])
             add_name(expr['command'], info, 'command')
         elif 'event' in expr:
-            check_keys(expr_elem, 'event', [], ['data'])
+            check_keys(expr_elem, 'event', [], ['data', 'boxed'])
             add_name(expr['event'], info, 'event')
         else:
             raise QAPIExprError(expr_elem['info'],
@@ -1163,6 +1175,9 @@ class QAPISchemaAlternateType(QAPISchemaType):
     def visit(self, visitor):
         visitor.visit_alternate_type(self.name, self.info, self.variants)
 
+    def is_empty(self):
+        return False
+
 
 class QAPISchemaCommand(QAPISchemaEntity):
     def __init__(self, name, info, arg_type, ret_type, gen, success_response,
@@ -1181,9 +1196,19 @@ class QAPISchemaCommand(QAPISchemaEntity):
     def check(self, schema):
         if self._arg_type_name:
             self.arg_type = schema.lookup_type(self._arg_type_name)
-            assert isinstance(self.arg_type, QAPISchemaObjectType)
-            assert not self.arg_type.variants   # not implemented
-            assert not self.boxed               # not implemented
+            assert (isinstance(self.arg_type, QAPISchemaObjectType) or
+                    isinstance(self.arg_type, QAPISchemaAlternateType))
+            self.arg_type.check(schema)
+            if self.boxed:
+                if self.arg_type.is_empty():
+                    raise QAPIExprError(self.info,
+                                        "Cannot use 'boxed' with empty type")
+            else:
+                assert not isinstance(self.arg_type, QAPISchemaAlternateType)
+                assert not self.arg_type.variants
+        elif self.boxed:
+            raise QAPIExprError(self.info,
+                                "Use of 'boxed' requires 'data'")
         if self._ret_type_name:
             self.ret_type = schema.lookup_type(self._ret_type_name)
             assert isinstance(self.ret_type, QAPISchemaType)
@@ -1205,9 +1230,19 @@ class QAPISchemaEvent(QAPISchemaEntity):
     def check(self, schema):
         if self._arg_type_name:
             self.arg_type = schema.lookup_type(self._arg_type_name)
-            assert isinstance(self.arg_type, QAPISchemaObjectType)
-            assert not self.arg_type.variants   # not implemented
-            assert not self.boxed               # not implemented
+            assert (isinstance(self.arg_type, QAPISchemaObjectType) or
+                    isinstance(self.arg_type, QAPISchemaAlternateType))
+            self.arg_type.check(schema)
+            if self.boxed:
+                if self.arg_type.is_empty():
+                    raise QAPIExprError(self.info,
+                                        "Cannot use 'boxed' with empty type")
+            else:
+                assert not isinstance(self.arg_type, QAPISchemaAlternateType)
+                assert not self.arg_type.variants
+        elif self.boxed:
+            raise QAPIExprError(self.info,
+                                "Use of 'boxed' requires 'data'")
 
     def visit(self, visitor):
         visitor.visit_event(self.name, self.info, self.arg_type, self.boxed)
@@ -1648,11 +1683,13 @@ extern const char *const %(c_name)s_lookup[];
 
 def gen_params(arg_type, boxed, extra):
     if not arg_type:
+        assert not boxed
         return extra
     ret = ''
     sep = ''
     if boxed:
-        assert False     # not implemented
+        ret += '%s arg' % arg_type.c_param_type()
+        sep = ', '
     else:
         assert not arg_type.variants
         for memb in arg_type.members:

diff --git a/tests/Makefile.include b/tests/Makefile.include
index a04c1991bb76686ddb6a1fbce26536825331792d..e7e50d6bd9409b1bdd77a35857797e3b0e32eaf8 100644 (file)
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -284,6 +284,10 @@ 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-boxed.json
+qapi-schema += args-boxed-anon.json
+qapi-schema += args-boxed-empty.json
+qapi-schema += args-boxed-string.json
 qapi-schema += args-int.json
 qapi-schema += args-invalid.json
 qapi-schema += args-member-array-bad.json
@@ -317,6 +321,7 @@ qapi-schema += enum-wrong-data.json
 qapi-schema += escape-outside-string.json
 qapi-schema += escape-too-big.json
 qapi-schema += escape-too-short.json
+qapi-schema += event-boxed-empty.json
 qapi-schema += event-case.json
 qapi-schema += event-nest-struct.json
 qapi-schema += flat-union-array-branch.json

diff --git a/tests/qapi-schema/args-bad-boxed.err b/tests/qapi-schema/args-bad-boxed.err
new file mode 100644 (file)
index 0000000..ad0d417
--- /dev/null
+++ b/tests/qapi-schema/args-bad-boxed.err
@@ -0,0 +1 @@
+tests/qapi-schema/args-bad-boxed.json:2: 'boxed' of command 'foo' should only use true value

diff --git a/tests/qapi-schema/args-bad-boxed.exit b/tests/qapi-schema/args-bad-boxed.exit
new file mode 100644 (file)
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/args-bad-boxed.exit
@@ -0,0 +1 @@
+1

diff --git a/tests/qapi-schema/args-bad-boxed.json b/tests/qapi-schema/args-bad-boxed.json
new file mode 100644 (file)
index 0000000..dea0cd0
--- /dev/null
+++ b/tests/qapi-schema/args-bad-boxed.json
@@ -0,0 +1,2 @@
+# 'boxed' should only appear with value true
+{ 'command': 'foo', 'boxed': false }

diff --git a/tests/qapi-schema/args-bad-boxed.out b/tests/qapi-schema/args-bad-boxed.out
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/tests/qapi-schema/args-boxed-anon.err b/tests/qapi-schema/args-boxed-anon.err
new file mode 100644 (file)
index 0000000..f24f345
--- /dev/null
+++ b/tests/qapi-schema/args-boxed-anon.err
@@ -0,0 +1 @@
+tests/qapi-schema/args-boxed-anon.json:2: 'data' for command 'foo' should be a type name

diff --git a/tests/qapi-schema/args-boxed-anon.exit b/tests/qapi-schema/args-boxed-anon.exit
new file mode 100644 (file)
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/args-boxed-anon.exit
@@ -0,0 +1 @@
+1

diff --git a/tests/qapi-schema/args-boxed-anon.json b/tests/qapi-schema/args-boxed-anon.json
new file mode 100644 (file)
index 0000000..95f60da
--- /dev/null
+++ b/tests/qapi-schema/args-boxed-anon.json
@@ -0,0 +1,2 @@
+# 'boxed' can only be used with named types
+{ 'command': 'foo', 'boxed': true, 'data': { 'string': 'str' } }

diff --git a/tests/qapi-schema/args-boxed-anon.out b/tests/qapi-schema/args-boxed-anon.out
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/tests/qapi-schema/args-boxed-empty.err b/tests/qapi-schema/args-boxed-empty.err
new file mode 100644 (file)
index 0000000..039603e
--- /dev/null
+++ b/tests/qapi-schema/args-boxed-empty.err
@@ -0,0 +1 @@
+tests/qapi-schema/args-boxed-empty.json:3: Cannot use 'boxed' with empty type

diff --git a/tests/qapi-schema/args-boxed-empty.exit b/tests/qapi-schema/args-boxed-empty.exit
new file mode 100644 (file)
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/args-boxed-empty.exit
@@ -0,0 +1 @@
+1

diff --git a/tests/qapi-schema/args-boxed-empty.json b/tests/qapi-schema/args-boxed-empty.json
new file mode 100644 (file)
index 0000000..52717e0
--- /dev/null
+++ b/tests/qapi-schema/args-boxed-empty.json
@@ -0,0 +1,3 @@
+# 'boxed' requires a non-empty type
+{ 'struct': 'Empty', 'data': {} }
+{ 'command': 'foo', 'boxed': true, 'data': 'Empty' }

diff --git a/tests/qapi-schema/args-boxed-empty.out b/tests/qapi-schema/args-boxed-empty.out
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/tests/qapi-schema/args-boxed-string.err b/tests/qapi-schema/args-boxed-string.err
new file mode 100644 (file)
index 0000000..d326b48
--- /dev/null
+++ b/tests/qapi-schema/args-boxed-string.err
@@ -0,0 +1 @@
+tests/qapi-schema/args-boxed-string.json:2: 'data' for command 'foo' cannot use built-in type 'str'

diff --git a/tests/qapi-schema/args-boxed-string.exit b/tests/qapi-schema/args-boxed-string.exit
new file mode 100644 (file)
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/args-boxed-string.exit
@@ -0,0 +1 @@
+1

diff --git a/tests/qapi-schema/args-boxed-string.json b/tests/qapi-schema/args-boxed-string.json
new file mode 100644 (file)
index 0000000..f91a150
--- /dev/null
+++ b/tests/qapi-schema/args-boxed-string.json
@@ -0,0 +1,2 @@
+# 'boxed' requires a complex (not built-in) type
+{ 'command': 'foo', 'boxed': true, 'data': 'str' }

diff --git a/tests/qapi-schema/args-boxed-string.out b/tests/qapi-schema/args-boxed-string.out
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/tests/qapi-schema/args-union.err b/tests/qapi-schema/args-union.err
index 1d693d74dae59cd748232a0a0d09dd5527a5abfb..f8ad223ddee62310aaa802b7b805dff3554c8654 100644 (file)
--- 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 7bdcbb7f085a5b77fd1d3a578e12fbe1114de604..2fcaeaae16305ef167f23611daefa514719abf90 100644 (file)
--- 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 'boxed':true
 { 'union': 'Uni', 'data': { 'case1': 'int', 'case2': 'str' } }
 { 'command': 'oops', 'data': 'Uni' }

diff --git a/tests/qapi-schema/event-boxed-empty.err b/tests/qapi-schema/event-boxed-empty.err
new file mode 100644 (file)
index 0000000..68ec6f2
--- /dev/null
+++ b/tests/qapi-schema/event-boxed-empty.err
@@ -0,0 +1 @@
+tests/qapi-schema/event-boxed-empty.json:2: Use of 'boxed' requires 'data'

diff --git a/tests/qapi-schema/event-boxed-empty.exit b/tests/qapi-schema/event-boxed-empty.exit
new file mode 100644 (file)
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/event-boxed-empty.exit
@@ -0,0 +1 @@
+1

diff --git a/tests/qapi-schema/event-boxed-empty.json b/tests/qapi-schema/event-boxed-empty.json
new file mode 100644 (file)
index 0000000..cb145f1
--- /dev/null
+++ b/tests/qapi-schema/event-boxed-empty.json
@@ -0,0 +1,2 @@
+# 'boxed' requires a non-empty type
+{ 'event': 'FOO', 'boxed': true }

diff --git a/tests/qapi-schema/event-boxed-empty.out b/tests/qapi-schema/event-boxed-empty.out
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index f571e1bb34ba966ba3c55ece71ce03337c629854..17194637baeedef2a13a7c8d691016abe99ad9e2 100644 (file)
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -127,6 +127,8 @@
 { 'command': 'guest-get-time', 'data': {'a': 'int', '*b': 'int' },
   'returns': 'int' }
 { 'command': 'guest-sync', 'data': { 'arg': 'any' }, 'returns': 'any' }
+{ 'command': 'boxed-struct', 'boxed': true, 'data': 'UserDefZero' }
+{ 'command': 'boxed-union', 'data': 'UserDefNativeListUnion', 'boxed': true }
 
 # For testing integer range flattening in opts-visitor. The following schema
 # corresponds to the option format:
@@ -154,6 +156,8 @@
   'data': { '*a': 'int', '*b': 'UserDefOne', 'c': 'str' } }
 { 'event': 'EVENT_D',
   'data': { 'a' : 'EventStructOne', 'b' : 'str', '*c': 'str', '*enum3': 'EnumOne' } }
+{ 'event': 'EVENT_E', 'boxed': true, 'data': 'UserDefZero' }
+{ 'event': 'EVENT_F', 'boxed': true, 'data': 'UserDefAlternate' }
 
 # 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 1aace719019fff142ff3027ec440275770c28acb..9d99c4eebbeb132ba5064eea330937b882a5fdb6 100644 (file)
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -30,6 +30,10 @@ event EVENT_C q_obj_EVENT_C-arg
    boxed=False
 event EVENT_D q_obj_EVENT_D-arg
    boxed=False
+event EVENT_E UserDefZero
+   boxed=True
+event EVENT_F UserDefAlternate
+   boxed=True
 object Empty1
 object Empty2
     base Empty1
@@ -153,6 +157,10 @@ object __org.qemu_x-Union2
     case __org.qemu_x-value: __org.qemu_x-Struct2
 command __org.qemu_x-command q_obj___org.qemu_x-command-arg -> __org.qemu_x-Union1
    gen=True success_response=True boxed=False
+command boxed-struct UserDefZero -> None
+   gen=True success_response=True boxed=True
+command boxed-union UserDefNativeListUnion -> None
+   gen=True success_response=True boxed=True
 command guest-get-time q_obj_guest-get-time-arg -> int
    gen=True success_response=True boxed=False
 command guest-sync q_obj_guest-sync-arg -> any

diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 8ffeb045cdd34e60f125a6b06f6d222af16c1450..5af1a468b84dc8ccaf08bb97d20833056f352335 100644 (file)
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -59,6 +59,14 @@ QObject *qmp_guest_sync(QObject *arg, Error **errp)
     return arg;
 }
 
+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,