Turn on the ability to pass command and event arguments in
a single boxed parameter. For structs, it makes it possible
to pass a single qapi type instead of a breakout of all
struct members; for unions, it is now possible to use a
union as the data for a command or event.
Generated code is unchanged, as long as no client uses the
new feature.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v7: rebase to latest
v6: retitle, rebase, and merge v5 40/46 and 41/46 into one patch
---
scripts/qapi.py | 29 +++++++++++++++++++++--------
scripts/qapi-commands.py | 3 ++-
scripts/qapi-event.py | 5 ++++-
tests/test-qmp-commands.c | 12 ++++++++++++
docs/qapi-code-gen.txt | 24 ++++++++++++++++++++++--
tests/Makefile | 2 ++
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-union.err | 2 +-
tests/qapi-schema/args-union.json | 3 +--
tests/qapi-schema/qapi-schema-test.json | 6 +++++-
tests/qapi-schema/qapi-schema-test.out | 10 +++++++++-
18 files changed, 87 insertions(+), 17 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
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 108363d..6742e7a 100644
--- 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']
+ box = expr.get('box', False)
+ args_meta = ['struct']
+ if box:
+ args_meta += ['union']
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']
@@ -537,11 +541,15 @@ 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']
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):
@@ -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 == '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,
@@ -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', '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'],
@@ -1640,12 +1652,13 @@ extern const char *const %(c_name)s_lookup[];
def gen_params(arg_type, box, extra):
- if not arg_type:
+ if not arg_type or arg_type.is_empty():
return extra
ret = ''
sep = ''
if box:
- assert False # not implemented
+ ret += '%s arg' % arg_type.c_param_type()
+ sep = ', '
else:
for memb in arg_type.members:
ret += sep
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 79d4eea..dc41fad 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -30,7 +30,8 @@ def gen_call(name, arg_type, box, ret_type):
argstr = ''
if box:
- assert False # not implemented
+ if 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 fd953fe..b8ca8c8 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -96,7 +96,10 @@ def gen_event_send(name, arg_type, box):
''')
if box:
- assert False # not implemented
+ ret += mcgen('''
+ visit_type_%(c_name)s(v, NULL, &arg, &err);
+''',
+ c_name=arg_type.c_name(), name=arg_type.name)
else:
ret += mcgen('''
visit_start_struct(v, "%(name)s", NULL, 0, &err);
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 87fc759..007db37 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -60,6 +60,18 @@ QObject *qmp_guest_sync(QObject *arg, Error **errp)
return arg;
}
+void qmp_boxed_empty(Error **errp)
+{
+}
+
+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,
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index aefc29e..40f050e 100644
--- 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, '*box': true,
'*gen': false, '*success-response': false }
Commands are defined by using a dictionary containing several members,
@@ -461,6 +461,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
@@ -484,7 +495,8 @@ use of this member.
=== 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
@@ -505,6 +517,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/tests/Makefile b/tests/Makefile
index c608f9a..5cd6177 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -272,6 +272,8 @@ 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-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-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 f571e1b..df91f3d 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -127,6 +127,9 @@
{ 'command': 'guest-get-time', 'data': {'a': 'int', '*b': 'int' },
'returns': 'int' }
{ 'command': 'guest-sync', 'data': { 'arg': 'any' }, 'returns': 'any' }
+{ 'command': 'boxed-empty', 'box': true }
+{ 'command': 'boxed-struct', 'box': true, 'data': 'UserDefZero' }
+{ 'command': 'boxed-union', 'data': 'UserDefNativeListUnion', 'box': true }
# For testing integer range flattening in opts-visitor. The following schema
# corresponds to the option format:
@@ -147,13 +150,14 @@
{ 'struct': 'EventStructOne',
'data': { 'struct1': 'UserDefOne', 'string': 'str', '*enum2': 'EnumOne' } }
-{ 'event': 'EVENT_A' }
+{ 'event': 'EVENT_A', 'box': true }
{ 'event': 'EVENT_B',
'data': { } }
{ 'event': 'EVENT_C',
'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, 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 754a926..8a00c6b 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -23,13 +23,15 @@ alternate AltStrNum
case s: str
case n: number
event EVENT_A None
- box=False
+ box=True
event EVENT_B None
box=False
event EVENT_C q_obj_EVENT_C-arg
box=False
event EVENT_D q_obj_EVENT_D-arg
box=False
+event EVENT_E UserDefZero
+ box=True
object Empty1
object Empty2
base Empty1
@@ -153,6 +155,12 @@ 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 box=False
+command boxed-empty None -> None
+ gen=True success_response=True box=True
+command boxed-struct UserDefZero -> None
+ gen=True success_response=True box=True
+command boxed-union UserDefNativeListUnion -> None
+ gen=True success_response=True box=True
command guest-get-time q_obj_guest-get-time-arg -> int
gen=True success_response=True box=False
command guest-sync q_obj_guest-sync-arg -> any
--
2.5.5
