[Qemu-devel] [PATCH RFC v4 25/29] qapi: Support boxed unions

[Eric Blake]

This patch completes support for boxed types, by allowing
union types to be used when 'box':true is specified.  It also
avoids a python crash when attempting to use boxing on an
anonymous type.  While it was possible to support 'box':true
on an empty event, it was easier to just reject missing 'data'
for commands.  Add some tests to cover new error messages, as
well as enhancing qapi-schema-test to cover situations that
are now valid.

Signed-off-by: Eric Blake <address@hidden>
---
 scripts/qapi.py                         | 22 +++++++++++++++++-----
 tests/Makefile                          |  5 ++---
 tests/qapi-schema/args-bad-box.err      |  1 +
 tests/qapi-schema/args-bad-box.exit     |  1 +
 tests/qapi-schema/args-bad-box.json     |  2 ++
 tests/qapi-schema/args-bad-box.out      |  0
 tests/qapi-schema/args-box-anon.err     |  1 +
 tests/qapi-schema/args-box-anon.exit    |  1 +
 tests/qapi-schema/args-box-anon.json    |  2 ++
 tests/qapi-schema/args-box-anon.out     |  0
 tests/qapi-schema/args-box-empty.err    |  1 +
 tests/qapi-schema/args-box-empty.exit   |  1 +
 tests/qapi-schema/args-box-empty.json   |  2 ++
 tests/qapi-schema/args-box-empty.out    |  0
 tests/qapi-schema/args-union.err        |  2 +-
 tests/qapi-schema/args-union.json       |  3 +--
 tests/qapi-schema/qapi-schema-test.json |  3 ++-
 tests/qapi-schema/qapi-schema-test.out  |  4 +++-
 tests/test-qmp-commands.c               |  4 ++++
 19 files changed, 42 insertions(+), 13 deletions(-)
 create mode 100644 tests/qapi-schema/args-bad-box.err
 create mode 100644 tests/qapi-schema/args-bad-box.exit
 create mode 100644 tests/qapi-schema/args-bad-box.json
 create mode 100644 tests/qapi-schema/args-bad-box.out
 create mode 100644 tests/qapi-schema/args-box-anon.err
 create mode 100644 tests/qapi-schema/args-box-anon.exit
 create mode 100644 tests/qapi-schema/args-box-anon.json
 create mode 100644 tests/qapi-schema/args-box-anon.out
 create mode 100644 tests/qapi-schema/args-box-empty.err
 create mode 100644 tests/qapi-schema/args-box-empty.exit
 create mode 100644 tests/qapi-schema/args-box-empty.json
 create mode 100644 tests/qapi-schema/args-box-empty.out

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 92e9e74..181d40f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -492,10 +492,18 @@ def check_member_clash(expr_info, base_name, data, source 
= ""):

 def check_command(expr, expr_info):
     name = expr['command']
+    box = expr.get('box', False)

+    args_meta = ['struct']
+    if box:
+        args_meta += ['union']
+        if not expr.get('data'):
+            raise QAPIExprError(expr_info,
+                                "Use of 'box' requires 'data' for command '%s'"
+                                % name)
     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 box, allow_optional=True,
+               allow_metas=args_meta)
     returns_meta = ['union', 'struct']
     if name in returns_whitelist:
         returns_meta += ['built-in', 'alternate', 'enum']
@@ -506,13 +514,17 @@ def check_command(expr, expr_info):
 def check_event(expr, expr_info):
     global events
     name = expr['event']
+    box = expr.get('box', False)
+    meta = ['struct']

+    if box:
+        meta += ['union']
     if name.upper() == 'MAX':
         raise QAPIExprError(expr_info, "Event name 'MAX' cannot be created")
     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 box, allow_optional=True,
+               allow_metas=meta)

 def check_union(expr, expr_info):
     name = expr['union']
@@ -1488,7 +1500,7 @@ def gen_params(arg_type, box, extra):
         return extra
     ret = ''
     sep = ''
-    if box:
+    if box and arg_type.members:
         ret += '%s arg' % arg_type.c_type(is_param=True)
         sep = ', '
     else:
diff --git a/tests/Makefile b/tests/Makefile
index d1d647f..ebc6809 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -229,12 +229,11 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
        bad-type-dict.json double-data.json unknown-expr-key.json \
        redefined-type.json redefined-command.json redefined-builtin.json \
        redefined-event.json command-int.json bad-data.json event-max.json \
-       type-bypass-bad-gen.json \
-       args-invalid.json \
+       type-bypass-bad-gen.json args-invalid.json args-bad-box.json \
        args-array-empty.json args-array-unknown.json args-int.json \
        args-unknown.json args-member-unknown.json args-member-array.json \
        args-member-array-bad.json args-alternate.json args-union.json \
-       args-any.json \
+       args-any.json args-box-anon.json args-box-empty.json \
        returns-array-bad.json returns-int.json returns-dict.json \
        returns-unknown.json returns-alternate.json returns-whitelist.json \
        missing-colon.json missing-comma-list.json missing-comma-object.json \
diff --git a/tests/qapi-schema/args-bad-box.err 
b/tests/qapi-schema/args-bad-box.err
new file mode 100644
index 0000000..16afe3c
--- /dev/null
+++ b/tests/qapi-schema/args-bad-box.err
@@ -0,0 +1 @@
+tests/qapi-schema/args-bad-box.json:2: 'box' of command 'foo' should only use 
true value
diff --git a/tests/qapi-schema/args-bad-box.exit 
b/tests/qapi-schema/args-bad-box.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/args-bad-box.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/args-bad-box.json 
b/tests/qapi-schema/args-bad-box.json
new file mode 100644
index 0000000..8d5737a
--- /dev/null
+++ b/tests/qapi-schema/args-bad-box.json
@@ -0,0 +1,2 @@
+# 'box' should only appear with value true
+{ 'command': 'foo', 'box': false }
diff --git a/tests/qapi-schema/args-bad-box.out 
b/tests/qapi-schema/args-bad-box.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/args-box-anon.err 
b/tests/qapi-schema/args-box-anon.err
new file mode 100644
index 0000000..11eaefc
--- /dev/null
+++ b/tests/qapi-schema/args-box-anon.err
@@ -0,0 +1 @@
+tests/qapi-schema/args-box-anon.json:2: 'data' for command 'foo' should be a 
type name
diff --git a/tests/qapi-schema/args-box-anon.exit 
b/tests/qapi-schema/args-box-anon.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/args-box-anon.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/args-box-anon.json 
b/tests/qapi-schema/args-box-anon.json
new file mode 100644
index 0000000..947e3c6
--- /dev/null
+++ b/tests/qapi-schema/args-box-anon.json
@@ -0,0 +1,2 @@
+# 'box' can only be used with named types
+{ 'command': 'foo', 'box': true, 'data': { 'string': 'str' } }
diff --git a/tests/qapi-schema/args-box-anon.out 
b/tests/qapi-schema/args-box-anon.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/args-box-empty.err 
b/tests/qapi-schema/args-box-empty.err
new file mode 100644
index 0000000..574c8b5
--- /dev/null
+++ b/tests/qapi-schema/args-box-empty.err
@@ -0,0 +1 @@
+tests/qapi-schema/args-box-empty.json:2: Use of 'box' requires 'data' for 
command 'foo'
diff --git a/tests/qapi-schema/args-box-empty.exit 
b/tests/qapi-schema/args-box-empty.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/args-box-empty.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/args-box-empty.json 
b/tests/qapi-schema/args-box-empty.json
new file mode 100644
index 0000000..31dca5e
--- /dev/null
+++ b/tests/qapi-schema/args-box-empty.json
@@ -0,0 +1,2 @@
+# 'box' can only be used with named types
+{ 'command': 'foo', 'box': true }
diff --git a/tests/qapi-schema/args-box-empty.out 
b/tests/qapi-schema/args-box-empty.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/args-union.err b/tests/qapi-schema/args-union.err
index 1d693d7..f8ad223 100644
--- 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 7bdcbb7..c0ce091 100644
--- 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 'box':true
 { 'union': 'Uni', 'data': { 'case1': 'int', 'case2': 'str' } }
 { 'command': 'oops', 'data': 'Uni' }
diff --git a/tests/qapi-schema/qapi-schema-test.json 
b/tests/qapi-schema/qapi-schema-test.json
index 92fc178..585bc0e 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -95,6 +95,7 @@
 # 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' }
+{ 'command': 'boxed2', 'data': 'UserDefNativeListUnion', 'box': true }

 # For testing integer range flattening in opts-visitor. The following schema
 # corresponds to the option format:
@@ -115,7 +116,7 @@
 { 'struct': 'EventStructOne',
   'data': { 'struct1': 'UserDefOne', 'string': 'str', '*enum2': 'EnumOne' } }

-{ 'event': 'EVENT_A' }
+{ 'event': 'EVENT_A', 'box': true }
 { 'event': 'EVENT_B',
   'data': { } }
 { 'event': 'EVENT_C',
diff --git a/tests/qapi-schema/qapi-schema-test.out 
b/tests/qapi-schema/qapi-schema-test.out
index 734cd76..4127b86 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -72,7 +72,7 @@ alternate AltTwo
     case s: str
     case n: number
 event EVENT_A None
-   box=False
+   box=True
 event EVENT_B None
    box=False
 event EVENT_C :obj-EVENT_C-arg
@@ -182,6 +182,8 @@ command __org.qemu_x-command :obj-__org.qemu_x-command-arg 
-> __org.qemu_x-Union
    gen=True success_response=True box=False
 command boxed UserDefZero -> None
    gen=True success_response=True box=True
+command boxed2 UserDefNativeListUnion -> 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 a190f31..886de98 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -58,6 +58,10 @@ void qmp_boxed(UserDefZero *arg, Error **errp)
 {
 }

+void qmp_boxed2(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,
-- 
2.4.3