[Qemu-devel] [PATCH v5 41/46] 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                          |  3 +++
 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, 43 insertions(+), 10 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 a54ff64..66d9000 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -501,10 +501,18 @@ def check_type(expr_info, source, value, 
allow_array=False,

 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']
@@ -516,13 +524,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):
@@ -1567,7 +1579,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 638bf50..8ce3665 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -234,6 +234,9 @@ 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-box.json
+qapi-schema += args-box-anon.json
+qapi-schema += args-box-empty.json
 qapi-schema += args-int.json
 qapi-schema += args-invalid.json
 qapi-schema += args-member-array-bad.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 f0f5ad2..4d7fb07 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -104,6 +104,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:
@@ -124,7 +125,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 fb58d54..a8730fc 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
@@ -189,6 +189,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