* [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types
@ 2016-03-10 0:55 Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 01/14] qapi: Assert in places where variants are not handled Eric Blake
` (14 more replies)
0 siblings, 15 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-10 0:55 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Eduardo Habkost
Addresses Markus' comments on v4; the series was pretty close, and
most of the changes here are a new naming scheme ('q_obj' rather
than ':obj') and additional cleanups (for example, c_null() is
unused). I dropped patch 10, but the new patches make the series
a bit longer.
Built directly on master qemu.git.
Also available as a tag at this location:
git fetch git://repo.or.cz/qemu/ericb.git qapi-implicit-v5
and will soon be part of my branch at:
http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/qapi
v4: https://lists.gnu.org/archive/html/qemu-devel/2016-03/msg01205.html
001/14:[0001] [FC] 'qapi: Assert in places where variants are not handled'
002/14:[----] [--] 'qapi: Fix command with named empty argument type'
003/14:[----] [--] 'qapi: Make c_type() more OO-like'
004/14:[down] 'qapi: Adjust names of implicit types'
005/14:[0019] [FC] 'qapi: Emit implicit structs in generated C'
006/14:[down] 'qapi-event: Slightly shrink generated code'
007/14:[0055] [FC] 'qapi: Utilize implicit struct visits'
008/14:[----] [--] 'qapi-commands: Inline single-use helpers of gen_marshal()'
009/14:[down] 'qapi: Inline gen_visit_members() into lone caller'
010/14:[down] 'qapi: Drop unused c_null()'
011/14:[0001] [FC] 'qapi: Don't special-case simple union wrappers'
012/14:[down] 'qapi: Make BlockdevOptions doc example closer to reality'
013/14:[0026] [FC] 'qapi: Allow anonymous base for flat union'
014/14:[0002] [FC] 'qapi: Use anonymous bases in QMP flat unions'
Eric Blake (14):
qapi: Assert in places where variants are not handled
qapi: Fix command with named empty argument type
qapi: Make c_type() more OO-like
qapi: Adjust names of implicit types
qapi: Emit implicit structs in generated C
qapi-event: Slightly shrink generated code
qapi: Utilize implicit struct visits
qapi-commands: Inline single-use helpers of gen_marshal()
qapi: Inline gen_visit_members() into lone caller
qapi: Drop unused c_null()
qapi: Don't special-case simple union wrappers
qapi: Make BlockdevOptions doc example closer to reality
qapi: Allow anonymous base for flat union
qapi: Use anonymous bases in QMP flat unions
scripts/qapi.py | 171 ++++++++++++-----------------
scripts/qapi-commands.py | 117 ++++++++------------
scripts/qapi-event.py | 58 +++++++---
scripts/qapi-types.py | 29 ++---
scripts/qapi-visit.py | 65 +++++------
backends/baum.c | 2 +-
backends/msmouse.c | 2 +-
block/nbd.c | 6 +-
block/qcow2.c | 6 +-
block/vmdk.c | 8 +-
blockdev.c | 24 ++--
hmp.c | 8 +-
hw/char/escc.c | 2 +-
hw/input/hid.c | 8 +-
hw/input/ps2.c | 6 +-
hw/input/virtio-input-hid.c | 8 +-
hw/mem/pc-dimm.c | 2 +-
net/dump.c | 2 +-
net/hub.c | 2 +-
net/l2tpv3.c | 2 +-
net/net.c | 4 +-
net/netmap.c | 2 +-
net/slirp.c | 2 +-
net/socket.c | 2 +-
net/tap-win32.c | 2 +-
net/tap.c | 4 +-
net/vde.c | 2 +-
net/vhost-user.c | 2 +-
numa.c | 4 +-
qemu-char.c | 82 +++++++-------
qemu-nbd.c | 6 +-
replay/replay-input.c | 44 ++++----
spice-qemu-char.c | 14 ++-
tests/test-io-channel-socket.c | 40 +++----
tests/test-qmp-commands.c | 7 +-
tests/test-qmp-input-visitor.c | 25 +++--
tests/test-qmp-output-visitor.c | 24 ++--
tpm.c | 2 +-
ui/console.c | 4 +-
ui/input-keymap.c | 10 +-
ui/input-legacy.c | 8 +-
ui/input.c | 34 +++---
ui/vnc-auth-sasl.c | 3 +-
ui/vnc.c | 29 ++---
util/qemu-sockets.c | 35 +++---
docs/qapi-code-gen.txt | 98 ++++++++---------
qapi-schema.json | 20 +---
qapi/block-core.json | 98 ++++++++---------
qapi/introspect.json | 12 +-
tests/qapi-schema/comments.out | 2 +-
tests/qapi-schema/empty.out | 2 +-
tests/qapi-schema/event-case.out | 2 +-
tests/qapi-schema/flat-union-bad-base.err | 2 +-
tests/qapi-schema/flat-union-bad-base.json | 5 +-
tests/qapi-schema/ident-with-escape.out | 8 +-
tests/qapi-schema/include-relpath.out | 2 +-
tests/qapi-schema/include-repetition.out | 2 +-
tests/qapi-schema/include-simple.out | 2 +-
tests/qapi-schema/indented-expr.out | 2 +-
tests/qapi-schema/qapi-schema-test.json | 8 +-
tests/qapi-schema/qapi-schema-test.out | 166 ++++++++++++++--------------
61 files changed, 648 insertions(+), 702 deletions(-)
--
2.5.0
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v5 01/14] qapi: Assert in places where variants are not handled
2016-03-10 0:55 [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Eric Blake
@ 2016-03-10 0:55 ` Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 02/14] qapi: Fix command with named empty argument type Eric Blake
` (13 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-10 0:55 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
We are getting closer to the point where we could use one union
as the base or variant type within another union type (as long
as there are no collisions between any possible combination of
member names allowed across all discriminator choices). But
until we get to that point, it is worth asserting that variants
are not present in places where we are not prepared to handle
them: when exploding a type into a parameter list, we do not
expect variants. The qapi.py code is already checking this,
via the older check_type() method; but someday we hope to get
rid of that and move checking into QAPISchema*.check(). The
two asserts added here make sure any refactoring still catches
problems, and makes it locally obvious why we can iterate over
only type.members without worrying about type.variants.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v5: drop redundant hunk, improve commit message
v4: new patch, split out from .is_empty() patch
---
scripts/qapi-commands.py | 3 ++-
scripts/qapi-event.py | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index f44e01f..edcbd10 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -2,7 +2,7 @@
# QAPI command marshaller generator
#
# Copyright IBM, Corp. 2011
-# Copyright (C) 2014-2015 Red Hat, Inc.
+# Copyright (C) 2014-2016 Red Hat, Inc.
#
# Authors:
# Anthony Liguori <aliguori@us.ibm.com>
@@ -30,6 +30,7 @@ def gen_call(name, arg_type, ret_type):
argstr = ''
if arg_type:
+ assert not arg_type.variants
for memb in arg_type.members:
if memb.optional:
argstr += 'has_%s, ' % c_name(memb.name)
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index fb579dd..c03cb78 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -59,6 +59,7 @@ def gen_event_send(name, arg_type):
name=name)
if arg_type and arg_type.members:
+ assert not arg_type.variants
ret += mcgen('''
qov = qmp_output_visitor_new();
v = qmp_output_get_visitor(qov);
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v5 02/14] qapi: Fix command with named empty argument type
2016-03-10 0:55 [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 01/14] qapi: Assert in places where variants are not handled Eric Blake
@ 2016-03-10 0:55 ` Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 03/14] qapi: Make c_type() more OO-like Eric Blake
` (12 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-10 0:55 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
The generator special-cased
{ 'command':'foo', 'data': {} }
to avoid emitting a visitor variable, but failed to see that
{ 'struct':'NamedEmptyType, 'data': {} }
{ 'command':'foo', 'data':'NamedEmptyType' }
needs the same treatment. There, the generator happily generates a
visitor to get no arguments, and a visitor to destroy no arguments;
and the compiler isn't happy with that, as demonstrated by the updated
qapi-schema-test.json:
tests/test-qmp-marshal.c: In function ‘qmp_marshal_user_def_cmd0’:
tests/test-qmp-marshal.c:264:14: error: variable ‘v’ set but not used [-Werror=unused-but-set-variable]
Visitor *v;
^
No change to generated code except for the testsuite addition.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v5: no change
v4: move earlier in series, rebase without is_empty, drop R-b
[no v3]
v2: no change
v1: enhance commit message
Previously posted as part of qapi cleanup subset E:
v9: no change
v8: no change
v7: no change
v6: new patch
---
scripts/qapi-commands.py | 4 ++--
tests/test-qmp-commands.c | 5 +++++
tests/qapi-schema/qapi-schema-test.json | 2 ++
tests/qapi-schema/qapi-schema-test.out | 2 ++
4 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index edcbd10..3784f33 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -66,7 +66,7 @@ def gen_marshal_vars(arg_type, ret_type):
''',
c_type=ret_type.c_type())
- if arg_type:
+ if arg_type and arg_type.members:
ret += mcgen('''
QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
QapiDeallocVisitor *qdv;
@@ -98,7 +98,7 @@ def gen_marshal_vars(arg_type, ret_type):
def gen_marshal_input_visit(arg_type, dealloc=False):
ret = ''
- if not arg_type:
+ if not arg_type or not arg_type.members:
return ret
if dealloc:
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index d6171f2..650ba46 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -13,6 +13,11 @@ void qmp_user_def_cmd(Error **errp)
{
}
+Empty2 *qmp_user_def_cmd0(Error **errp)
+{
+ return g_new0(Empty2, 1);
+}
+
void qmp_user_def_cmd1(UserDefOne * ud1, Error **errp)
{
}
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 728659e..e722748 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -18,6 +18,8 @@
{ 'struct': 'Empty1', 'data': { } }
{ 'struct': 'Empty2', 'base': 'Empty1', 'data': { } }
+{ 'command': 'user_def_cmd0', 'data': 'Empty2', 'returns': 'Empty2' }
+
# for testing override of default naming heuristic
{ 'enum': 'QEnumTwo',
'prefix': 'QENUM_TWO',
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index f5e2a73..f531961 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -203,6 +203,8 @@ command guest-sync :obj-guest-sync-arg -> any
gen=True success_response=True
command user_def_cmd None -> None
gen=True success_response=True
+command user_def_cmd0 Empty2 -> Empty2
+ gen=True success_response=True
command user_def_cmd1 :obj-user_def_cmd1-arg -> None
gen=True success_response=True
command user_def_cmd2 :obj-user_def_cmd2-arg -> UserDefTwo
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v5 03/14] qapi: Make c_type() more OO-like
2016-03-10 0:55 [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 01/14] qapi: Assert in places where variants are not handled Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 02/14] qapi: Fix command with named empty argument type Eric Blake
@ 2016-03-10 0:55 ` Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 04/14] qapi: Adjust names of implicit types Eric Blake
` (11 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-10 0:55 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
QAPISchemaType.c_type() is a bit awkward: it takes two optional
boolean flags is_param and is_unboxed, and they should never both
be True.
Add a new method for each of the flags, and drop the flags from
c_type().
Most callers pass no flags; they remain unchanged.
One caller passes is_param=True; call the new .c_param_type()
instead.
One caller passes is_unboxed=True, except for simple union types.
This is actually an ugly special case that will go away soon, so
until then, we now have to call either .c_type() or the new
.c_unboxed_type(). Tolerable in the interim.
It requires slightly more Python, but is arguably easier to read.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v5: improve commit message
v4: hoist earlier in series, rework simple union hack in qapi-types,
improve docs
[no v3]
v2: no change
---
scripts/qapi.py | 39 ++++++++++++++++++++++++++++++---------
scripts/qapi-types.py | 7 +++++--
2 files changed, 35 insertions(+), 11 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 6b2aa6e..b7fbdae 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -822,8 +822,18 @@ class QAPISchemaVisitor(object):
class QAPISchemaType(QAPISchemaEntity):
- def c_type(self, is_param=False, is_unboxed=False):
- return c_name(self.name) + pointer_suffix
+ # Return the C type for common use.
+ # For the types we commonly box, this is a pointer type.
+ def c_type(self):
+ pass
+
+ # Return the C type to be used in a parameter list.
+ def c_param_type(self):
+ return self.c_type()
+
+ # Return the C type to be used where we suppress boxing.
+ def c_unboxed_type(self):
+ return self.c_type()
def c_null(self):
return 'NULL'
@@ -855,8 +865,11 @@ class QAPISchemaBuiltinType(QAPISchemaType):
def c_name(self):
return self.name
- def c_type(self, is_param=False, is_unboxed=False):
- if is_param and self.name == 'str':
+ def c_type(self):
+ return self._c_type_name
+
+ def c_param_type(self):
+ if self.name == 'str':
return 'const ' + self._c_type_name
return self._c_type_name
@@ -889,7 +902,7 @@ class QAPISchemaEnumType(QAPISchemaType):
# See QAPISchema._make_implicit_enum_type()
return self.name.endswith('Kind')
- def c_type(self, is_param=False, is_unboxed=False):
+ def c_type(self):
return c_name(self.name)
def member_names(self):
@@ -921,6 +934,9 @@ class QAPISchemaArrayType(QAPISchemaType):
def is_implicit(self):
return True
+ def c_type(self):
+ return c_name(self.name) + pointer_suffix
+
def json_type(self):
return 'array'
@@ -985,12 +1001,14 @@ class QAPISchemaObjectType(QAPISchemaType):
assert not self.is_implicit()
return QAPISchemaType.c_name(self)
- def c_type(self, is_param=False, is_unboxed=False):
+ def c_type(self):
assert not self.is_implicit()
- if is_unboxed:
- return c_name(self.name)
return c_name(self.name) + pointer_suffix
+ def c_unboxed_type(self):
+ assert not self.is_implicit()
+ return c_name(self.name)
+
def json_type(self):
return 'object'
@@ -1139,6 +1157,9 @@ class QAPISchemaAlternateType(QAPISchemaType):
for v in self.variants.variants:
v.check_clash(self.info, seen)
+ def c_type(self):
+ return c_name(self.name) + pointer_suffix
+
def json_type(self):
return 'value'
@@ -1630,7 +1651,7 @@ def gen_params(arg_type, extra):
sep = ', '
if memb.optional:
ret += 'bool has_%s, ' % c_name(memb.name)
- ret += '%s %s' % (memb.type.c_type(is_param=True), c_name(memb.name))
+ ret += '%s %s' % (memb.type.c_param_type(), c_name(memb.name))
if extra:
ret += sep + extra
return ret
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 0306a88..f194bea 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -124,11 +124,14 @@ def gen_variants(variants):
for var in variants.variants:
# Ugly special case for simple union TODO get rid of it
simple_union_type = var.simple_union_type()
- typ = simple_union_type or var.type
+ if simple_union_type:
+ typ = simple_union_type.c_type()
+ else:
+ typ = var.type.c_unboxed_type()
ret += mcgen('''
%(c_type)s %(c_name)s;
''',
- c_type=typ.c_type(is_unboxed=not simple_union_type),
+ c_type=typ,
c_name=c_name(var.name))
ret += mcgen('''
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v5 04/14] qapi: Adjust names of implicit types
2016-03-10 0:55 [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Eric Blake
` (2 preceding siblings ...)
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 03/14] qapi: Make c_type() more OO-like Eric Blake
@ 2016-03-10 0:55 ` Eric Blake
2016-03-10 13:39 ` Markus Armbruster
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 05/14] qapi: Emit implicit structs in generated C Eric Blake
` (10 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2016-03-10 0:55 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
The original choice of ':obj-' as the prefix for implicit types
made it obvious that we weren't going to clash with any user-defined
names. But now we want to create structs for implicit types. We
could transliterate ':' to '_', except that C99 says that a leading
underscore and lower-case letter should be used only for file scope
identifiers, while we would be exposing it in qapi-types.h. So it's
time to change our naming convention; we can instead use the 'q_'
prefix that we reserved for ourselves back in commit 9fb081e0. As
long as we don't declare 'empty' or 'obj' ticklish, it shouldn't
clash with c_name() prepending 'q_' to the user's ticklish names.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v5: new patch
---
scripts/qapi.py | 18 ++--
docs/qapi-code-gen.txt | 14 +--
| 2 +-
tests/qapi-schema/empty.out | 2 +-
tests/qapi-schema/event-case.out | 2 +-
tests/qapi-schema/ident-with-escape.out | 8 +-
tests/qapi-schema/include-relpath.out | 2 +-
tests/qapi-schema/include-repetition.out | 2 +-
tests/qapi-schema/include-simple.out | 2 +-
tests/qapi-schema/indented-expr.out | 2 +-
tests/qapi-schema/qapi-schema-test.out | 154 +++++++++++++++----------------
11 files changed, 105 insertions(+), 103 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index b7fbdae..f6701f5 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -391,7 +391,8 @@ def check_name(expr_info, source, name, allow_optional=False,
# code always prefixes it with the enum name
if enum_member and membername[0].isdigit():
membername = 'D' + membername
- # Reserve the entire 'q_' namespace for c_name()
+ # Reserve the entire 'q_' namespace for c_name(), and for 'q_empty'
+ # and 'q_obj_*' implicit type names.
if not valid_name.match(membername) or \
c_name(membername, False).startswith('q_'):
raise QAPIExprError(expr_info,
@@ -994,8 +995,9 @@ class QAPISchemaObjectType(QAPISchemaType):
m.check_clash(info, seen)
def is_implicit(self):
- # See QAPISchema._make_implicit_object_type()
- return self.name[0] == ':'
+ # See QAPISchema._make_implicit_object_type(), as well as
+ # _def_predefineds()
+ return self.name.startswith('q_')
def c_name(self):
assert not self.is_implicit()
@@ -1044,10 +1046,10 @@ class QAPISchemaMember(object):
def _pretty_owner(self):
owner = self.owner
- if owner.startswith(':obj-'):
+ if owner.startswith('q_obj_'):
# See QAPISchema._make_implicit_object_type() - reverse the
# mapping there to create a nice human-readable description
- owner = owner[5:]
+ owner = owner[6:]
if owner.endswith('-arg'):
return '(parameter of %s)' % owner[:-4]
else:
@@ -1266,8 +1268,8 @@ class QAPISchema(object):
('bool', 'boolean', 'bool', 'false'),
('any', 'value', 'QObject' + pointer_suffix, 'NULL')]:
self._def_builtin_type(*t)
- self.the_empty_object_type = QAPISchemaObjectType(':empty', None, None,
- [], None)
+ self.the_empty_object_type = QAPISchemaObjectType('q_empty', None,
+ None, [], None)
self._def_entity(self.the_empty_object_type)
qtype_values = self._make_enum_members(['none', 'qnull', 'qint',
'qstring', 'qdict', 'qlist',
@@ -1295,7 +1297,7 @@ class QAPISchema(object):
if not members:
return None
# See also QAPISchemaObjectTypeMember._pretty_owner()
- name = ':obj-%s-%s' % (name, role)
+ name = 'q_obj_%s-%s' % (name, role)
if not self.lookup_entity(name, QAPISchemaObjectType):
self._def_entity(QAPISchemaObjectType(name, info, None,
members, None))
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index e0b2ef1..c648f76 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -575,9 +575,9 @@ names an object type without members.
Example: the SchemaInfo for command query-qmp-schema
{ "name": "query-qmp-schema", "meta-type": "command",
- "arg-type": ":empty", "ret-type": "SchemaInfoList" }
+ "arg-type": "q_empty", "ret-type": "SchemaInfoList" }
- Type ":empty" is an object type without members, and type
+ Type "q_empty" is an automatic object type without members, and type
"SchemaInfoList" is the array of SchemaInfo type.
The SchemaInfo for an event has meta-type "event", and variant member
@@ -594,9 +594,9 @@ QAPI schema implicitly defines an object type.
Example: the SchemaInfo for EVENT_C from section Events
{ "name": "EVENT_C", "meta-type": "event",
- "arg-type": ":obj-EVENT_C-arg" }
+ "arg-type": "q_obj-EVENT_C-arg" }
- Type ":obj-EVENT_C-arg" is an implicitly defined object type with
+ Type "q_obj-EVENT_C-arg" is an implicitly defined object type with
the two members from the event's definition.
The SchemaInfo for struct and union types has meta-type "object".
@@ -660,11 +660,11 @@ Union types
{ "name": "type", "type": "BlockdevOptionsKind" } ],
"tag": "type",
"variants": [
- { "case": "file", "type": ":obj-FileOptions-wrapper" },
- { "case": "qcow2", "type": ":obj-Qcow2Options-wrapper" } ] }
+ { "case": "file", "type": "q_obj-FileOptions-wrapper" },
+ { "case": "qcow2", "type": "q_obj-Qcow2Options-wrapper" } ] }
Enumeration type "BlockdevOptionsKind" and the object types
- ":obj-FileOptions-wrapper", ":obj-Qcow2Options-wrapper" are
+ "q_obj-FileOptions-wrapper", "q_obj-Qcow2Options-wrapper" are
implicitly defined.
The SchemaInfo for an alternate type has meta-type "alternate", and
--git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out
index 97be601..5d7c13c 100644
--- a/tests/qapi-schema/comments.out
+++ b/tests/qapi-schema/comments.out
@@ -1,4 +1,4 @@
-object :empty
enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
prefix QTYPE
enum Status ['good', 'bad', 'ugly']
+object q_empty
diff --git a/tests/qapi-schema/empty.out b/tests/qapi-schema/empty.out
index 6522940..8a5b034 100644
--- a/tests/qapi-schema/empty.out
+++ b/tests/qapi-schema/empty.out
@@ -1,3 +1,3 @@
-object :empty
enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
prefix QTYPE
+object q_empty
diff --git a/tests/qapi-schema/event-case.out b/tests/qapi-schema/event-case.out
index 6350d64..b6b4134 100644
--- a/tests/qapi-schema/event-case.out
+++ b/tests/qapi-schema/event-case.out
@@ -1,4 +1,4 @@
-object :empty
enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
prefix QTYPE
event oops None
+object q_empty
diff --git a/tests/qapi-schema/ident-with-escape.out b/tests/qapi-schema/ident-with-escape.out
index 453e0b2..382ce2f 100644
--- a/tests/qapi-schema/ident-with-escape.out
+++ b/tests/qapi-schema/ident-with-escape.out
@@ -1,7 +1,7 @@
-object :empty
-object :obj-fooA-arg
- member bar1: str optional=False
enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
prefix QTYPE
-command fooA :obj-fooA-arg -> None
+command fooA q_obj_fooA-arg -> None
gen=True success_response=True
+object q_empty
+object q_obj_fooA-arg
+ member bar1: str optional=False
diff --git a/tests/qapi-schema/include-relpath.out b/tests/qapi-schema/include-relpath.out
index 97be601..5d7c13c 100644
--- a/tests/qapi-schema/include-relpath.out
+++ b/tests/qapi-schema/include-relpath.out
@@ -1,4 +1,4 @@
-object :empty
enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
prefix QTYPE
enum Status ['good', 'bad', 'ugly']
+object q_empty
diff --git a/tests/qapi-schema/include-repetition.out b/tests/qapi-schema/include-repetition.out
index 97be601..5d7c13c 100644
--- a/tests/qapi-schema/include-repetition.out
+++ b/tests/qapi-schema/include-repetition.out
@@ -1,4 +1,4 @@
-object :empty
enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
prefix QTYPE
enum Status ['good', 'bad', 'ugly']
+object q_empty
diff --git a/tests/qapi-schema/include-simple.out b/tests/qapi-schema/include-simple.out
index 97be601..5d7c13c 100644
--- a/tests/qapi-schema/include-simple.out
+++ b/tests/qapi-schema/include-simple.out
@@ -1,4 +1,4 @@
-object :empty
enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
prefix QTYPE
enum Status ['good', 'bad', 'ugly']
+object q_empty
diff --git a/tests/qapi-schema/indented-expr.out b/tests/qapi-schema/indented-expr.out
index ce37ff5..ae3293a 100644
--- a/tests/qapi-schema/indented-expr.out
+++ b/tests/qapi-schema/indented-expr.out
@@ -1,7 +1,7 @@
-object :empty
enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
prefix QTYPE
command eins None -> None
gen=True success_response=True
+object q_empty
command zwei None -> None
gen=True success_response=True
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index f531961..d49fe1d 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -1,58 +1,3 @@
-object :empty
-object :obj-EVENT_C-arg
- member a: int optional=True
- member b: UserDefOne optional=True
- member c: str optional=False
-object :obj-EVENT_D-arg
- member a: EventStructOne optional=False
- member b: str optional=False
- member c: str optional=True
- member enum3: EnumOne optional=True
-object :obj-__org.qemu_x-command-arg
- member a: __org.qemu_x-EnumList optional=False
- member b: __org.qemu_x-StructList optional=False
- member c: __org.qemu_x-Union2 optional=False
- member d: __org.qemu_x-Alt optional=False
-object :obj-anyList-wrapper
- member data: anyList optional=False
-object :obj-boolList-wrapper
- member data: boolList optional=False
-object :obj-guest-get-time-arg
- member a: int optional=False
- member b: int optional=True
-object :obj-guest-sync-arg
- member arg: any optional=False
-object :obj-int16List-wrapper
- member data: int16List optional=False
-object :obj-int32List-wrapper
- member data: int32List optional=False
-object :obj-int64List-wrapper
- member data: int64List optional=False
-object :obj-int8List-wrapper
- member data: int8List optional=False
-object :obj-intList-wrapper
- member data: intList optional=False
-object :obj-numberList-wrapper
- member data: numberList optional=False
-object :obj-sizeList-wrapper
- member data: sizeList optional=False
-object :obj-str-wrapper
- member data: str optional=False
-object :obj-strList-wrapper
- member data: strList optional=False
-object :obj-uint16List-wrapper
- member data: uint16List optional=False
-object :obj-uint32List-wrapper
- member data: uint32List optional=False
-object :obj-uint64List-wrapper
- member data: uint64List optional=False
-object :obj-uint8List-wrapper
- member data: uint8List optional=False
-object :obj-user_def_cmd1-arg
- member ud1a: UserDefOne optional=False
-object :obj-user_def_cmd2-arg
- member ud1a: UserDefOne optional=False
- member ud1b: UserDefOne optional=True
alternate AltIntNum
case i: int
case n: number
@@ -73,8 +18,8 @@ alternate AltStrNum
case n: number
event EVENT_A None
event EVENT_B None
-event EVENT_C :obj-EVENT_C-arg
-event EVENT_D :obj-EVENT_D-arg
+event EVENT_C q_obj_EVENT_C-arg
+event EVENT_D q_obj_EVENT_D-arg
object Empty1
object Empty2
base Empty1
@@ -127,20 +72,20 @@ object UserDefFlatUnion2
case value2: UserDefB
object UserDefNativeListUnion
member type: UserDefNativeListUnionKind optional=False
- case integer: :obj-intList-wrapper
- case s8: :obj-int8List-wrapper
- case s16: :obj-int16List-wrapper
- case s32: :obj-int32List-wrapper
- case s64: :obj-int64List-wrapper
- case u8: :obj-uint8List-wrapper
- case u16: :obj-uint16List-wrapper
- case u32: :obj-uint32List-wrapper
- case u64: :obj-uint64List-wrapper
- case number: :obj-numberList-wrapper
- case boolean: :obj-boolList-wrapper
- case string: :obj-strList-wrapper
- case sizes: :obj-sizeList-wrapper
- case any: :obj-anyList-wrapper
+ case integer: q_obj_intList-wrapper
+ case s8: q_obj_int8List-wrapper
+ case s16: q_obj_int16List-wrapper
+ case s32: q_obj_int32List-wrapper
+ case s64: q_obj_int64List-wrapper
+ case u8: q_obj_uint8List-wrapper
+ case u16: q_obj_uint16List-wrapper
+ case u32: q_obj_uint32List-wrapper
+ case u64: q_obj_uint64List-wrapper
+ case number: q_obj_numberList-wrapper
+ case boolean: q_obj_boolList-wrapper
+ case string: q_obj_strList-wrapper
+ case sizes: q_obj_sizeList-wrapper
+ case any: q_obj_anyList-wrapper
enum UserDefNativeListUnionKind ['integer', 's8', 's16', 's32', 's64', 'u8', 'u16', 'u32', 'u64', 'number', 'boolean', 'string', 'sizes', 'any']
object UserDefOne
base UserDefZero
@@ -189,23 +134,78 @@ object __org.qemu_x-Struct2
member array: __org.qemu_x-Union1List optional=False
object __org.qemu_x-Union1
member type: __org.qemu_x-Union1Kind optional=False
- case __org.qemu_x-branch: :obj-str-wrapper
+ case __org.qemu_x-branch: q_obj_str-wrapper
enum __org.qemu_x-Union1Kind ['__org.qemu_x-branch']
object __org.qemu_x-Union2
base __org.qemu_x-Base
tag __org.qemu_x-member1
case __org.qemu_x-value: __org.qemu_x-Struct2
-command __org.qemu_x-command :obj-__org.qemu_x-command-arg -> __org.qemu_x-Union1
+command __org.qemu_x-command q_obj___org.qemu_x-command-arg -> __org.qemu_x-Union1
gen=True success_response=True
-command guest-get-time :obj-guest-get-time-arg -> int
+command guest-get-time q_obj_guest-get-time-arg -> int
gen=True success_response=True
-command guest-sync :obj-guest-sync-arg -> any
+command guest-sync q_obj_guest-sync-arg -> any
gen=True success_response=True
+object q_empty
+object q_obj_EVENT_C-arg
+ member a: int optional=True
+ member b: UserDefOne optional=True
+ member c: str optional=False
+object q_obj_EVENT_D-arg
+ member a: EventStructOne optional=False
+ member b: str optional=False
+ member c: str optional=True
+ member enum3: EnumOne optional=True
+object q_obj___org.qemu_x-command-arg
+ member a: __org.qemu_x-EnumList optional=False
+ member b: __org.qemu_x-StructList optional=False
+ member c: __org.qemu_x-Union2 optional=False
+ member d: __org.qemu_x-Alt optional=False
+object q_obj_anyList-wrapper
+ member data: anyList optional=False
+object q_obj_boolList-wrapper
+ member data: boolList optional=False
+object q_obj_guest-get-time-arg
+ member a: int optional=False
+ member b: int optional=True
+object q_obj_guest-sync-arg
+ member arg: any optional=False
+object q_obj_int16List-wrapper
+ member data: int16List optional=False
+object q_obj_int32List-wrapper
+ member data: int32List optional=False
+object q_obj_int64List-wrapper
+ member data: int64List optional=False
+object q_obj_int8List-wrapper
+ member data: int8List optional=False
+object q_obj_intList-wrapper
+ member data: intList optional=False
+object q_obj_numberList-wrapper
+ member data: numberList optional=False
+object q_obj_sizeList-wrapper
+ member data: sizeList optional=False
+object q_obj_str-wrapper
+ member data: str optional=False
+object q_obj_strList-wrapper
+ member data: strList optional=False
+object q_obj_uint16List-wrapper
+ member data: uint16List optional=False
+object q_obj_uint32List-wrapper
+ member data: uint32List optional=False
+object q_obj_uint64List-wrapper
+ member data: uint64List optional=False
+object q_obj_uint8List-wrapper
+ member data: uint8List optional=False
+object q_obj_user_def_cmd1-arg
+ member ud1a: UserDefOne optional=False
+object q_obj_user_def_cmd2-arg
+ member ud1a: UserDefOne optional=False
+ member ud1b: UserDefOne optional=True
command user_def_cmd None -> None
gen=True success_response=True
command user_def_cmd0 Empty2 -> Empty2
gen=True success_response=True
-command user_def_cmd1 :obj-user_def_cmd1-arg -> None
+command user_def_cmd1 q_obj_user_def_cmd1-arg -> None
gen=True success_response=True
-command user_def_cmd2 :obj-user_def_cmd2-arg -> UserDefTwo
+command user_def_cmd2 q_obj_user_def_cmd2-arg -> UserDefTwo
gen=True success_response=True
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v5 05/14] qapi: Emit implicit structs in generated C
2016-03-10 0:55 [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Eric Blake
` (3 preceding siblings ...)
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 04/14] qapi: Adjust names of implicit types Eric Blake
@ 2016-03-10 0:55 ` Eric Blake
2016-03-10 14:25 ` Markus Armbruster
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 06/14] qapi-event: Slightly shrink generated code Eric Blake
` (9 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2016-03-10 0:55 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
We already have several places that want to visit all the members
of an implicit object within a larger context (simple union variant,
event with anonymous data, command with anonymous arguments struct);
and will be adding another one soon (the ability to declare an
anonymous base for a flat union). Having a C struct declared for
these implicit types, along with a visit_type_FOO_members() helper
function, will make for fewer special cases in our generator.
We do not, however, need qapi_free_FOO() or visit_type_FOO()
functions for implicit types, because they should not be used
directly outside of the generated code. This is done by adding a
conditional in visit_object_type() for both qapi-types.py and
qapi-visit.py based on the object name. The comparison of
"name.startswith('q_')" is a bit hacky (it's basically duplicating
what .is_implicit() already uses), but beats changing the signature
of the visit_object_type() callback to pass a new 'implicit' flag.
The hack should be temporary: we are considering adding a future
patch that consolidates the narrow visit_object_type() and
visit_object_type_flat() [with different pieces already broken out]
into a broader visit_object_type() [where the visitor can query the
type object directly].
Also, now that we WANT to output C code for implicits, we have to
change the condition in the visit_needed() filter. We have to
special-case 'q_empty' in that filter as something that does not
get output: because it is a built-in type, it would be emitted in
multiple files (the main qapi-types.h and in qga-qapi-types.h)
causing compilation failure due to redefinition. But since it
has no members, it's easier to just avoid an attempt to visit
that particular type.
The patch relies on the changed naming of implicit types in the
previous patch. It is a bit unfortunate that the generated struct
names and visit_type_FOO_members() don't match normal naming
conventions, but it's not too bad, since they will only be used in
generated code.
The generated code grows substantially in size: the implicit
'-wrapper' types must be emitted in qapi-types.h before any union
can include an unboxed member of that type. Arguably, the '-args'
types could be emitted in a private header for just qapi-visit.c
and qmp-marshal.c, rather than polluting qapi-types.h; but adding
complexity to the generator to split the output location according
to role doesn't seem worth the maintenance costs. Another idea
for a later patch would be reworking error.h to not need to
include all of qapi-types.h.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v5: rebase onto different naming scheme, document future improvements
v4: new patch
---
scripts/qapi.py | 2 --
scripts/qapi-types.py | 14 ++++++++------
scripts/qapi-visit.py | 20 ++++++++++----------
3 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index f6701f5..96fb216 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1000,7 +1000,6 @@ class QAPISchemaObjectType(QAPISchemaType):
return self.name.startswith('q_')
def c_name(self):
- assert not self.is_implicit()
return QAPISchemaType.c_name(self)
def c_type(self):
@@ -1008,7 +1007,6 @@ class QAPISchemaObjectType(QAPISchemaType):
return c_name(self.name) + pointer_suffix
def c_unboxed_type(self):
- assert not self.is_implicit()
return c_name(self.name)
def json_type(self):
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index f194bea..107eabe 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -61,8 +61,7 @@ def gen_object(name, base, members, variants):
ret = ''
if variants:
for v in variants.variants:
- if (isinstance(v.type, QAPISchemaObjectType) and
- not v.type.is_implicit()):
+ if isinstance(v.type, QAPISchemaObjectType):
ret += gen_object(v.type.name, v.type.base,
v.type.local_members, v.type.variants)
@@ -197,9 +196,8 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
self._btin = None
def visit_needed(self, entity):
- # Visit everything except implicit objects
- return not (entity.is_implicit() and
- isinstance(entity, QAPISchemaObjectType))
+ # Visit everything except the special empty builtin
+ return entity.name != 'q_empty'
def _gen_type_cleanup(self, name):
self.decl += gen_type_cleanup_decl(name)
@@ -233,7 +231,11 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
self.decl += gen_object(name, base, members, variants)
if base:
self.decl += gen_upcast(name, base)
- self._gen_type_cleanup(name)
+ # FIXME Worth changing the visitor signature, so we could
+ # directly use rather than repeat type.is_implicit()?
+ if not name.startswith('q_'):
+ # implicit types won't be directly allocated/freed
+ self._gen_type_cleanup(name)
def visit_alternate_type(self, name, info, variants):
self._fwdecl += gen_fwd_object_or_array(name)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index a712e9a..c486aaa 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -215,13 +215,11 @@ out:
def gen_visit_object(name, base, members, variants):
- ret = gen_visit_object_members(name, base, members, variants)
-
# FIXME: if *obj is NULL on entry, and visit_start_struct() assigns to
# *obj, but then visit_type_FOO_members() fails, we should clean up *obj
# rather than leaving it non-NULL. As currently written, the caller must
# call qapi_free_FOO() to avoid a memory leak of the partial FOO.
- ret += mcgen('''
+ return mcgen('''
void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error **errp)
{
@@ -245,8 +243,6 @@ out:
''',
c_name=c_name(name))
- return ret
-
class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
def __init__(self):
@@ -269,9 +265,8 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
self._btin = None
def visit_needed(self, entity):
- # Visit everything except implicit objects
- return not (entity.is_implicit() and
- isinstance(entity, QAPISchemaObjectType))
+ # Visit everything except the special empty builtin
+ return entity.name != 'q_empty'
def visit_enum_type(self, name, info, values, prefix):
# Special case for our lone builtin enum type
@@ -297,8 +292,13 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
def visit_object_type(self, name, info, base, members, variants):
self.decl += gen_visit_members_decl(name)
- self.decl += gen_visit_decl(name)
- self.defn += gen_visit_object(name, base, members, variants)
+ self.defn += gen_visit_object_members(name, base, members, variants)
+ # FIXME Worth changing the visitor signature, so we could
+ # directly use rather than repeat type.is_implicit()?
+ if not name.startswith('q_'):
+ # only explicit types need an allocating visit
+ self.decl += gen_visit_decl(name)
+ self.defn += gen_visit_object(name, base, members, variants)
def visit_alternate_type(self, name, info, variants):
self.decl += gen_visit_decl(name)
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v5 06/14] qapi-event: Slightly shrink generated code
2016-03-10 0:55 [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Eric Blake
` (4 preceding siblings ...)
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 05/14] qapi: Emit implicit structs in generated C Eric Blake
@ 2016-03-10 0:55 ` Eric Blake
2016-03-10 18:50 ` Markus Armbruster
2016-03-16 14:41 ` Markus Armbruster
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 07/14] qapi: Utilize implicit struct visits Eric Blake
` (8 subsequent siblings)
14 siblings, 2 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-10 0:55 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Slightly rearrange the code in gen_event_send() for less generated
output, by initializing 'emit' sooner, deferring an assertion
to qdict_put_obj, and dropping a now-unused 'obj' local variable.
While at it, document a FIXME related to the potential for a
compiler naming collision - if the user ever creates a QAPI event
whose name matches 'errp' or one of our local variables (like
'emit'), we'll have to revisit how we generate functions to
avoid the problem.
|@@ -25,16 +25,13 @@ void qapi_event_send_acpi_device_ost(ACP
| {
| QDict *qmp;
| Error *err = NULL;
|- QMPEventFuncEmit emit;
|+ QMPEventFuncEmit emit = qmp_event_get_func_emit();
| QmpOutputVisitor *qov;
| Visitor *v;
|- QObject *obj;
|
|- emit = qmp_event_get_func_emit();
| if (!emit) {
| return;
| }
|-
| qmp = qmp_event_build_dict("ACPI_DEVICE_OST");
|
| qov = qmp_output_visitor_new();
|@@ -53,11 +50,7 @@ out_obj:
| if (err) {
| goto out;
| }
|-
|- obj = qmp_output_get_qobject(qov);
|- g_assert(obj);
|-
|- qdict_put_obj(qmp, "data", obj);
|+ qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
| emit(QAPI_EVENT_ACPI_DEVICE_OST, qmp, &err);
|
| out:
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v5: new patch
---
scripts/qapi-event.py | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index c03cb78..02c9556 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -29,13 +29,19 @@ def gen_event_send_decl(name, arg_type):
def gen_event_send(name, arg_type):
+ # FIXME: Our declaration of local variables (and of 'errp' in the
+ # parameter list) can collide with exploded members of the event's
+ # data type passed in as parameters. If this collision ever hits in
+ # practice, we can rename our local variables with a leading _ prefix,
+ # or split the code into a wrapper function that creates a boxed
+ # 'param' object then calls another to do the real work.
ret = mcgen('''
%(proto)s
{
QDict *qmp;
Error *err = NULL;
- QMPEventFuncEmit emit;
+ QMPEventFuncEmit emit = qmp_event_get_func_emit();
''',
proto=gen_event_send_proto(name, arg_type))
@@ -43,16 +49,13 @@ def gen_event_send(name, arg_type):
ret += mcgen('''
QmpOutputVisitor *qov;
Visitor *v;
- QObject *obj;
-
''')
ret += mcgen('''
- emit = qmp_event_get_func_emit();
+
if (!emit) {
return;
}
-
qmp = qmp_event_build_dict("%(name)s");
''',
@@ -76,11 +79,7 @@ out_obj:
if (err) {
goto out;
}
-
- obj = qmp_output_get_qobject(qov);
- g_assert(obj);
-
- qdict_put_obj(qmp, "data", obj);
+ qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
''')
ret += mcgen('''
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v5 07/14] qapi: Utilize implicit struct visits
2016-03-10 0:55 [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Eric Blake
` (5 preceding siblings ...)
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 06/14] qapi-event: Slightly shrink generated code Eric Blake
@ 2016-03-10 0:55 ` Eric Blake
2016-03-10 19:05 ` Markus Armbruster
2016-03-16 14:45 ` Markus Armbruster
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 08/14] qapi-commands: Inline single-use helpers of gen_marshal() Eric Blake
` (7 subsequent siblings)
14 siblings, 2 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-10 0:55 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Rather than generate inline per-member visits, take advantage
of the 'visit_type_FOO_members()' function for both event and
command marshalling. This is possible now that implicit
structs can be visited like any other.
Generated code shrinks accordingly; events initialize a struct
based on parameters, through a new gen_param_var() helper, like:
|@@ -338,6 +250,9 @@ void qapi_event_send_block_job_error(con
| QMPEventFuncEmit emit = qmp_event_get_func_emit();
| QmpOutputVisitor *qov;
| Visitor *v;
|+ q_obj_BLOCK_JOB_ERROR_arg param = {
|+ (char *)device, operation, action
|+ };
|
| if (!emit) {
| return;
@@ -351,19 +266,7 @@ void qapi_event_send_block_job_error(con
| if (err) {
| goto out;
| }
|- visit_type_str(v, "device", (char **)&device, &err);
|- if (err) {
|- goto out_obj;
|- }
|- visit_type_IoOperationType(v, "operation", &operation, &err);
|- if (err) {
|- goto out_obj;
|- }
|- visit_type_BlockErrorAction(v, "action", &action, &err);
|- if (err) {
|- goto out_obj;
|- }
|-out_obj:
|+ visit_type_q_obj_BLOCK_JOB_ERROR_arg_members(v, ¶m, &err);
| visit_end_struct(v, err ? NULL : &err);
Notice that the initialization of 'param' has to cast away const
(just as the old gen_visit_members() had to do): we can't change
the signature of the user function (which uses 'const char *'), but
have to assign it to a non-const QAPI object (which requires
'char *').
Likewise, command marshalling generates call arguments from a
stack-allocated struct, rather than a list of local variables:
|@@ -57,26 +57,15 @@ void qmp_marshal_add_fd(QDict *args, QOb
| QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
| QapiDeallocVisitor *qdv;
| Visitor *v;
|- bool has_fdset_id = false;
|- int64_t fdset_id = 0;
|- bool has_opaque = false;
|- char *opaque = NULL;
|-
|- v = qmp_input_get_visitor(qiv);
|- if (visit_optional(v, "fdset-id", &has_fdset_id)) {
|- visit_type_int(v, "fdset-id", &fdset_id, &err);
|- if (err) {
|- goto out;
|- }
|- }
|- if (visit_optional(v, "opaque", &has_opaque)) {
|- visit_type_str(v, "opaque", &opaque, &err);
|- if (err) {
|- goto out;
|- }
|+ q_obj_add_fd_arg qapi = {0};
|+
|+ v = qmp_input_get_visitor(qiv);
|+ visit_type_q_obj_add_fd_arg_members(v, &qapi, &err);
|+ if (err) {
|+ goto out;
| }
|
|- retval = qmp_add_fd(has_fdset_id, fdset_id, has_opaque, opaque, &err);
|+ retval = qmp_add_fd(qapi.has_fdset_id, qapi.fdset_id, qapi.has_opaque, qapi.opaque, &err);
| if (err) {
| goto out;
| }
|@@ -88,12 +77,7 @@ out:
| qmp_input_visitor_cleanup(qiv);
| qdv = qapi_dealloc_visitor_new();
| v = qapi_dealloc_get_visitor(qdv);
|- if (visit_optional(v, "fdset-id", &has_fdset_id)) {
|- visit_type_int(v, "fdset-id", &fdset_id, NULL);
|- }
|- if (visit_optional(v, "opaque", &has_opaque)) {
|- visit_type_str(v, "opaque", &opaque, NULL);
|- }
|+ visit_type_q_obj_add_fd_arg_members(v, &qapi, NULL);
| qapi_dealloc_visitor_cleanup(qdv);
| }
For the marshaller, it has the nice side effect of eliminating a
chance of collision between argument QMP names and local variables.
This patch also paves the way for some followup simplifications
in the generator, in subsequent patches.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v5: move qapi.py:gen_struct_init() to qapi-event.py:gen_param_var(),
improve commit message
v4: new patch
---
scripts/qapi-commands.py | 28 ++++++++++++----------------
scripts/qapi-event.py | 40 +++++++++++++++++++++++++++++++---------
2 files changed, 43 insertions(+), 25 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 3784f33..5ffc381 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -33,8 +33,8 @@ def gen_call(name, arg_type, ret_type):
assert not arg_type.variants
for memb in arg_type.members:
if memb.optional:
- argstr += 'has_%s, ' % c_name(memb.name)
- argstr += '%s, ' % c_name(memb.name)
+ argstr += 'qapi.has_%s, ' % c_name(memb.name)
+ argstr += 'qapi.%s, ' % c_name(memb.name)
lhs = ''
if ret_type:
@@ -71,21 +71,10 @@ def gen_marshal_vars(arg_type, ret_type):
QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
QapiDeallocVisitor *qdv;
Visitor *v;
-''')
+ %(c_name)s qapi = {0};
- for memb in arg_type.members:
- if memb.optional:
- ret += mcgen('''
- bool has_%(c_name)s = false;
''',
- c_name=c_name(memb.name))
- ret += mcgen('''
- %(c_type)s %(c_name)s = %(c_null)s;
-''',
- c_name=c_name(memb.name),
- c_type=memb.type.c_type(),
- c_null=memb.type.c_null())
- ret += '\n'
+ c_name=arg_type.c_name())
else:
ret += mcgen('''
@@ -107,17 +96,24 @@ def gen_marshal_input_visit(arg_type, dealloc=False):
qdv = qapi_dealloc_visitor_new();
v = qapi_dealloc_get_visitor(qdv);
''')
+ errp = 'NULL'
else:
ret += mcgen('''
v = qmp_input_get_visitor(qiv);
''')
+ errp = '&err'
- ret += gen_visit_members(arg_type.members, skiperr=dealloc)
+ ret += mcgen('''
+ visit_type_%(c_name)s_members(v, &qapi, %(errp)s);
+''',
+ c_name=arg_type.c_name(), errp=errp)
if dealloc:
ret += mcgen('''
qapi_dealloc_visitor_cleanup(qdv);
''')
+ else:
+ ret += gen_err_check()
return ret
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 02c9556..c125ecb 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -28,6 +28,30 @@ def gen_event_send_decl(name, arg_type):
proto=gen_event_send_proto(name, arg_type))
+# Declare and initialize an object 'qapi' using parameters from gen_params()
+def gen_param_var(typ):
+ assert not typ.variants
+ ret = mcgen('''
+ %(c_name)s param = {
+''',
+ c_name=typ.c_name())
+ sep = ' '
+ for memb in typ.members:
+ ret += sep
+ sep = ', '
+ if memb.optional:
+ ret += 'has_' + c_name(memb.name) + sep
+ if memb.type.name == 'str':
+ # Cast away const added in gen_params()
+ ret += '(char *)'
+ ret += c_name(memb.name)
+ ret += mcgen('''
+
+ };
+''')
+ return ret
+
+
def gen_event_send(name, arg_type):
# FIXME: Our declaration of local variables (and of 'errp' in the
# parameter list) can collide with exploded members of the event's
@@ -50,6 +74,7 @@ def gen_event_send(name, arg_type):
QmpOutputVisitor *qov;
Visitor *v;
''')
+ ret += gen_param_var(arg_type)
ret += mcgen('''
@@ -62,25 +87,22 @@ def gen_event_send(name, arg_type):
name=name)
if arg_type and arg_type.members:
- assert not arg_type.variants
ret += mcgen('''
qov = qmp_output_visitor_new();
v = qmp_output_get_visitor(qov);
visit_start_struct(v, "%(name)s", NULL, 0, &err);
-''',
- name=name)
- ret += gen_err_check()
- ret += gen_visit_members(arg_type.members, need_cast=True,
- label='out_obj')
- ret += mcgen('''
-out_obj:
+ if (err) {
+ goto out;
+ }
+ visit_type_%(c_name)s_members(v, ¶m, &err);
visit_end_struct(v, err ? NULL : &err);
if (err) {
goto out;
}
qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
-''')
+''',
+ name=name, c_name=arg_type.c_name())
ret += mcgen('''
emit(%(c_enum)s, qmp, &err);
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v5 08/14] qapi-commands: Inline single-use helpers of gen_marshal()
2016-03-10 0:55 [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Eric Blake
` (6 preceding siblings ...)
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 07/14] qapi: Utilize implicit struct visits Eric Blake
@ 2016-03-10 0:55 ` Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 09/14] qapi: Inline gen_visit_members() into lone caller Eric Blake
` (6 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-10 0:55 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Originally, gen_marshal_input_visit() (or gen_visitor_input_block()
before commit f1538019) was factored out to make it easy to do two
passes of a visit to each member of a (possibly-implicit) object,
without duplicating lots of code. But after recent changes, those
visits now occupy a single line of emitted code, and the helper
method has become a series of conditionals both before and after
the one important line, making it rather awkward to see at a glance
what gets emitted on the first (parsing) or second (deallocation)
pass. It's a lot easier to read the generator code if we just
inline both uses directly into gen_marshal(), without all the
conditionals.
Once we've done that, it's easy to notice that gen_marshal_vars()
is used only once, and inlining it too lets us consolidate some
mcgen() calls that used to be split across helpers.
gen_call() remains a single-use helper function, but it has
enough indentation and complexity that inlining it would hamper
legibility.
No change to generated output. The fact that the diffstat shows
a net reduction in lines is an argument in favor of this cleanup.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v5: no change
v4: new patch
---
scripts/qapi-commands.py | 106 +++++++++++++++++------------------------------
1 file changed, 39 insertions(+), 67 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 5ffc381..710e853 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -55,68 +55,6 @@ def gen_call(name, arg_type, ret_type):
return ret
-def gen_marshal_vars(arg_type, ret_type):
- ret = mcgen('''
- Error *err = NULL;
-''')
-
- if ret_type:
- ret += mcgen('''
- %(c_type)s retval;
-''',
- c_type=ret_type.c_type())
-
- if arg_type and arg_type.members:
- ret += mcgen('''
- QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
- QapiDeallocVisitor *qdv;
- Visitor *v;
- %(c_name)s qapi = {0};
-
-''',
- c_name=arg_type.c_name())
- else:
- ret += mcgen('''
-
- (void)args;
-''')
-
- return ret
-
-
-def gen_marshal_input_visit(arg_type, dealloc=False):
- ret = ''
-
- if not arg_type or not arg_type.members:
- return ret
-
- if dealloc:
- ret += mcgen('''
- qmp_input_visitor_cleanup(qiv);
- qdv = qapi_dealloc_visitor_new();
- v = qapi_dealloc_get_visitor(qdv);
-''')
- errp = 'NULL'
- else:
- ret += mcgen('''
- v = qmp_input_get_visitor(qiv);
-''')
- errp = '&err'
-
- ret += mcgen('''
- visit_type_%(c_name)s_members(v, &qapi, %(errp)s);
-''',
- c_name=arg_type.c_name(), errp=errp)
-
- if dealloc:
- ret += mcgen('''
- qapi_dealloc_visitor_cleanup(qdv);
-''')
- else:
- ret += gen_err_check()
- return ret
-
-
def gen_marshal_output(ret_type):
return mcgen('''
@@ -165,15 +103,40 @@ def gen_marshal(name, arg_type, ret_type):
%(proto)s
{
+ Error *err = NULL;
''',
proto=gen_marshal_proto(name))
- ret += gen_marshal_vars(arg_type, ret_type)
- ret += gen_marshal_input_visit(arg_type)
+ if ret_type:
+ ret += mcgen('''
+ %(c_type)s retval;
+''',
+ c_type=ret_type.c_type())
+
+ if arg_type and arg_type.members:
+ ret += mcgen('''
+ QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
+ QapiDeallocVisitor *qdv;
+ Visitor *v;
+ %(c_name)s qapi = {0};
+
+ v = qmp_input_get_visitor(qiv);
+ visit_type_%(c_name)s_members(v, &qapi, &err);
+ if (err) {
+ goto out;
+ }
+''',
+ c_name=arg_type.c_name())
+
+ else:
+ ret += mcgen('''
+
+ (void)args;
+''')
+
ret += gen_call(name, arg_type, ret_type)
- # 'goto out' produced by gen_marshal_input_visit->gen_visit_members()
- # for each arg_type member, and by gen_call() for ret_type
+ # 'goto out' produced above for arg_type, and by gen_call() for ret_type
if (arg_type and arg_type.members) or ret_type:
ret += mcgen('''
@@ -182,7 +145,16 @@ out:
ret += mcgen('''
error_propagate(errp, err);
''')
- ret += gen_marshal_input_visit(arg_type, dealloc=True)
+ if arg_type and arg_type.members:
+ ret += mcgen('''
+ qmp_input_visitor_cleanup(qiv);
+ qdv = qapi_dealloc_visitor_new();
+ v = qapi_dealloc_get_visitor(qdv);
+ visit_type_%(c_name)s_members(v, &qapi, NULL);
+ qapi_dealloc_visitor_cleanup(qdv);
+''',
+ c_name=arg_type.c_name())
+
ret += mcgen('''
}
''')
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v5 09/14] qapi: Inline gen_visit_members() into lone caller
2016-03-10 0:55 [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Eric Blake
` (7 preceding siblings ...)
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 08/14] qapi-commands: Inline single-use helpers of gen_marshal() Eric Blake
@ 2016-03-10 0:55 ` Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 10/14] qapi: Drop unused c_null() Eric Blake
` (5 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-10 0:55 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Commit 82ca8e46 noticed that we had multiple implementations of
visiting every member of a struct, and consolidated it into
gen_visit_fields() (now gen_visit_members()) with enough
parameters to cater to slight differences between the clients.
But recent exposure of implicit types has meant that we are now
down to a single use of that method, so we can clean up the
unused conditionals and just inline it into the remaining
caller: gen_visit_object_members().
Likewise, gen_err_check() no longer needs optional parameters,
as the lone use of non-defaults was via gen_visit_members().
No change to generated code.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v5: new patch
---
scripts/qapi.py | 46 ++--------------------------------------------
scripts/qapi-visit.py | 23 ++++++++++++++++++++---
2 files changed, 22 insertions(+), 47 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 96fb216..3b50e4d 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1657,54 +1657,12 @@ def gen_params(arg_type, extra):
return ret
-def gen_err_check(label='out', skiperr=False):
- if skiperr:
- return ''
+def gen_err_check():
return mcgen('''
if (err) {
- goto %(label)s;
- }
-''',
- label=label)
-
-
-def gen_visit_members(members, prefix='', need_cast=False, skiperr=False,
- label='out'):
- ret = ''
- if skiperr:
- errparg = 'NULL'
- else:
- errparg = '&err'
-
- for memb in members:
- if memb.optional:
- ret += mcgen('''
- if (visit_optional(v, "%(name)s", &%(prefix)shas_%(c_name)s)) {
-''',
- prefix=prefix, c_name=c_name(memb.name),
- name=memb.name)
- push_indent()
-
- # Ugly: sometimes we need to cast away const
- if need_cast and memb.type.name == 'str':
- cast = '(char **)'
- else:
- cast = ''
-
- ret += mcgen('''
- visit_type_%(c_type)s(v, "%(name)s", %(cast)s&%(prefix)s%(c_name)s, %(errp)s);
-''',
- c_type=memb.type.c_name(), prefix=prefix, cast=cast,
- c_name=c_name(memb.name), name=memb.name,
- errp=errparg)
- ret += gen_err_check(skiperr=skiperr, label=label)
-
- if memb.optional:
- pop_indent()
- ret += mcgen('''
+ goto out;
}
''')
- return ret
#
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index c486aaa..b425620 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -51,7 +51,24 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
c_type=base.c_name())
ret += gen_err_check()
- ret += gen_visit_members(members, prefix='obj->')
+ for memb in members:
+ if memb.optional:
+ ret += mcgen('''
+ if (visit_optional(v, "%(name)s", &obj->has_%(c_name)s)) {
+''',
+ name=memb.name, c_name=c_name(memb.name))
+ push_indent()
+ ret += mcgen('''
+ visit_type_%(c_type)s(v, "%(name)s", &obj->%(c_name)s, &err);
+''',
+ c_type=memb.type.c_name(), name=memb.name,
+ c_name=c_name(memb.name))
+ ret += gen_err_check()
+ if memb.optional:
+ pop_indent()
+ ret += mcgen('''
+ }
+''')
if variants:
ret += mcgen('''
@@ -90,8 +107,8 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
}
''')
- # 'goto out' produced for base, by gen_visit_members() for each member,
- # and if variants were present
+ # 'goto out' produced for base, for each member, and if variants were
+ # present
if base or members or variants:
ret += mcgen('''
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v5 10/14] qapi: Drop unused c_null()
2016-03-10 0:55 [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Eric Blake
` (8 preceding siblings ...)
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 09/14] qapi: Inline gen_visit_members() into lone caller Eric Blake
@ 2016-03-10 0:55 ` Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 11/14] qapi: Don't special-case simple union wrappers Eric Blake
` (4 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-10 0:55 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Now that we are always bulk-initializing a QAPI C struct to 0
(whether by g_malloc0() or by 'Type qapi = {0};'), we no longer
have any clients of c_null() in the generator for per-element
initialization. This patch is easy enough to revert if we find
a use in the future, but in the present, get rid of the dead code.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v5: new patch
---
scripts/qapi.py | 46 +++++++++++++++++-----------------------------
1 file changed, 17 insertions(+), 29 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 3b50e4d..08d63bf 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -836,9 +836,6 @@ class QAPISchemaType(QAPISchemaEntity):
def c_unboxed_type(self):
return self.c_type()
- def c_null(self):
- return 'NULL'
-
def json_type(self):
pass
@@ -854,14 +851,13 @@ class QAPISchemaType(QAPISchemaEntity):
class QAPISchemaBuiltinType(QAPISchemaType):
- def __init__(self, name, json_type, c_type, c_null):
+ def __init__(self, name, json_type, c_type):
QAPISchemaType.__init__(self, name, None)
assert not c_type or isinstance(c_type, str)
assert json_type in ('string', 'number', 'int', 'boolean', 'null',
'value')
self._json_type_name = json_type
self._c_type_name = c_type
- self._c_null_val = c_null
def c_name(self):
return self.name
@@ -874,9 +870,6 @@ class QAPISchemaBuiltinType(QAPISchemaType):
return 'const ' + self._c_type_name
return self._c_type_name
- def c_null(self):
- return self._c_null_val
-
def json_type(self):
return self._json_type_name
@@ -909,10 +902,6 @@ class QAPISchemaEnumType(QAPISchemaType):
def member_names(self):
return [v.name for v in self.values]
- def c_null(self):
- return c_enum_const(self.name, (self.member_names() + ['_MAX'])[0],
- self.prefix)
-
def json_type(self):
return 'string'
@@ -1240,9 +1229,8 @@ class QAPISchema(object):
def lookup_type(self, name):
return self.lookup_entity(name, QAPISchemaType)
- def _def_builtin_type(self, name, json_type, c_type, c_null):
- self._def_entity(QAPISchemaBuiltinType(name, json_type,
- c_type, c_null))
+ def _def_builtin_type(self, name, json_type, c_type):
+ self._def_entity(QAPISchemaBuiltinType(name, json_type, c_type))
# TODO As long as we have QAPI_TYPES_BUILTIN to share multiple
# qapi-types.h from a single .c, all arrays of builtins must be
# declared in the first file whether or not they are used. Nicer
@@ -1251,20 +1239,20 @@ class QAPISchema(object):
self._make_array_type(name, None)
def _def_predefineds(self):
- for t in [('str', 'string', 'char' + pointer_suffix, 'NULL'),
- ('number', 'number', 'double', '0'),
- ('int', 'int', 'int64_t', '0'),
- ('int8', 'int', 'int8_t', '0'),
- ('int16', 'int', 'int16_t', '0'),
- ('int32', 'int', 'int32_t', '0'),
- ('int64', 'int', 'int64_t', '0'),
- ('uint8', 'int', 'uint8_t', '0'),
- ('uint16', 'int', 'uint16_t', '0'),
- ('uint32', 'int', 'uint32_t', '0'),
- ('uint64', 'int', 'uint64_t', '0'),
- ('size', 'int', 'uint64_t', '0'),
- ('bool', 'boolean', 'bool', 'false'),
- ('any', 'value', 'QObject' + pointer_suffix, 'NULL')]:
+ for t in [('str', 'string', 'char' + pointer_suffix),
+ ('number', 'number', 'double'),
+ ('int', 'int', 'int64_t'),
+ ('int8', 'int', 'int8_t'),
+ ('int16', 'int', 'int16_t'),
+ ('int32', 'int', 'int32_t'),
+ ('int64', 'int', 'int64_t'),
+ ('uint8', 'int', 'uint8_t'),
+ ('uint16', 'int', 'uint16_t'),
+ ('uint32', 'int', 'uint32_t'),
+ ('uint64', 'int', 'uint64_t'),
+ ('size', 'int', 'uint64_t'),
+ ('bool', 'boolean', 'bool'),
+ ('any', 'value', 'QObject' + pointer_suffix)]:
self._def_builtin_type(*t)
self.the_empty_object_type = QAPISchemaObjectType('q_empty', None,
None, [], None)
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v5 11/14] qapi: Don't special-case simple union wrappers
2016-03-10 0:55 [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Eric Blake
` (9 preceding siblings ...)
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 10/14] qapi: Drop unused c_null() Eric Blake
@ 2016-03-10 0:55 ` Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 12/14] qapi: Make BlockdevOptions doc example closer to reality Eric Blake
` (3 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-10 0:55 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Fam Zheng, Eduardo Habkost,
open list:Block layer core, Michael S. Tsirkin, Jan Kiszka,
Jason Wang, armbru, Vincenzo Maffione, Luiz Capitulino,
Igor Mammedov, Gerd Hoffmann, Paolo Bonzini, Samuel Thibault,
Giuseppe Lettieri, Luigi Rizzo, Michael Roth
Simple unions were carrying a special case that hid their 'data'
QMP member from the resulting C struct, via the hack method
QAPISchemaObjectTypeVariant.simple_union_type(). But by using
the work we started by unboxing flat union and alternate
branches, coupled with the ability to visit the members of an
implicit type, we can now expose the simple union's implicit
type in qapi-types.h:
| struct q_obj_ImageInfoSpecificQCow2_wrapper {
| ImageInfoSpecificQCow2 *data;
| };
|
| struct q_obj_ImageInfoSpecificVmdk_wrapper {
| ImageInfoSpecificVmdk *data;
| };
...
| struct ImageInfoSpecific {
| ImageInfoSpecificKind type;
| union { /* union tag is @type */
| void *data;
|- ImageInfoSpecificQCow2 *qcow2;
|- ImageInfoSpecificVmdk *vmdk;
|+ q_obj_ImageInfoSpecificQCow2_wrapper qcow2;
|+ q_obj_ImageInfoSpecificVmdk_wrapper vmdk;
| } u;
| };
Doing this removes asymmetry between QAPI's QMP side and its
C side (both sides now expose 'data'), and means that the
treatment of a simple union as sugar for a flat union is now
equivalent in both languages (previously the two approaches used
a different layer of dereferencing, where the simple union could
be converted to a flat union with equivalent C layout but
different {} on the wire, or to an equivalent QMP wire form
but with different C representation). Using the implicit type
also lets us get rid of the simple_union_type() hack.
Of course, now all clients of simple unions have to adjust from
using su->u.member to using su->u.member.data; while this touches
a number of files in the tree, some earlier cleanup patches
helped minimize the change to the initialization of a temporary
variable rather than every single member access. The generated
qapi-visit.c code is also affected by the layout change:
|@@ -7393,10 +7393,10 @@ void visit_type_ImageInfoSpecific_member
| }
| switch (obj->type) {
| case IMAGE_INFO_SPECIFIC_KIND_QCOW2:
|- visit_type_ImageInfoSpecificQCow2(v, "data", &obj->u.qcow2, &err);
|+ visit_type_q_obj_ImageInfoSpecificQCow2_wrapper_members(v, &obj->u.qcow2, &err);
| break;
| case IMAGE_INFO_SPECIFIC_KIND_VMDK:
|- visit_type_ImageInfoSpecificVmdk(v, "data", &obj->u.vmdk, &err);
|+ visit_type_q_obj_ImageInfoSpecificVmdk_wrapper_members(v, &obj->u.vmdk, &err);
| break;
| default:
| abort();
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v5: rebase to q_obj naming change, hoist assertion earlier in series
v4: improve commit message, rebase onto exposed implicit types
[no v3]
v2: rebase onto s/fields/members/ change, changes in master; pick
up missing net/ files
---
scripts/qapi.py | 10 -----
scripts/qapi-types.py | 8 +---
scripts/qapi-visit.py | 22 ++---------
backends/baum.c | 2 +-
backends/msmouse.c | 2 +-
block/nbd.c | 6 +--
block/qcow2.c | 6 +--
block/vmdk.c | 8 ++--
blockdev.c | 24 ++++++------
hmp.c | 8 ++--
hw/char/escc.c | 2 +-
hw/input/hid.c | 8 ++--
hw/input/ps2.c | 6 +--
hw/input/virtio-input-hid.c | 8 ++--
hw/mem/pc-dimm.c | 2 +-
net/dump.c | 2 +-
net/hub.c | 2 +-
net/l2tpv3.c | 2 +-
net/net.c | 4 +-
net/netmap.c | 2 +-
net/slirp.c | 2 +-
net/socket.c | 2 +-
net/tap-win32.c | 2 +-
net/tap.c | 4 +-
net/vde.c | 2 +-
net/vhost-user.c | 2 +-
numa.c | 4 +-
qemu-char.c | 82 +++++++++++++++++++++--------------------
qemu-nbd.c | 6 +--
replay/replay-input.c | 44 +++++++++++-----------
spice-qemu-char.c | 14 ++++---
tests/test-io-channel-socket.c | 40 ++++++++++----------
tests/test-qmp-commands.c | 2 +-
tests/test-qmp-input-visitor.c | 25 +++++++------
tests/test-qmp-output-visitor.c | 24 ++++++------
tpm.c | 2 +-
ui/console.c | 4 +-
ui/input-keymap.c | 10 ++---
ui/input-legacy.c | 8 ++--
ui/input.c | 34 ++++++++---------
ui/vnc-auth-sasl.c | 3 +-
ui/vnc.c | 29 ++++++++-------
util/qemu-sockets.c | 35 +++++++++---------
43 files changed, 246 insertions(+), 268 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 08d63bf..d91af94 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1115,16 +1115,6 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
def __init__(self, name, typ):
QAPISchemaObjectTypeMember.__init__(self, name, typ, False)
- # This function exists to support ugly simple union special cases
- # TODO get rid of them, and drop the function
- def simple_union_type(self):
- if (self.type.is_implicit() and
- isinstance(self.type, QAPISchemaObjectType)):
- assert len(self.type.members) == 1
- assert not self.type.variants
- return self.type.members[0].type
- return None
-
class QAPISchemaAlternateType(QAPISchemaType):
def __init__(self, name, info, variants):
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 107eabe..185b33e 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -121,16 +121,10 @@ def gen_variants(variants):
c_name=c_name(variants.tag_member.name))
for var in variants.variants:
- # Ugly special case for simple union TODO get rid of it
- simple_union_type = var.simple_union_type()
- if simple_union_type:
- typ = simple_union_type.c_type()
- else:
- typ = var.type.c_unboxed_type()
ret += mcgen('''
%(c_type)s %(c_name)s;
''',
- c_type=typ,
+ c_type=var.type.c_unboxed_type(),
c_name=c_name(var.name))
ret += mcgen('''
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index b425620..6d2b38c 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -77,29 +77,15 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
c_name=c_name(variants.tag_member.name))
for var in variants.variants:
- # TODO ugly special case for simple union
- simple_union_type = var.simple_union_type()
ret += mcgen('''
case %(case)s:
+ visit_type_%(c_type)s_members(v, &obj->u.%(c_name)s, &err);
+ break;
''',
case=c_enum_const(variants.tag_member.type.name,
var.name,
- variants.tag_member.type.prefix))
- if simple_union_type:
- ret += mcgen('''
- visit_type_%(c_type)s(v, "data", &obj->u.%(c_name)s, &err);
-''',
- c_type=simple_union_type.c_name(),
- c_name=c_name(var.name))
- else:
- ret += mcgen('''
- visit_type_%(c_type)s_members(v, &obj->u.%(c_name)s, &err);
-''',
- c_type=var.type.c_name(),
- c_name=c_name(var.name))
- ret += mcgen('''
- break;
-''')
+ variants.tag_member.type.prefix),
+ c_type=var.type.c_name(), c_name=c_name(var.name))
ret += mcgen('''
default:
diff --git a/backends/baum.c b/backends/baum.c
index c11320e..eef3467 100644
--- a/backends/baum.c
+++ b/backends/baum.c
@@ -567,7 +567,7 @@ static CharDriverState *chr_baum_init(const char *id,
ChardevReturn *ret,
Error **errp)
{
- ChardevCommon *common = backend->u.braille;
+ ChardevCommon *common = backend->u.braille.data;
BaumDriverState *baum;
CharDriverState *chr;
brlapi_handle_t *handle;
diff --git a/backends/msmouse.c b/backends/msmouse.c
index 5e1833c..8dea5a1 100644
--- a/backends/msmouse.c
+++ b/backends/msmouse.c
@@ -68,7 +68,7 @@ static CharDriverState *qemu_chr_open_msmouse(const char *id,
ChardevReturn *ret,
Error **errp)
{
- ChardevCommon *common = backend->u.msmouse;
+ ChardevCommon *common = backend->u.msmouse.data;
CharDriverState *chr;
chr = qemu_chr_alloc(common, errp);
diff --git a/block/nbd.c b/block/nbd.c
index 9f333c9..836424c 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -206,13 +206,13 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, char **export,
if (qdict_haskey(options, "path")) {
UnixSocketAddress *q_unix;
saddr->type = SOCKET_ADDRESS_KIND_UNIX;
- q_unix = saddr->u.q_unix = g_new0(UnixSocketAddress, 1);
+ q_unix = saddr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
q_unix->path = g_strdup(qdict_get_str(options, "path"));
qdict_del(options, "path");
} else {
InetSocketAddress *inet;
saddr->type = SOCKET_ADDRESS_KIND_INET;
- inet = saddr->u.inet = g_new0(InetSocketAddress, 1);
+ inet = saddr->u.inet.data = g_new0(InetSocketAddress, 1);
inet->host = g_strdup(qdict_get_str(options, "host"));
if (!qdict_get_try_str(options, "port")) {
inet->port = g_strdup_printf("%d", NBD_DEFAULT_PORT);
@@ -321,7 +321,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
error_setg(errp, "TLS only supported over IP sockets");
goto error;
}
- hostname = saddr->u.inet->host;
+ hostname = saddr->u.inet.data->host;
}
/* establish TCP connection, return error if it fails
diff --git a/block/qcow2.c b/block/qcow2.c
index 8babecd..31f35f4 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2809,15 +2809,15 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs)
*spec_info = (ImageInfoSpecific){
.type = IMAGE_INFO_SPECIFIC_KIND_QCOW2,
- .u.qcow2 = g_new(ImageInfoSpecificQCow2, 1),
+ .u.qcow2.data = g_new(ImageInfoSpecificQCow2, 1),
};
if (s->qcow_version == 2) {
- *spec_info->u.qcow2 = (ImageInfoSpecificQCow2){
+ *spec_info->u.qcow2.data = (ImageInfoSpecificQCow2){
.compat = g_strdup("0.10"),
.refcount_bits = s->refcount_bits,
};
} else if (s->qcow_version == 3) {
- *spec_info->u.qcow2 = (ImageInfoSpecificQCow2){
+ *spec_info->u.qcow2.data = (ImageInfoSpecificQCow2){
.compat = g_strdup("1.1"),
.lazy_refcounts = s->compatible_features &
QCOW2_COMPAT_LAZY_REFCOUNTS,
diff --git a/block/vmdk.c b/block/vmdk.c
index a8db5d9..0fc2ce2 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2183,18 +2183,18 @@ static ImageInfoSpecific *vmdk_get_specific_info(BlockDriverState *bs)
*spec_info = (ImageInfoSpecific){
.type = IMAGE_INFO_SPECIFIC_KIND_VMDK,
- {
- .vmdk = g_new0(ImageInfoSpecificVmdk, 1),
+ .u = {
+ .vmdk.data = g_new0(ImageInfoSpecificVmdk, 1),
},
};
- *spec_info->u.vmdk = (ImageInfoSpecificVmdk) {
+ *spec_info->u.vmdk.data = (ImageInfoSpecificVmdk) {
.create_type = g_strdup(s->create_type),
.cid = s->cid,
.parent_cid = s->parent_cid,
};
- next = &spec_info->u.vmdk->extents;
+ next = &spec_info->u.vmdk.data->extents;
for (i = 0; i < s->num_extents; i++) {
*next = g_new0(ImageInfoList, 1);
(*next)->value = vmdk_get_extent_info(&s->extents[i]);
diff --git a/blockdev.c b/blockdev.c
index 0f20c65..0e08aea 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1234,7 +1234,7 @@ void qmp_blockdev_snapshot_sync(bool has_device, const char *device,
};
TransactionAction action = {
.type = TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC,
- .u.blockdev_snapshot_sync = &snapshot,
+ .u.blockdev_snapshot_sync.data = &snapshot,
};
blockdev_do_action(&action, errp);
}
@@ -1248,7 +1248,7 @@ void qmp_blockdev_snapshot(const char *node, const char *overlay,
};
TransactionAction action = {
.type = TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT,
- .u.blockdev_snapshot = &snapshot_data,
+ .u.blockdev_snapshot.data = &snapshot_data,
};
blockdev_do_action(&action, errp);
}
@@ -1263,7 +1263,7 @@ void qmp_blockdev_snapshot_internal_sync(const char *device,
};
TransactionAction action = {
.type = TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_INTERNAL_SYNC,
- .u.blockdev_snapshot_internal_sync = &snapshot,
+ .u.blockdev_snapshot_internal_sync.data = &snapshot,
};
blockdev_do_action(&action, errp);
}
@@ -1502,7 +1502,7 @@ static void internal_snapshot_prepare(BlkActionState *common,
g_assert(common->action->type ==
TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_INTERNAL_SYNC);
- internal = common->action->u.blockdev_snapshot_internal_sync;
+ internal = common->action->u.blockdev_snapshot_internal_sync.data;
state = DO_UPCAST(InternalSnapshotState, common, common);
/* 1. parse input */
@@ -1652,7 +1652,7 @@ static void external_snapshot_prepare(BlkActionState *common,
switch (action->type) {
case TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT:
{
- BlockdevSnapshot *s = action->u.blockdev_snapshot;
+ BlockdevSnapshot *s = action->u.blockdev_snapshot.data;
device = s->node;
node_name = s->node;
new_image_file = NULL;
@@ -1661,7 +1661,7 @@ static void external_snapshot_prepare(BlkActionState *common,
break;
case TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC:
{
- BlockdevSnapshotSync *s = action->u.blockdev_snapshot_sync;
+ BlockdevSnapshotSync *s = action->u.blockdev_snapshot_sync.data;
device = s->has_device ? s->device : NULL;
node_name = s->has_node_name ? s->node_name : NULL;
new_image_file = s->snapshot_file;
@@ -1710,7 +1710,7 @@ static void external_snapshot_prepare(BlkActionState *common,
}
if (action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC) {
- BlockdevSnapshotSync *s = action->u.blockdev_snapshot_sync;
+ BlockdevSnapshotSync *s = action->u.blockdev_snapshot_sync.data;
const char *format = s->has_format ? s->format : "qcow2";
enum NewImageMode mode;
const char *snapshot_node_name =
@@ -1843,7 +1843,7 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
Error *local_err = NULL;
assert(common->action->type == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
- backup = common->action->u.drive_backup;
+ backup = common->action->u.drive_backup.data;
blk = blk_by_name(backup->device);
if (!blk) {
@@ -1925,7 +1925,7 @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
Error *local_err = NULL;
assert(common->action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
- backup = common->action->u.blockdev_backup;
+ backup = common->action->u.blockdev_backup.data;
blk = blk_by_name(backup->device);
if (!blk) {
@@ -2011,7 +2011,7 @@ static void block_dirty_bitmap_add_prepare(BlkActionState *common,
return;
}
- action = common->action->u.block_dirty_bitmap_add;
+ action = common->action->u.block_dirty_bitmap_add.data;
/* AIO context taken and released within qmp_block_dirty_bitmap_add */
qmp_block_dirty_bitmap_add(action->node, action->name,
action->has_granularity, action->granularity,
@@ -2030,7 +2030,7 @@ static void block_dirty_bitmap_add_abort(BlkActionState *common)
BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
common, common);
- action = common->action->u.block_dirty_bitmap_add;
+ action = common->action->u.block_dirty_bitmap_add.data;
/* Should not be able to fail: IF the bitmap was added via .prepare(),
* then the node reference and bitmap name must have been valid.
*/
@@ -2050,7 +2050,7 @@ static void block_dirty_bitmap_clear_prepare(BlkActionState *common,
return;
}
- action = common->action->u.block_dirty_bitmap_clear;
+ action = common->action->u.block_dirty_bitmap_clear.data;
state->bitmap = block_dirty_bitmap_lookup(action->node,
action->name,
&state->bs,
diff --git a/hmp.c b/hmp.c
index 5b6084a..6ace227 100644
--- a/hmp.c
+++ b/hmp.c
@@ -857,7 +857,7 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
switch (ti->options->type) {
case TPM_TYPE_OPTIONS_KIND_PASSTHROUGH:
- tpo = ti->options->u.passthrough;
+ tpo = ti->options->u.passthrough.data;
monitor_printf(mon, "%s%s%s%s",
tpo->has_path ? ",path=" : "",
tpo->has_path ? tpo->path : "",
@@ -1753,14 +1753,14 @@ void hmp_sendkey(Monitor *mon, const QDict *qdict)
goto err_out;
}
keylist->value->type = KEY_VALUE_KIND_NUMBER;
- keylist->value->u.number = value;
+ keylist->value->u.number.data = value;
} else {
int idx = index_from_key(keys, keyname_len);
if (idx == Q_KEY_CODE__MAX) {
goto err_out;
}
keylist->value->type = KEY_VALUE_KIND_QCODE;
- keylist->value->u.qcode = idx;
+ keylist->value->u.qcode.data = idx;
}
if (!separator) {
@@ -1977,7 +1977,7 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)
if (value) {
switch (value->type) {
case MEMORY_DEVICE_INFO_KIND_DIMM:
- di = value->u.dimm;
+ di = value->u.dimm.data;
monitor_printf(mon, "Memory device [%s]: \"%s\"\n",
MemoryDeviceInfoKind_lookup[value->type],
diff --git a/hw/char/escc.c b/hw/char/escc.c
index c7a24ac..7bf09a0 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -845,7 +845,7 @@ static void sunkbd_handle_event(DeviceState *dev, QemuConsole *src,
InputKeyEvent *key;
assert(evt->type == INPUT_EVENT_KIND_KEY);
- key = evt->u.key;
+ key = evt->u.key.data;
qcode = qemu_input_key_value_to_qcode(key->key);
trace_escc_sunkbd_event_in(qcode, QKeyCode_lookup[qcode],
key->down);
diff --git a/hw/input/hid.c b/hw/input/hid.c
index 41a9387..5912677 100644
--- a/hw/input/hid.c
+++ b/hw/input/hid.c
@@ -124,7 +124,7 @@ static void hid_pointer_event(DeviceState *dev, QemuConsole *src,
switch (evt->type) {
case INPUT_EVENT_KIND_REL:
- move = evt->u.rel;
+ move = evt->u.rel.data;
if (move->axis == INPUT_AXIS_X) {
e->xdx += move->value;
} else if (move->axis == INPUT_AXIS_Y) {
@@ -133,7 +133,7 @@ static void hid_pointer_event(DeviceState *dev, QemuConsole *src,
break;
case INPUT_EVENT_KIND_ABS:
- move = evt->u.abs;
+ move = evt->u.abs.data;
if (move->axis == INPUT_AXIS_X) {
e->xdx = move->value;
} else if (move->axis == INPUT_AXIS_Y) {
@@ -142,7 +142,7 @@ static void hid_pointer_event(DeviceState *dev, QemuConsole *src,
break;
case INPUT_EVENT_KIND_BTN:
- btn = evt->u.btn;
+ btn = evt->u.btn.data;
if (btn->down) {
e->buttons_state |= bmap[btn->button];
if (btn->button == INPUT_BUTTON_WHEEL_UP) {
@@ -228,7 +228,7 @@ static void hid_keyboard_event(DeviceState *dev, QemuConsole *src,
HIDState *hs = (HIDState *)dev;
int scancodes[3], i, count;
int slot;
- InputKeyEvent *key = evt->u.key;
+ InputKeyEvent *key = evt->u.key.data;
count = qemu_input_key_value_to_scancode(key->key,
key->down,
diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index 86df1a0..58892d5 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -182,7 +182,7 @@ static void ps2_keyboard_event(DeviceState *dev, QemuConsole *src,
{
PS2KbdState *s = (PS2KbdState *)dev;
int scancodes[3], i, count;
- InputKeyEvent *key = evt->u.key;
+ InputKeyEvent *key = evt->u.key.data;
qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
count = qemu_input_key_value_to_scancode(key->key,
@@ -399,7 +399,7 @@ static void ps2_mouse_event(DeviceState *dev, QemuConsole *src,
switch (evt->type) {
case INPUT_EVENT_KIND_REL:
- move = evt->u.rel;
+ move = evt->u.rel.data;
if (move->axis == INPUT_AXIS_X) {
s->mouse_dx += move->value;
} else if (move->axis == INPUT_AXIS_Y) {
@@ -408,7 +408,7 @@ static void ps2_mouse_event(DeviceState *dev, QemuConsole *src,
break;
case INPUT_EVENT_KIND_BTN:
- btn = evt->u.btn;
+ btn = evt->u.btn.data;
if (btn->down) {
s->mouse_buttons |= bmap[btn->button];
if (btn->button == INPUT_BUTTON_WHEEL_UP) {
diff --git a/hw/input/virtio-input-hid.c b/hw/input/virtio-input-hid.c
index e5480c3..5d12157 100644
--- a/hw/input/virtio-input-hid.c
+++ b/hw/input/virtio-input-hid.c
@@ -197,7 +197,7 @@ static void virtio_input_handle_event(DeviceState *dev, QemuConsole *src,
switch (evt->type) {
case INPUT_EVENT_KIND_KEY:
- key = evt->u.key;
+ key = evt->u.key.data;
qcode = qemu_input_key_value_to_qcode(key->key);
if (qcode && keymap_qcode[qcode]) {
event.type = cpu_to_le16(EV_KEY);
@@ -212,7 +212,7 @@ static void virtio_input_handle_event(DeviceState *dev, QemuConsole *src,
}
break;
case INPUT_EVENT_KIND_BTN:
- btn = evt->u.btn;
+ btn = evt->u.btn.data;
if (keymap_button[btn->button]) {
event.type = cpu_to_le16(EV_KEY);
event.code = cpu_to_le16(keymap_button[btn->button]);
@@ -227,14 +227,14 @@ static void virtio_input_handle_event(DeviceState *dev, QemuConsole *src,
}
break;
case INPUT_EVENT_KIND_REL:
- move = evt->u.rel;
+ move = evt->u.rel.data;
event.type = cpu_to_le16(EV_REL);
event.code = cpu_to_le16(axismap_rel[move->axis]);
event.value = cpu_to_le32(move->value);
virtio_input_send(vinput, &event);
break;
case INPUT_EVENT_KIND_ABS:
- move = evt->u.abs;
+ move = evt->u.abs.data;
event.type = cpu_to_le16(EV_ABS);
event.code = cpu_to_le16(axismap_abs[move->axis]);
event.value = cpu_to_le32(move->value);
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 650f0f8..73c2426 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -180,7 +180,7 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque)
NULL);
di->memdev = object_get_canonical_path(OBJECT(dimm->hostmem));
- info->u.dimm = di;
+ info->u.dimm.data = di;
elem->value = info;
elem->next = NULL;
**prev = elem;
diff --git a/net/dump.c b/net/dump.c
index 61dec9d..94ac32a 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -189,7 +189,7 @@ int net_init_dump(const NetClientOptions *opts, const char *name,
DumpNetClient *dnc;
assert(opts->type == NET_CLIENT_OPTIONS_KIND_DUMP);
- dump = opts->u.dump;
+ dump = opts->u.dump.data;
assert(peer);
diff --git a/net/hub.c b/net/hub.c
index b6d44fd..6d90c6e 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -288,7 +288,7 @@ int net_init_hubport(const NetClientOptions *opts, const char *name,
assert(opts->type == NET_CLIENT_OPTIONS_KIND_HUBPORT);
assert(!peer);
- hubport = opts->u.hubport;
+ hubport = opts->u.hubport.data;
net_hub_add_port(hubport->hubid, name);
return 0;
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index 824161c..5c668f7 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -546,7 +546,7 @@ int net_init_l2tpv3(const NetClientOptions *opts,
s->header_mismatch = false;
assert(opts->type == NET_CLIENT_OPTIONS_KIND_L2TPV3);
- l2tpv3 = opts->u.l2tpv3;
+ l2tpv3 = opts->u.l2tpv3.data;
if (l2tpv3->has_ipv6 && l2tpv3->ipv6) {
s->ipv6 = l2tpv3->ipv6;
diff --git a/net/net.c b/net/net.c
index b0c832e..6274377 100644
--- a/net/net.c
+++ b/net/net.c
@@ -893,7 +893,7 @@ static int net_init_nic(const NetClientOptions *opts, const char *name,
const NetLegacyNicOptions *nic;
assert(opts->type == NET_CLIENT_OPTIONS_KIND_NIC);
- nic = opts->u.nic;
+ nic = opts->u.nic.data;
idx = nic_get_free_idx();
if (idx == -1 || nb_nics >= MAX_NICS) {
@@ -1025,7 +1025,7 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
/* Do not add to a vlan if it's a nic with a netdev= parameter. */
if (opts->type != NET_CLIENT_OPTIONS_KIND_NIC ||
- !opts->u.nic->has_netdev) {
+ !opts->u.nic.data->has_netdev) {
peer = net_hub_add_port(net->has_vlan ? net->vlan : 0, NULL);
}
}
diff --git a/net/netmap.c b/net/netmap.c
index 1b42728..6fa2c41 100644
--- a/net/netmap.c
+++ b/net/netmap.c
@@ -420,7 +420,7 @@ static NetClientInfo net_netmap_info = {
int net_init_netmap(const NetClientOptions *opts,
const char *name, NetClientState *peer, Error **errp)
{
- const NetdevNetmapOptions *netmap_opts = opts->u.netmap;
+ const NetdevNetmapOptions *netmap_opts = opts->u.netmap.data;
struct nm_desc *nmd;
NetClientState *nc;
Error *err = NULL;
diff --git a/net/slirp.c b/net/slirp.c
index 6b51fbc..6866913 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -747,7 +747,7 @@ int net_init_slirp(const NetClientOptions *opts, const char *name,
const char **dnssearch;
assert(opts->type == NET_CLIENT_OPTIONS_KIND_USER);
- user = opts->u.user;
+ user = opts->u.user.data;
vnet = user->has_net ? g_strdup(user->net) :
user->has_ip ? g_strdup_printf("%s/24", user->ip) :
diff --git a/net/socket.c b/net/socket.c
index e32e3cb..d7d0dec 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -707,7 +707,7 @@ int net_init_socket(const NetClientOptions *opts, const char *name,
const NetdevSocketOptions *sock;
assert(opts->type == NET_CLIENT_OPTIONS_KIND_SOCKET);
- sock = opts->u.socket;
+ sock = opts->u.socket.data;
if (sock->has_fd + sock->has_listen + sock->has_connect + sock->has_mcast +
sock->has_udp != 1) {
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 38bbac0..f1e142a 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -795,7 +795,7 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
const NetdevTapOptions *tap;
assert(opts->type == NET_CLIENT_OPTIONS_KIND_TAP);
- tap = opts->u.tap;
+ tap = opts->u.tap.data;
if (!tap->has_ifname) {
error_report("tap: no interface name");
diff --git a/net/tap.c b/net/tap.c
index cd7a7fc..8f790d1 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -565,7 +565,7 @@ int net_init_bridge(const NetClientOptions *opts, const char *name,
int fd, vnet_hdr;
assert(opts->type == NET_CLIENT_OPTIONS_KIND_BRIDGE);
- bridge = opts->u.bridge;
+ bridge = opts->u.bridge.data;
helper = bridge->has_helper ? bridge->helper : DEFAULT_BRIDGE_HELPER;
br = bridge->has_br ? bridge->br : DEFAULT_BRIDGE_INTERFACE;
@@ -728,7 +728,7 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
char ifname[128];
assert(opts->type == NET_CLIENT_OPTIONS_KIND_TAP);
- tap = opts->u.tap;
+ tap = opts->u.tap.data;
queues = tap->has_queues ? tap->queues : 1;
vhostfdname = tap->has_vhostfd ? tap->vhostfd : NULL;
diff --git a/net/vde.c b/net/vde.c
index 973faf5..9427eaa 100644
--- a/net/vde.c
+++ b/net/vde.c
@@ -116,7 +116,7 @@ int net_init_vde(const NetClientOptions *opts, const char *name,
const NetdevVdeOptions *vde;
assert(opts->type == NET_CLIENT_OPTIONS_KIND_VDE);
- vde = opts->u.vde;
+ vde = opts->u.vde.data;
/* missing optional values have been initialized to "all bits zero" */
if (net_vde_init(peer, "vde", name, vde->sock, vde->port, vde->group,
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 451dbbf..61f1cb4 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -303,7 +303,7 @@ int net_init_vhost_user(const NetClientOptions *opts, const char *name,
CharDriverState *chr;
assert(opts->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
- vhost_user_opts = opts->u.vhost_user;
+ vhost_user_opts = opts->u.vhost_user.data;
chr = net_vhost_parse_chardev(vhost_user_opts, errp);
if (!chr) {
diff --git a/numa.c b/numa.c
index da27bf8..572712c 100644
--- a/numa.c
+++ b/numa.c
@@ -228,7 +228,7 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
switch (object->type) {
case NUMA_OPTIONS_KIND_NODE:
- numa_node_parse(object->u.node, opts, &err);
+ numa_node_parse(object->u.node.data, opts, &err);
if (err) {
goto error;
}
@@ -482,7 +482,7 @@ static void numa_stat_memory_devices(uint64_t node_mem[])
if (value) {
switch (value->type) {
case MEMORY_DEVICE_INFO_KIND_DIMM:
- node_mem[value->u.dimm->node] += value->u.dimm->size;
+ node_mem[value->u.dimm.data->node] += value->u.dimm.data->size;
break;
default:
break;
diff --git a/qemu-char.c b/qemu-char.c
index e0147f3..863e9fc 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -96,16 +96,18 @@ static char *SocketAddress_to_str(const char *prefix, SocketAddress *addr,
switch (addr->type) {
case SOCKET_ADDRESS_KIND_INET:
return g_strdup_printf("%s%s:%s:%s%s", prefix,
- is_telnet ? "telnet" : "tcp", addr->u.inet->host,
- addr->u.inet->port, is_listen ? ",server" : "");
+ is_telnet ? "telnet" : "tcp",
+ addr->u.inet.data->host,
+ addr->u.inet.data->port,
+ is_listen ? ",server" : "");
break;
case SOCKET_ADDRESS_KIND_UNIX:
return g_strdup_printf("%sunix:%s%s", prefix,
- addr->u.q_unix->path,
+ addr->u.q_unix.data->path,
is_listen ? ",server" : "");
break;
case SOCKET_ADDRESS_KIND_FD:
- return g_strdup_printf("%sfd:%s%s", prefix, addr->u.fd->str,
+ return g_strdup_printf("%sfd:%s%s", prefix, addr->u.fd.data->str,
is_listen ? ",server" : "");
break;
default:
@@ -420,7 +422,7 @@ static CharDriverState *qemu_chr_open_null(const char *id,
Error **errp)
{
CharDriverState *chr;
- ChardevCommon *common = backend->u.null;
+ ChardevCommon *common = backend->u.null.data;
chr = qemu_chr_alloc(common, errp);
if (!chr) {
@@ -721,7 +723,7 @@ static CharDriverState *qemu_chr_open_mux(const char *id,
ChardevBackend *backend,
ChardevReturn *ret, Error **errp)
{
- ChardevMux *mux = backend->u.mux;
+ ChardevMux *mux = backend->u.mux.data;
CharDriverState *chr, *drv;
MuxDriver *d;
ChardevCommon *common = qapi_ChardevMux_base(mux);
@@ -1038,7 +1040,7 @@ static CharDriverState *qemu_chr_open_pipe(const char *id,
ChardevReturn *ret,
Error **errp)
{
- ChardevHostdev *opts = backend->u.pipe;
+ ChardevHostdev *opts = backend->u.pipe.data;
int fd_in, fd_out;
char *filename_in;
char *filename_out;
@@ -1120,7 +1122,7 @@ static CharDriverState *qemu_chr_open_stdio(const char *id,
ChardevReturn *ret,
Error **errp)
{
- ChardevStdio *opts = backend->u.stdio;
+ ChardevStdio *opts = backend->u.stdio.data;
CharDriverState *chr;
struct sigaction act;
ChardevCommon *common = qapi_ChardevStdio_base(opts);
@@ -1366,7 +1368,7 @@ static CharDriverState *qemu_chr_open_pty(const char *id,
PtyCharDriver *s;
int master_fd, slave_fd;
char pty_name[PATH_MAX];
- ChardevCommon *common = backend->u.pty;
+ ChardevCommon *common = backend->u.pty.data;
master_fd = qemu_openpty_raw(&slave_fd, pty_name);
if (master_fd < 0) {
@@ -2137,7 +2139,7 @@ static CharDriverState *qemu_chr_open_pipe(const char *id,
ChardevReturn *ret,
Error **errp)
{
- ChardevHostdev *opts = backend->u.pipe;
+ ChardevHostdev *opts = backend->u.pipe.data;
const char *filename = opts->device;
CharDriverState *chr;
WinCharState *s;
@@ -2183,7 +2185,7 @@ static CharDriverState *qemu_chr_open_win_con(const char *id,
ChardevReturn *ret,
Error **errp)
{
- ChardevCommon *common = backend->u.console;
+ ChardevCommon *common = backend->u.console.data;
return qemu_chr_open_win_file(GetStdHandle(STD_OUTPUT_HANDLE),
common, errp);
}
@@ -2333,7 +2335,7 @@ static CharDriverState *qemu_chr_open_stdio(const char *id,
WinStdioCharState *stdio;
DWORD dwMode;
int is_console = 0;
- ChardevCommon *common = qapi_ChardevStdio_base(backend->u.stdio);
+ ChardevCommon *common = qapi_ChardevStdio_base(backend->u.stdio.data);
chr = qemu_chr_alloc(common, errp);
if (!chr) {
@@ -2968,7 +2970,7 @@ static void tcp_chr_tls_init(CharDriverState *chr)
} else {
tioc = qio_channel_tls_new_client(
s->ioc, s->tls_creds,
- s->addr->u.inet->host,
+ s->addr->u.inet.data->host,
&err);
}
if (tioc == NULL) {
@@ -3215,7 +3217,7 @@ static CharDriverState *qemu_chr_open_ringbuf(const char *id,
ChardevReturn *ret,
Error **errp)
{
- ChardevRingbuf *opts = backend->u.ringbuf;
+ ChardevRingbuf *opts = backend->u.ringbuf.data;
ChardevCommon *common = qapi_ChardevRingbuf_base(opts);
CharDriverState *chr;
RingBufCharDriver *d;
@@ -3512,7 +3514,7 @@ static void qemu_chr_parse_file_out(QemuOpts *opts, ChardevBackend *backend,
error_setg(errp, "chardev: file: no filename given");
return;
}
- file = backend->u.file = g_new0(ChardevFile, 1);
+ file = backend->u.file.data = g_new0(ChardevFile, 1);
qemu_chr_parse_common(opts, qapi_ChardevFile_base(file));
file->out = g_strdup(path);
@@ -3525,7 +3527,7 @@ static void qemu_chr_parse_stdio(QemuOpts *opts, ChardevBackend *backend,
{
ChardevStdio *stdio;
- stdio = backend->u.stdio = g_new0(ChardevStdio, 1);
+ stdio = backend->u.stdio.data = g_new0(ChardevStdio, 1);
qemu_chr_parse_common(opts, qapi_ChardevStdio_base(stdio));
stdio->has_signal = true;
stdio->signal = qemu_opt_get_bool(opts, "signal", true);
@@ -3542,7 +3544,7 @@ static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,
error_setg(errp, "chardev: serial/tty: no device path given");
return;
}
- serial = backend->u.serial = g_new0(ChardevHostdev, 1);
+ serial = backend->u.serial.data = g_new0(ChardevHostdev, 1);
qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(serial));
serial->device = g_strdup(device);
}
@@ -3559,7 +3561,7 @@ static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend,
error_setg(errp, "chardev: parallel: no device path given");
return;
}
- parallel = backend->u.parallel = g_new0(ChardevHostdev, 1);
+ parallel = backend->u.parallel.data = g_new0(ChardevHostdev, 1);
qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(parallel));
parallel->device = g_strdup(device);
}
@@ -3575,7 +3577,7 @@ static void qemu_chr_parse_pipe(QemuOpts *opts, ChardevBackend *backend,
error_setg(errp, "chardev: pipe: no device path given");
return;
}
- dev = backend->u.pipe = g_new0(ChardevHostdev, 1);
+ dev = backend->u.pipe.data = g_new0(ChardevHostdev, 1);
qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(dev));
dev->device = g_strdup(device);
}
@@ -3586,7 +3588,7 @@ static void qemu_chr_parse_ringbuf(QemuOpts *opts, ChardevBackend *backend,
int val;
ChardevRingbuf *ringbuf;
- ringbuf = backend->u.ringbuf = g_new0(ChardevRingbuf, 1);
+ ringbuf = backend->u.ringbuf.data = g_new0(ChardevRingbuf, 1);
qemu_chr_parse_common(opts, qapi_ChardevRingbuf_base(ringbuf));
val = qemu_opt_get_size(opts, "size", 0);
@@ -3606,7 +3608,7 @@ static void qemu_chr_parse_mux(QemuOpts *opts, ChardevBackend *backend,
error_setg(errp, "chardev: mux: no chardev given");
return;
}
- mux = backend->u.mux = g_new0(ChardevMux, 1);
+ mux = backend->u.mux.data = g_new0(ChardevMux, 1);
qemu_chr_parse_common(opts, qapi_ChardevMux_base(mux));
mux->chardev = g_strdup(chardev);
}
@@ -3642,7 +3644,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
}
}
- sock = backend->u.socket = g_new0(ChardevSocket, 1);
+ sock = backend->u.socket.data = g_new0(ChardevSocket, 1);
qemu_chr_parse_common(opts, qapi_ChardevSocket_base(sock));
sock->has_nodelay = true;
@@ -3661,12 +3663,12 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
if (path) {
UnixSocketAddress *q_unix;
addr->type = SOCKET_ADDRESS_KIND_UNIX;
- q_unix = addr->u.q_unix = g_new0(UnixSocketAddress, 1);
+ q_unix = addr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
q_unix->path = g_strdup(path);
} else {
addr->type = SOCKET_ADDRESS_KIND_INET;
- addr->u.inet = g_new(InetSocketAddress, 1);
- *addr->u.inet = (InetSocketAddress) {
+ addr->u.inet.data = g_new(InetSocketAddress, 1);
+ *addr->u.inet.data = (InetSocketAddress) {
.host = g_strdup(host),
.port = g_strdup(port),
.has_to = qemu_opt_get(opts, "to"),
@@ -3709,13 +3711,13 @@ static void qemu_chr_parse_udp(QemuOpts *opts, ChardevBackend *backend,
has_local = true;
}
- udp = backend->u.udp = g_new0(ChardevUdp, 1);
+ udp = backend->u.udp.data = g_new0(ChardevUdp, 1);
qemu_chr_parse_common(opts, qapi_ChardevUdp_base(udp));
addr = g_new0(SocketAddress, 1);
addr->type = SOCKET_ADDRESS_KIND_INET;
- addr->u.inet = g_new(InetSocketAddress, 1);
- *addr->u.inet = (InetSocketAddress) {
+ addr->u.inet.data = g_new(InetSocketAddress, 1);
+ *addr->u.inet.data = (InetSocketAddress) {
.host = g_strdup(host),
.port = g_strdup(port),
.has_ipv4 = qemu_opt_get(opts, "ipv4"),
@@ -3729,8 +3731,8 @@ static void qemu_chr_parse_udp(QemuOpts *opts, ChardevBackend *backend,
udp->has_local = true;
addr = g_new0(SocketAddress, 1);
addr->type = SOCKET_ADDRESS_KIND_INET;
- addr->u.inet = g_new(InetSocketAddress, 1);
- *addr->u.inet = (InetSocketAddress) {
+ addr->u.inet.data = g_new(InetSocketAddress, 1);
+ *addr->u.inet.data = (InetSocketAddress) {
.host = g_strdup(localaddr),
.port = g_strdup(localport),
};
@@ -3817,7 +3819,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
} else {
ChardevCommon *cc = g_new0(ChardevCommon, 1);
qemu_chr_parse_common(opts, cc);
- backend->u.null = cc; /* Any ChardevCommon member would work */
+ backend->u.null.data = cc; /* Any ChardevCommon member would work */
}
ret = qmp_chardev_add(bid ? bid : id, backend, errp);
@@ -3829,9 +3831,9 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
qapi_free_ChardevBackend(backend);
qapi_free_ChardevReturn(ret);
backend = g_new0(ChardevBackend, 1);
- backend->u.mux = g_new0(ChardevMux, 1);
+ backend->u.mux.data = g_new0(ChardevMux, 1);
backend->type = CHARDEV_BACKEND_KIND_MUX;
- backend->u.mux->chardev = g_strdup(bid);
+ backend->u.mux.data->chardev = g_strdup(bid);
ret = qmp_chardev_add(id, backend, errp);
if (!ret) {
chr = qemu_chr_find(bid);
@@ -4144,7 +4146,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id,
ChardevReturn *ret,
Error **errp)
{
- ChardevFile *file = backend->u.file;
+ ChardevFile *file = backend->u.file.data;
ChardevCommon *common = qapi_ChardevFile_base(file);
HANDLE out;
@@ -4167,7 +4169,7 @@ static CharDriverState *qmp_chardev_open_serial(const char *id,
ChardevReturn *ret,
Error **errp)
{
- ChardevHostdev *serial = backend->u.serial;
+ ChardevHostdev *serial = backend->u.serial.data;
ChardevCommon *common = qapi_ChardevHostdev_base(serial);
return qemu_chr_open_win_path(serial->device, common, errp);
}
@@ -4191,7 +4193,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id,
ChardevReturn *ret,
Error **errp)
{
- ChardevFile *file = backend->u.file;
+ ChardevFile *file = backend->u.file.data;
ChardevCommon *common = qapi_ChardevFile_base(file);
int flags, in = -1, out;
@@ -4225,7 +4227,7 @@ static CharDriverState *qmp_chardev_open_serial(const char *id,
ChardevReturn *ret,
Error **errp)
{
- ChardevHostdev *serial = backend->u.serial;
+ ChardevHostdev *serial = backend->u.serial.data;
ChardevCommon *common = qapi_ChardevHostdev_base(serial);
int fd;
@@ -4244,7 +4246,7 @@ static CharDriverState *qmp_chardev_open_parallel(const char *id,
ChardevReturn *ret,
Error **errp)
{
- ChardevHostdev *parallel = backend->u.parallel;
+ ChardevHostdev *parallel = backend->u.parallel.data;
ChardevCommon *common = qapi_ChardevHostdev_base(parallel);
int fd;
@@ -4290,7 +4292,7 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
{
CharDriverState *chr;
TCPCharDriver *s;
- ChardevSocket *sock = backend->u.socket;
+ ChardevSocket *sock = backend->u.socket.data;
SocketAddress *addr = sock->addr;
bool do_nodelay = sock->has_nodelay ? sock->nodelay : false;
bool is_listen = sock->has_server ? sock->server : true;
@@ -4396,7 +4398,7 @@ static CharDriverState *qmp_chardev_open_udp(const char *id,
ChardevReturn *ret,
Error **errp)
{
- ChardevUdp *udp = backend->u.udp;
+ ChardevUdp *udp = backend->u.udp.data;
ChardevCommon *common = qapi_ChardevUdp_base(udp);
QIOChannelSocket *sioc = qio_channel_socket_new();
diff --git a/qemu-nbd.c b/qemu-nbd.c
index a5c1d95..ac0b0e3 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -377,12 +377,12 @@ static SocketAddress *nbd_build_socket_address(const char *sockpath,
saddr = g_new0(SocketAddress, 1);
if (sockpath) {
saddr->type = SOCKET_ADDRESS_KIND_UNIX;
- saddr->u.q_unix = g_new0(UnixSocketAddress, 1);
- saddr->u.q_unix->path = g_strdup(sockpath);
+ saddr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
+ saddr->u.q_unix.data->path = g_strdup(sockpath);
} else {
InetSocketAddress *inet;
saddr->type = SOCKET_ADDRESS_KIND_INET;
- inet = saddr->u.inet = g_new0(InetSocketAddress, 1);
+ inet = saddr->u.inet.data = g_new0(InetSocketAddress, 1);
inet->host = g_strdup(bindto);
if (port) {
inet->port = g_strdup(port);
diff --git a/replay/replay-input.c b/replay/replay-input.c
index c38af50..2d5d919 100644
--- a/replay/replay-input.c
+++ b/replay/replay-input.c
@@ -54,16 +54,16 @@ void replay_save_input_event(InputEvent *evt)
switch (evt->type) {
case INPUT_EVENT_KIND_KEY:
- key = evt->u.key;
+ key = evt->u.key.data;
replay_put_dword(key->key->type);
switch (key->key->type) {
case KEY_VALUE_KIND_NUMBER:
- replay_put_qword(key->key->u.number);
+ replay_put_qword(key->key->u.number.data);
replay_put_byte(key->down);
break;
case KEY_VALUE_KIND_QCODE:
- replay_put_dword(key->key->u.qcode);
+ replay_put_dword(key->key->u.qcode.data);
replay_put_byte(key->down);
break;
case KEY_VALUE_KIND__MAX:
@@ -72,17 +72,17 @@ void replay_save_input_event(InputEvent *evt)
}
break;
case INPUT_EVENT_KIND_BTN:
- btn = evt->u.btn;
+ btn = evt->u.btn.data;
replay_put_dword(btn->button);
replay_put_byte(btn->down);
break;
case INPUT_EVENT_KIND_REL:
- move = evt->u.rel;
+ move = evt->u.rel.data;
replay_put_dword(move->axis);
replay_put_qword(move->value);
break;
case INPUT_EVENT_KIND_ABS:
- move = evt->u.abs;
+ move = evt->u.abs.data;
replay_put_dword(move->axis);
replay_put_qword(move->value);
break;
@@ -105,17 +105,17 @@ InputEvent *replay_read_input_event(void)
evt.type = replay_get_dword();
switch (evt.type) {
case INPUT_EVENT_KIND_KEY:
- evt.u.key = &key;
- evt.u.key->key->type = replay_get_dword();
+ evt.u.key.data = &key;
+ evt.u.key.data->key->type = replay_get_dword();
- switch (evt.u.key->key->type) {
+ switch (evt.u.key.data->key->type) {
case KEY_VALUE_KIND_NUMBER:
- evt.u.key->key->u.number = replay_get_qword();
- evt.u.key->down = replay_get_byte();
+ evt.u.key.data->key->u.number.data = replay_get_qword();
+ evt.u.key.data->down = replay_get_byte();
break;
case KEY_VALUE_KIND_QCODE:
- evt.u.key->key->u.qcode = (QKeyCode)replay_get_dword();
- evt.u.key->down = replay_get_byte();
+ evt.u.key.data->key->u.qcode.data = (QKeyCode)replay_get_dword();
+ evt.u.key.data->down = replay_get_byte();
break;
case KEY_VALUE_KIND__MAX:
/* keep gcc happy */
@@ -123,19 +123,19 @@ InputEvent *replay_read_input_event(void)
}
break;
case INPUT_EVENT_KIND_BTN:
- evt.u.btn = &btn;
- evt.u.btn->button = (InputButton)replay_get_dword();
- evt.u.btn->down = replay_get_byte();
+ evt.u.btn.data = &btn;
+ evt.u.btn.data->button = (InputButton)replay_get_dword();
+ evt.u.btn.data->down = replay_get_byte();
break;
case INPUT_EVENT_KIND_REL:
- evt.u.rel = &rel;
- evt.u.rel->axis = (InputAxis)replay_get_dword();
- evt.u.rel->value = replay_get_qword();
+ evt.u.rel.data = &rel;
+ evt.u.rel.data->axis = (InputAxis)replay_get_dword();
+ evt.u.rel.data->value = replay_get_qword();
break;
case INPUT_EVENT_KIND_ABS:
- evt.u.abs = &abs;
- evt.u.abs->axis = (InputAxis)replay_get_dword();
- evt.u.abs->value = replay_get_qword();
+ evt.u.abs.data = &abs;
+ evt.u.abs.data->axis = (InputAxis)replay_get_dword();
+ evt.u.abs.data->value = replay_get_qword();
break;
case INPUT_EVENT_KIND__MAX:
/* keep gcc happy */
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 21885c5..351fcaa 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -305,9 +305,10 @@ static CharDriverState *qemu_chr_open_spice_vmc(const char *id,
ChardevReturn *ret,
Error **errp)
{
- const char *type = backend->u.spicevmc->type;
+ ChardevSpiceChannel *spicevmc = backend->u.spicevmc.data;
+ const char *type = spicevmc->type;
const char **psubtype = spice_server_char_device_recognized_subtypes();
- ChardevCommon *common = qapi_ChardevSpiceChannel_base(backend->u.spicevmc);
+ ChardevCommon *common = qapi_ChardevSpiceChannel_base(spicevmc);
for (; *psubtype != NULL; ++psubtype) {
if (strcmp(type, *psubtype) == 0) {
@@ -329,8 +330,9 @@ static CharDriverState *qemu_chr_open_spice_port(const char *id,
ChardevReturn *ret,
Error **errp)
{
- const char *name = backend->u.spiceport->fqdn;
- ChardevCommon *common = qapi_ChardevSpicePort_base(backend->u.spiceport);
+ ChardevSpicePort *spiceport = backend->u.spiceport.data;
+ const char *name = spiceport->fqdn;
+ ChardevCommon *common = qapi_ChardevSpicePort_base(spiceport);
CharDriverState *chr;
SpiceCharDriver *s;
@@ -372,7 +374,7 @@ static void qemu_chr_parse_spice_vmc(QemuOpts *opts, ChardevBackend *backend,
error_setg(errp, "chardev: spice channel: no name given");
return;
}
- spicevmc = backend->u.spicevmc = g_new0(ChardevSpiceChannel, 1);
+ spicevmc = backend->u.spicevmc.data = g_new0(ChardevSpiceChannel, 1);
qemu_chr_parse_common(opts, qapi_ChardevSpiceChannel_base(spicevmc));
spicevmc->type = g_strdup(name);
}
@@ -387,7 +389,7 @@ static void qemu_chr_parse_spice_port(QemuOpts *opts, ChardevBackend *backend,
error_setg(errp, "chardev: spice port: no name given");
return;
}
- spiceport = backend->u.spiceport = g_new0(ChardevSpicePort, 1);
+ spiceport = backend->u.spiceport.data = g_new0(ChardevSpicePort, 1);
qemu_chr_parse_common(opts, qapi_ChardevSpicePort_base(spiceport));
spiceport->fqdn = g_strdup(name);
}
diff --git a/tests/test-io-channel-socket.c b/tests/test-io-channel-socket.c
index 8a34056..d8c3364 100644
--- a/tests/test-io-channel-socket.c
+++ b/tests/test-io-channel-socket.c
@@ -120,8 +120,8 @@ static void test_io_channel_setup_sync(SocketAddress *listen_addr,
SocketAddress *laddr = qio_channel_socket_get_local_address(
lioc, &error_abort);
- g_free(connect_addr->u.inet->port);
- connect_addr->u.inet->port = g_strdup(laddr->u.inet->port);
+ g_free(connect_addr->u.inet.data->port);
+ connect_addr->u.inet.data->port = g_strdup(laddr->u.inet.data->port);
qapi_free_SocketAddress(laddr);
}
@@ -181,8 +181,8 @@ static void test_io_channel_setup_async(SocketAddress *listen_addr,
SocketAddress *laddr = qio_channel_socket_get_local_address(
lioc, &error_abort);
- g_free(connect_addr->u.inet->port);
- connect_addr->u.inet->port = g_strdup(laddr->u.inet->port);
+ g_free(connect_addr->u.inet.data->port);
+ connect_addr->u.inet.data->port = g_strdup(laddr->u.inet.data->port);
qapi_free_SocketAddress(laddr);
}
@@ -283,15 +283,15 @@ static void test_io_channel_ipv4(bool async)
SocketAddress *connect_addr = g_new0(SocketAddress, 1);
listen_addr->type = SOCKET_ADDRESS_KIND_INET;
- listen_addr->u.inet = g_new(InetSocketAddress, 1);
- *listen_addr->u.inet = (InetSocketAddress) {
+ listen_addr->u.inet.data = g_new(InetSocketAddress, 1);
+ *listen_addr->u.inet.data = (InetSocketAddress) {
.host = g_strdup("127.0.0.1"),
.port = NULL, /* Auto-select */
};
connect_addr->type = SOCKET_ADDRESS_KIND_INET;
- connect_addr->u.inet = g_new(InetSocketAddress, 1);
- *connect_addr->u.inet = (InetSocketAddress) {
+ connect_addr->u.inet.data = g_new(InetSocketAddress, 1);
+ *connect_addr->u.inet.data = (InetSocketAddress) {
.host = g_strdup("127.0.0.1"),
.port = NULL, /* Filled in later */
};
@@ -321,15 +321,15 @@ static void test_io_channel_ipv6(bool async)
SocketAddress *connect_addr = g_new0(SocketAddress, 1);
listen_addr->type = SOCKET_ADDRESS_KIND_INET;
- listen_addr->u.inet = g_new(InetSocketAddress, 1);
- *listen_addr->u.inet = (InetSocketAddress) {
+ listen_addr->u.inet.data = g_new(InetSocketAddress, 1);
+ *listen_addr->u.inet.data = (InetSocketAddress) {
.host = g_strdup("::1"),
.port = NULL, /* Auto-select */
};
connect_addr->type = SOCKET_ADDRESS_KIND_INET;
- connect_addr->u.inet = g_new(InetSocketAddress, 1);
- *connect_addr->u.inet = (InetSocketAddress) {
+ connect_addr->u.inet.data = g_new(InetSocketAddress, 1);
+ *connect_addr->u.inet.data = (InetSocketAddress) {
.host = g_strdup("::1"),
.port = NULL, /* Filled in later */
};
@@ -361,12 +361,12 @@ static void test_io_channel_unix(bool async)
#define TEST_SOCKET "test-io-channel-socket.sock"
listen_addr->type = SOCKET_ADDRESS_KIND_UNIX;
- listen_addr->u.q_unix = g_new0(UnixSocketAddress, 1);
- listen_addr->u.q_unix->path = g_strdup(TEST_SOCKET);
+ listen_addr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
+ listen_addr->u.q_unix.data->path = g_strdup(TEST_SOCKET);
connect_addr->type = SOCKET_ADDRESS_KIND_UNIX;
- connect_addr->u.q_unix = g_new0(UnixSocketAddress, 1);
- connect_addr->u.q_unix->path = g_strdup(TEST_SOCKET);
+ connect_addr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
+ connect_addr->u.q_unix.data->path = g_strdup(TEST_SOCKET);
test_io_channel(async, listen_addr, connect_addr, true);
@@ -410,12 +410,12 @@ static void test_io_channel_unix_fd_pass(void)
fdsend[2] = testfd;
listen_addr->type = SOCKET_ADDRESS_KIND_UNIX;
- listen_addr->u.q_unix = g_new0(UnixSocketAddress, 1);
- listen_addr->u.q_unix->path = g_strdup(TEST_SOCKET);
+ listen_addr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
+ listen_addr->u.q_unix.data->path = g_strdup(TEST_SOCKET);
connect_addr->type = SOCKET_ADDRESS_KIND_UNIX;
- connect_addr->u.q_unix = g_new0(UnixSocketAddress, 1);
- connect_addr->u.q_unix->path = g_strdup(TEST_SOCKET);
+ connect_addr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
+ connect_addr->u.q_unix.data->path = g_strdup(TEST_SOCKET);
test_io_channel_setup_sync(listen_addr, connect_addr, &src, &dst);
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 650ba46..14a9ebb 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -69,7 +69,7 @@ __org_qemu_x_Union1 *qmp___org_qemu_x_command(__org_qemu_x_EnumList *a,
__org_qemu_x_Union1 *ret = g_new0(__org_qemu_x_Union1, 1);
ret->type = ORG_QEMU_X_UNION1_KIND___ORG_QEMU_X_BRANCH;
- ret->u.__org_qemu_x_branch = strdup("blah1");
+ ret->u.__org_qemu_x_branch.data = strdup("blah1");
/* Also test that 'wchar-t' was munged to 'q_wchar_t' */
if (b && b->value && !b->value->has_q_wchar_t) {
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index b05da5b..5941e90 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -477,63 +477,64 @@ static void test_native_list_integer_helper(TestInputVisitorData *data,
switch (kind) {
case USER_DEF_NATIVE_LIST_UNION_KIND_INTEGER: {
intList *elem = NULL;
- for (i = 0, elem = cvalue->u.integer; elem; elem = elem->next, i++) {
+ for (i = 0, elem = cvalue->u.integer.data;
+ elem; elem = elem->next, i++) {
g_assert_cmpint(elem->value, ==, i);
}
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_S8: {
int8List *elem = NULL;
- for (i = 0, elem = cvalue->u.s8; elem; elem = elem->next, i++) {
+ for (i = 0, elem = cvalue->u.s8.data; elem; elem = elem->next, i++) {
g_assert_cmpint(elem->value, ==, i);
}
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_S16: {
int16List *elem = NULL;
- for (i = 0, elem = cvalue->u.s16; elem; elem = elem->next, i++) {
+ for (i = 0, elem = cvalue->u.s16.data; elem; elem = elem->next, i++) {
g_assert_cmpint(elem->value, ==, i);
}
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_S32: {
int32List *elem = NULL;
- for (i = 0, elem = cvalue->u.s32; elem; elem = elem->next, i++) {
+ for (i = 0, elem = cvalue->u.s32.data; elem; elem = elem->next, i++) {
g_assert_cmpint(elem->value, ==, i);
}
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_S64: {
int64List *elem = NULL;
- for (i = 0, elem = cvalue->u.s64; elem; elem = elem->next, i++) {
+ for (i = 0, elem = cvalue->u.s64.data; elem; elem = elem->next, i++) {
g_assert_cmpint(elem->value, ==, i);
}
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_U8: {
uint8List *elem = NULL;
- for (i = 0, elem = cvalue->u.u8; elem; elem = elem->next, i++) {
+ for (i = 0, elem = cvalue->u.u8.data; elem; elem = elem->next, i++) {
g_assert_cmpint(elem->value, ==, i);
}
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_U16: {
uint16List *elem = NULL;
- for (i = 0, elem = cvalue->u.u16; elem; elem = elem->next, i++) {
+ for (i = 0, elem = cvalue->u.u16.data; elem; elem = elem->next, i++) {
g_assert_cmpint(elem->value, ==, i);
}
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_U32: {
uint32List *elem = NULL;
- for (i = 0, elem = cvalue->u.u32; elem; elem = elem->next, i++) {
+ for (i = 0, elem = cvalue->u.u32.data; elem; elem = elem->next, i++) {
g_assert_cmpint(elem->value, ==, i);
}
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_U64: {
uint64List *elem = NULL;
- for (i = 0, elem = cvalue->u.u64; elem; elem = elem->next, i++) {
+ for (i = 0, elem = cvalue->u.u64.data; elem; elem = elem->next, i++) {
g_assert_cmpint(elem->value, ==, i);
}
break;
@@ -635,7 +636,7 @@ static void test_visitor_in_native_list_bool(TestInputVisitorData *data,
g_assert(cvalue != NULL);
g_assert_cmpint(cvalue->type, ==, USER_DEF_NATIVE_LIST_UNION_KIND_BOOLEAN);
- for (i = 0, elem = cvalue->u.boolean; elem; elem = elem->next, i++) {
+ for (i = 0, elem = cvalue->u.boolean.data; elem; elem = elem->next, i++) {
g_assert_cmpint(elem->value, ==, (i % 3 == 0) ? 1 : 0);
}
@@ -668,7 +669,7 @@ static void test_visitor_in_native_list_string(TestInputVisitorData *data,
g_assert(cvalue != NULL);
g_assert_cmpint(cvalue->type, ==, USER_DEF_NATIVE_LIST_UNION_KIND_STRING);
- for (i = 0, elem = cvalue->u.string; elem; elem = elem->next, i++) {
+ for (i = 0, elem = cvalue->u.string.data; elem; elem = elem->next, i++) {
gchar str[8];
sprintf(str, "%d", i);
g_assert_cmpstr(elem->value, ==, str);
@@ -705,7 +706,7 @@ static void test_visitor_in_native_list_number(TestInputVisitorData *data,
g_assert(cvalue != NULL);
g_assert_cmpint(cvalue->type, ==, USER_DEF_NATIVE_LIST_UNION_KIND_NUMBER);
- for (i = 0, elem = cvalue->u.number; elem; elem = elem->next, i++) {
+ for (i = 0, elem = cvalue->u.number.data; elem; elem = elem->next, i++) {
GString *double_expected = g_string_new("");
GString *double_actual = g_string_new("");
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index a7f8b45..dc35752 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -493,7 +493,7 @@ static void init_native_list(UserDefNativeListUnion *cvalue)
int i;
switch (cvalue->type) {
case USER_DEF_NATIVE_LIST_UNION_KIND_INTEGER: {
- intList **list = &cvalue->u.integer;
+ intList **list = &cvalue->u.integer.data;
for (i = 0; i < 32; i++) {
*list = g_new0(intList, 1);
(*list)->value = i;
@@ -503,7 +503,7 @@ static void init_native_list(UserDefNativeListUnion *cvalue)
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_S8: {
- int8List **list = &cvalue->u.s8;
+ int8List **list = &cvalue->u.s8.data;
for (i = 0; i < 32; i++) {
*list = g_new0(int8List, 1);
(*list)->value = i;
@@ -513,7 +513,7 @@ static void init_native_list(UserDefNativeListUnion *cvalue)
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_S16: {
- int16List **list = &cvalue->u.s16;
+ int16List **list = &cvalue->u.s16.data;
for (i = 0; i < 32; i++) {
*list = g_new0(int16List, 1);
(*list)->value = i;
@@ -523,7 +523,7 @@ static void init_native_list(UserDefNativeListUnion *cvalue)
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_S32: {
- int32List **list = &cvalue->u.s32;
+ int32List **list = &cvalue->u.s32.data;
for (i = 0; i < 32; i++) {
*list = g_new0(int32List, 1);
(*list)->value = i;
@@ -533,7 +533,7 @@ static void init_native_list(UserDefNativeListUnion *cvalue)
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_S64: {
- int64List **list = &cvalue->u.s64;
+ int64List **list = &cvalue->u.s64.data;
for (i = 0; i < 32; i++) {
*list = g_new0(int64List, 1);
(*list)->value = i;
@@ -543,7 +543,7 @@ static void init_native_list(UserDefNativeListUnion *cvalue)
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_U8: {
- uint8List **list = &cvalue->u.u8;
+ uint8List **list = &cvalue->u.u8.data;
for (i = 0; i < 32; i++) {
*list = g_new0(uint8List, 1);
(*list)->value = i;
@@ -553,7 +553,7 @@ static void init_native_list(UserDefNativeListUnion *cvalue)
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_U16: {
- uint16List **list = &cvalue->u.u16;
+ uint16List **list = &cvalue->u.u16.data;
for (i = 0; i < 32; i++) {
*list = g_new0(uint16List, 1);
(*list)->value = i;
@@ -563,7 +563,7 @@ static void init_native_list(UserDefNativeListUnion *cvalue)
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_U32: {
- uint32List **list = &cvalue->u.u32;
+ uint32List **list = &cvalue->u.u32.data;
for (i = 0; i < 32; i++) {
*list = g_new0(uint32List, 1);
(*list)->value = i;
@@ -573,7 +573,7 @@ static void init_native_list(UserDefNativeListUnion *cvalue)
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_U64: {
- uint64List **list = &cvalue->u.u64;
+ uint64List **list = &cvalue->u.u64.data;
for (i = 0; i < 32; i++) {
*list = g_new0(uint64List, 1);
(*list)->value = i;
@@ -583,7 +583,7 @@ static void init_native_list(UserDefNativeListUnion *cvalue)
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_BOOLEAN: {
- boolList **list = &cvalue->u.boolean;
+ boolList **list = &cvalue->u.boolean.data;
for (i = 0; i < 32; i++) {
*list = g_new0(boolList, 1);
(*list)->value = (i % 3 == 0);
@@ -593,7 +593,7 @@ static void init_native_list(UserDefNativeListUnion *cvalue)
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_STRING: {
- strList **list = &cvalue->u.string;
+ strList **list = &cvalue->u.string.data;
for (i = 0; i < 32; i++) {
*list = g_new0(strList, 1);
(*list)->value = g_strdup_printf("%d", i);
@@ -603,7 +603,7 @@ static void init_native_list(UserDefNativeListUnion *cvalue)
break;
}
case USER_DEF_NATIVE_LIST_UNION_KIND_NUMBER: {
- numberList **list = &cvalue->u.number;
+ numberList **list = &cvalue->u.number.data;
for (i = 0; i < 32; i++) {
*list = g_new0(numberList, 1);
(*list)->value = (double)i / 3;
diff --git a/tpm.c b/tpm.c
index 9ed708e..9a7c711 100644
--- a/tpm.c
+++ b/tpm.c
@@ -262,7 +262,7 @@ static TPMInfo *qmp_query_tpm_inst(TPMBackend *drv)
case TPM_TYPE_PASSTHROUGH:
res->options->type = TPM_TYPE_OPTIONS_KIND_PASSTHROUGH;
tpo = g_new0(TPMPassthroughOptions, 1);
- res->options->u.passthrough = tpo;
+ res->options->u.passthrough.data = tpo;
if (drv->path) {
tpo->path = g_strdup(drv->path);
tpo->has_path = true;
diff --git a/ui/console.c b/ui/console.c
index ae61382..121cc5a 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -2043,7 +2043,7 @@ static VcHandler *vc_handler = text_console_init;
static CharDriverState *vc_init(const char *id, ChardevBackend *backend,
ChardevReturn *ret, Error **errp)
{
- return vc_handler(backend->u.vc, errp);
+ return vc_handler(backend->u.vc.data, errp);
}
void register_vc_handler(VcHandler *handler)
@@ -2085,7 +2085,7 @@ static void qemu_chr_parse_vc(QemuOpts *opts, ChardevBackend *backend,
int val;
ChardevVC *vc;
- vc = backend->u.vc = g_new0(ChardevVC, 1);
+ vc = backend->u.vc.data = g_new0(ChardevVC, 1);
qemu_chr_parse_common(opts, qapi_ChardevVC_base(vc));
val = qemu_opt_get_number(opts, "width", 0);
diff --git a/ui/input-keymap.c b/ui/input-keymap.c
index fd2c09d..f1e700d 100644
--- a/ui/input-keymap.c
+++ b/ui/input-keymap.c
@@ -141,10 +141,10 @@ static int number_to_qcode[0x100];
int qemu_input_key_value_to_number(const KeyValue *value)
{
if (value->type == KEY_VALUE_KIND_QCODE) {
- return qcode_to_number[value->u.qcode];
+ return qcode_to_number[value->u.qcode.data];
} else {
assert(value->type == KEY_VALUE_KIND_NUMBER);
- return value->u.number;
+ return value->u.number.data;
}
}
@@ -168,10 +168,10 @@ int qemu_input_key_number_to_qcode(uint8_t nr)
int qemu_input_key_value_to_qcode(const KeyValue *value)
{
if (value->type == KEY_VALUE_KIND_QCODE) {
- return value->u.qcode;
+ return value->u.qcode.data;
} else {
assert(value->type == KEY_VALUE_KIND_NUMBER);
- return qemu_input_key_number_to_qcode(value->u.number);
+ return qemu_input_key_number_to_qcode(value->u.number.data);
}
}
@@ -182,7 +182,7 @@ int qemu_input_key_value_to_scancode(const KeyValue *value, bool down,
int count = 0;
if (value->type == KEY_VALUE_KIND_QCODE &&
- value->u.qcode == Q_KEY_CODE_PAUSE) {
+ value->u.qcode.data == Q_KEY_CODE_PAUSE) {
/* specific case */
int v = down ? 0 : 0x80;
codes[count++] = 0xe1;
diff --git a/ui/input-legacy.c b/ui/input-legacy.c
index f1c5cb4..7159747 100644
--- a/ui/input-legacy.c
+++ b/ui/input-legacy.c
@@ -110,7 +110,7 @@ static void legacy_kbd_event(DeviceState *dev, QemuConsole *src,
{
QEMUPutKbdEntry *entry = (QEMUPutKbdEntry *)dev;
int scancodes[3], i, count;
- InputKeyEvent *key = evt->u.key;
+ InputKeyEvent *key = evt->u.key.data;
if (!entry || !entry->put_kbd) {
return;
@@ -156,7 +156,7 @@ static void legacy_mouse_event(DeviceState *dev, QemuConsole *src,
switch (evt->type) {
case INPUT_EVENT_KIND_BTN:
- btn = evt->u.btn;
+ btn = evt->u.btn.data;
if (btn->down) {
s->buttons |= bmap[btn->button];
} else {
@@ -178,11 +178,11 @@ static void legacy_mouse_event(DeviceState *dev, QemuConsole *src,
}
break;
case INPUT_EVENT_KIND_ABS:
- move = evt->u.abs;
+ move = evt->u.abs.data;
s->axis[move->axis] = move->value;
break;
case INPUT_EVENT_KIND_REL:
- move = evt->u.rel;
+ move = evt->u.rel.data;
s->axis[move->axis] += move->value;
break;
default:
diff --git a/ui/input.c b/ui/input.c
index b035f86..ed88cda 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -166,7 +166,7 @@ void qmp_input_send_event(bool has_device, const char *device,
static void qemu_input_transform_abs_rotate(InputEvent *evt)
{
- InputMoveEvent *move = evt->u.abs;
+ InputMoveEvent *move = evt->u.abs.data;
switch (graphic_rotate) {
case 90:
if (move->axis == INPUT_AXIS_X) {
@@ -203,16 +203,16 @@ static void qemu_input_event_trace(QemuConsole *src, InputEvent *evt)
}
switch (evt->type) {
case INPUT_EVENT_KIND_KEY:
- key = evt->u.key;
+ key = evt->u.key.data;
switch (key->key->type) {
case KEY_VALUE_KIND_NUMBER:
- qcode = qemu_input_key_number_to_qcode(key->key->u.number);
+ qcode = qemu_input_key_number_to_qcode(key->key->u.number.data);
name = QKeyCode_lookup[qcode];
- trace_input_event_key_number(idx, key->key->u.number,
+ trace_input_event_key_number(idx, key->key->u.number.data,
name, key->down);
break;
case KEY_VALUE_KIND_QCODE:
- name = QKeyCode_lookup[key->key->u.qcode];
+ name = QKeyCode_lookup[key->key->u.qcode.data];
trace_input_event_key_qcode(idx, name, key->down);
break;
case KEY_VALUE_KIND__MAX:
@@ -221,17 +221,17 @@ static void qemu_input_event_trace(QemuConsole *src, InputEvent *evt)
}
break;
case INPUT_EVENT_KIND_BTN:
- btn = evt->u.btn;
+ btn = evt->u.btn.data;
name = InputButton_lookup[btn->button];
trace_input_event_btn(idx, name, btn->down);
break;
case INPUT_EVENT_KIND_REL:
- move = evt->u.rel;
+ move = evt->u.rel.data;
name = InputAxis_lookup[move->axis];
trace_input_event_rel(idx, name, move->value);
break;
case INPUT_EVENT_KIND_ABS:
- move = evt->u.abs;
+ move = evt->u.abs.data;
name = InputAxis_lookup[move->axis];
trace_input_event_abs(idx, name, move->value);
break;
@@ -366,10 +366,10 @@ void qemu_input_event_sync(void)
InputEvent *qemu_input_event_new_key(KeyValue *key, bool down)
{
InputEvent *evt = g_new0(InputEvent, 1);
- evt->u.key = g_new0(InputKeyEvent, 1);
+ evt->u.key.data = g_new0(InputKeyEvent, 1);
evt->type = INPUT_EVENT_KIND_KEY;
- evt->u.key->key = key;
- evt->u.key->down = down;
+ evt->u.key.data->key = key;
+ evt->u.key.data->down = down;
return evt;
}
@@ -391,7 +391,7 @@ void qemu_input_event_send_key_number(QemuConsole *src, int num, bool down)
{
KeyValue *key = g_new0(KeyValue, 1);
key->type = KEY_VALUE_KIND_NUMBER;
- key->u.number = num;
+ key->u.number.data = num;
qemu_input_event_send_key(src, key, down);
}
@@ -399,7 +399,7 @@ void qemu_input_event_send_key_qcode(QemuConsole *src, QKeyCode q, bool down)
{
KeyValue *key = g_new0(KeyValue, 1);
key->type = KEY_VALUE_KIND_QCODE;
- key->u.qcode = q;
+ key->u.qcode.data = q;
qemu_input_event_send_key(src, key, down);
}
@@ -416,10 +416,10 @@ void qemu_input_event_send_key_delay(uint32_t delay_ms)
InputEvent *qemu_input_event_new_btn(InputButton btn, bool down)
{
InputEvent *evt = g_new0(InputEvent, 1);
- evt->u.btn = g_new0(InputBtnEvent, 1);
+ evt->u.btn.data = g_new0(InputBtnEvent, 1);
evt->type = INPUT_EVENT_KIND_BTN;
- evt->u.btn->button = btn;
- evt->u.btn->down = down;
+ evt->u.btn.data->button = btn;
+ evt->u.btn.data->down = down;
return evt;
}
@@ -470,7 +470,7 @@ InputEvent *qemu_input_event_new_move(InputEventKind kind,
InputMoveEvent *move = g_new0(InputMoveEvent, 1);
evt->type = kind;
- evt->u.rel = move; /* evt->u.rel is the same as evt->u.abs */
+ evt->u.rel.data = move; /* evt->u.rel is the same as evt->u.abs */
move->axis = axis;
move->value = value;
return evt;
diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
index 13a59f5..56e45e3 100644
--- a/ui/vnc-auth-sasl.c
+++ b/ui/vnc-auth-sasl.c
@@ -513,7 +513,8 @@ vnc_socket_ip_addr_string(QIOChannelSocket *ioc,
error_setg(errp, "Not an inet socket type");
return NULL;
}
- ret = g_strdup_printf("%s;%s", addr->u.inet->host, addr->u.inet->port);
+ ret = g_strdup_printf("%s;%s", addr->u.inet.data->host,
+ addr->u.inet.data->port);
qapi_free_SocketAddress(addr);
return ret;
}
diff --git a/ui/vnc.c b/ui/vnc.c
index 6cd6314..376f0e0 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -112,9 +112,9 @@ static void vnc_init_basic_info(SocketAddress *addr,
{
switch (addr->type) {
case SOCKET_ADDRESS_KIND_INET:
- info->host = g_strdup(addr->u.inet->host);
- info->service = g_strdup(addr->u.inet->port);
- if (addr->u.inet->ipv6) {
+ info->host = g_strdup(addr->u.inet.data->host);
+ info->service = g_strdup(addr->u.inet.data->port);
+ if (addr->u.inet.data->ipv6) {
info->family = NETWORK_ADDRESS_FAMILY_IPV6;
} else {
info->family = NETWORK_ADDRESS_FAMILY_IPV4;
@@ -123,7 +123,7 @@ static void vnc_init_basic_info(SocketAddress *addr,
case SOCKET_ADDRESS_KIND_UNIX:
info->host = g_strdup("");
- info->service = g_strdup(addr->u.q_unix->path);
+ info->service = g_strdup(addr->u.q_unix.data->path);
info->family = NETWORK_ADDRESS_FAMILY_UNIX;
break;
@@ -385,9 +385,9 @@ VncInfo *qmp_query_vnc(Error **errp)
switch (addr->type) {
case SOCKET_ADDRESS_KIND_INET:
- info->host = g_strdup(addr->u.inet->host);
- info->service = g_strdup(addr->u.inet->port);
- if (addr->u.inet->ipv6) {
+ info->host = g_strdup(addr->u.inet.data->host);
+ info->service = g_strdup(addr->u.inet.data->port);
+ if (addr->u.inet.data->ipv6) {
info->family = NETWORK_ADDRESS_FAMILY_IPV6;
} else {
info->family = NETWORK_ADDRESS_FAMILY_IPV4;
@@ -396,7 +396,7 @@ VncInfo *qmp_query_vnc(Error **errp)
case SOCKET_ADDRESS_KIND_UNIX:
info->host = g_strdup("");
- info->service = g_strdup(addr->u.q_unix->path);
+ info->service = g_strdup(addr->u.q_unix.data->path);
info->family = NETWORK_ADDRESS_FAMILY_UNIX;
break;
@@ -3189,7 +3189,8 @@ char *vnc_display_local_addr(const char *id)
qapi_free_SocketAddress(addr);
return NULL;
}
- ret = g_strdup_printf("%s;%s", addr->u.inet->host, addr->u.inet->port);
+ ret = g_strdup_printf("%s;%s", addr->u.inet.data->host,
+ addr->u.inet.data->port);
qapi_free_SocketAddress(addr);
return ret;
@@ -3521,8 +3522,8 @@ void vnc_display_open(const char *id, Error **errp)
if (strncmp(vnc, "unix:", 5) == 0) {
saddr->type = SOCKET_ADDRESS_KIND_UNIX;
- saddr->u.q_unix = g_new0(UnixSocketAddress, 1);
- saddr->u.q_unix->path = g_strdup(vnc + 5);
+ saddr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
+ saddr->u.q_unix.data->path = g_strdup(vnc + 5);
if (vs->ws_enabled) {
error_setg(errp, "UNIX sockets not supported with websock");
@@ -3532,7 +3533,7 @@ void vnc_display_open(const char *id, Error **errp)
unsigned long long baseport;
InetSocketAddress *inet;
saddr->type = SOCKET_ADDRESS_KIND_INET;
- inet = saddr->u.inet = g_new0(InetSocketAddress, 1);
+ inet = saddr->u.inet.data = g_new0(InetSocketAddress, 1);
if (vnc[0] == '[' && vnc[hlen - 1] == ']') {
inet->host = g_strndup(vnc + 1, hlen - 2);
} else {
@@ -3561,8 +3562,8 @@ void vnc_display_open(const char *id, Error **errp)
if (vs->ws_enabled) {
wsaddr->type = SOCKET_ADDRESS_KIND_INET;
- inet = wsaddr->u.inet = g_new0(InetSocketAddress, 1);
- inet->host = g_strdup(saddr->u.inet->host);
+ inet = wsaddr->u.inet.data = g_new0(InetSocketAddress, 1);
+ inet->host = g_strdup(saddr->u.inet.data->host);
inet->port = g_strdup(websocket);
if (to) {
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index ad7c00c..ad0d21d 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -901,8 +901,8 @@ SocketAddress *socket_parse(const char *str, Error **errp)
goto fail;
} else {
addr->type = SOCKET_ADDRESS_KIND_UNIX;
- addr->u.q_unix = g_new(UnixSocketAddress, 1);
- addr->u.q_unix->path = g_strdup(str + 5);
+ addr->u.q_unix.data = g_new(UnixSocketAddress, 1);
+ addr->u.q_unix.data->path = g_strdup(str + 5);
}
} else if (strstart(str, "fd:", NULL)) {
if (str[3] == '\0') {
@@ -910,13 +910,13 @@ SocketAddress *socket_parse(const char *str, Error **errp)
goto fail;
} else {
addr->type = SOCKET_ADDRESS_KIND_FD;
- addr->u.fd = g_new(String, 1);
- addr->u.fd->str = g_strdup(str + 3);
+ addr->u.fd.data = g_new(String, 1);
+ addr->u.fd.data->str = g_strdup(str + 3);
}
} else {
addr->type = SOCKET_ADDRESS_KIND_INET;
- addr->u.inet = inet_parse(str, errp);
- if (addr->u.inet == NULL) {
+ addr->u.inet.data = inet_parse(str, errp);
+ if (addr->u.inet.data == NULL) {
goto fail;
}
}
@@ -934,15 +934,15 @@ int socket_connect(SocketAddress *addr, Error **errp,
switch (addr->type) {
case SOCKET_ADDRESS_KIND_INET:
- fd = inet_connect_saddr(addr->u.inet, errp, callback, opaque);
+ fd = inet_connect_saddr(addr->u.inet.data, errp, callback, opaque);
break;
case SOCKET_ADDRESS_KIND_UNIX:
- fd = unix_connect_saddr(addr->u.q_unix, errp, callback, opaque);
+ fd = unix_connect_saddr(addr->u.q_unix.data, errp, callback, opaque);
break;
case SOCKET_ADDRESS_KIND_FD:
- fd = monitor_get_fd(cur_mon, addr->u.fd->str, errp);
+ fd = monitor_get_fd(cur_mon, addr->u.fd.data->str, errp);
if (fd >= 0 && callback) {
qemu_set_nonblock(fd);
callback(fd, NULL, opaque);
@@ -961,15 +961,15 @@ int socket_listen(SocketAddress *addr, Error **errp)
switch (addr->type) {
case SOCKET_ADDRESS_KIND_INET:
- fd = inet_listen_saddr(addr->u.inet, 0, false, errp);
+ fd = inet_listen_saddr(addr->u.inet.data, 0, false, errp);
break;
case SOCKET_ADDRESS_KIND_UNIX:
- fd = unix_listen_saddr(addr->u.q_unix, false, errp);
+ fd = unix_listen_saddr(addr->u.q_unix.data, false, errp);
break;
case SOCKET_ADDRESS_KIND_FD:
- fd = monitor_get_fd(cur_mon, addr->u.fd->str, errp);
+ fd = monitor_get_fd(cur_mon, addr->u.fd.data->str, errp);
break;
default:
@@ -984,7 +984,8 @@ int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp)
switch (remote->type) {
case SOCKET_ADDRESS_KIND_INET:
- fd = inet_dgram_saddr(remote->u.inet, local ? local->u.inet : NULL, errp);
+ fd = inet_dgram_saddr(remote->u.inet.data,
+ local ? local->u.inet.data : NULL, errp);
break;
default:
@@ -1018,7 +1019,7 @@ socket_sockaddr_to_address_inet(struct sockaddr_storage *sa,
addr = g_new0(SocketAddress, 1);
addr->type = SOCKET_ADDRESS_KIND_INET;
- inet = addr->u.inet = g_new0(InetSocketAddress, 1);
+ inet = addr->u.inet.data = g_new0(InetSocketAddress, 1);
inet->host = g_strdup(host);
inet->port = g_strdup(serv);
if (sa->ss_family == AF_INET) {
@@ -1042,10 +1043,10 @@ socket_sockaddr_to_address_unix(struct sockaddr_storage *sa,
addr = g_new0(SocketAddress, 1);
addr->type = SOCKET_ADDRESS_KIND_UNIX;
- addr->u.q_unix = g_new0(UnixSocketAddress, 1);
+ addr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
if (su->sun_path[0]) {
- addr->u.q_unix->path = g_strndup(su->sun_path,
- sizeof(su->sun_path));
+ addr->u.q_unix.data->path = g_strndup(su->sun_path,
+ sizeof(su->sun_path));
}
return addr;
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v5 12/14] qapi: Make BlockdevOptions doc example closer to reality
2016-03-10 0:55 [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Eric Blake
` (10 preceding siblings ...)
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 11/14] qapi: Don't special-case simple union wrappers Eric Blake
@ 2016-03-10 0:55 ` Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 13/14] qapi: Allow anonymous base for flat union Eric Blake
` (2 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-10 0:55 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Although we don't want to repeat the entire BlockdevOptions
QMP command in the example, it helps if we aren't needlessly
diverging (the initial example was written before we had
committed the actual QMP interface). Use names that match what
is found in qapi/block-core.json, such as '*read-only' rather
than 'readonly', or 'BlockdevRef' rather than 'BlockRef'.
For the simple union example, invent BlockdevOptionsSimple so
that later text is unambiguous which of the two union forms is
meant (telling the user to refer back to two 'BlockdevOptions'
wasn't nice, and QMP has only the flat union form).
Also, mention that the discriminator of a flat union is
non-optional.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v5: split out from patch 8/10, sync more naming
---
docs/qapi-code-gen.txt | 74 +++++++++++++++++++++++++-------------------------
1 file changed, 37 insertions(+), 37 deletions(-)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index c648f76..12af1b8 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -297,22 +297,22 @@ be empty.
A simple union type defines a mapping from automatic discriminator
values to data types like in this example:
- { 'struct': 'FileOptions', 'data': { 'filename': 'str' } }
- { 'struct': 'Qcow2Options',
- 'data': { 'backing-file': 'str', 'lazy-refcounts': 'bool' } }
+ { 'struct': 'BlockdevOptionsFile', 'data': { 'filename': 'str' } }
+ { 'struct': 'BlockdevOptionsQcow2',
+ 'data': { 'backing': 'str', '*lazy-refcounts': 'bool' } }
- { 'union': 'BlockdevOptions',
- 'data': { 'file': 'FileOptions',
- 'qcow2': 'Qcow2Options' } }
+ { 'union': 'BlockdevOptionsSimple',
+ 'data': { 'file': 'BlockdevOptionsFile',
+ 'qcow2': 'BlockdevOptionsQcow2' } }
In the Client JSON Protocol, a simple union is represented by a
dictionary that contains the 'type' member as a discriminator, and a
'data' member that is of the specified data type corresponding to the
discriminator value, as in these examples:
- { "type": "file", "data" : { "filename": "/some/place/my-image" } }
- { "type": "qcow2", "data" : { "backing-file": "/some/place/my-image",
- "lazy-refcounts": true } }
+ { "type": "file", "data": { "filename": "/some/place/my-image" } }
+ { "type": "qcow2", "data": { "backing": "/some/place/my-image",
+ "lazy-refcounts": true } }
The generated C code uses a struct containing a union. Additionally,
an implicit C enum 'NameKind' is created, corresponding to the union
@@ -325,29 +325,29 @@ avoids nesting on the wire. All branches of the union must be
complex types, and the top-level members of the union dictionary on
the wire will be combination of members from both the base type and the
appropriate branch type (when merging two dictionaries, there must be
-no keys in common). The 'discriminator' member must be the name of an
-enum-typed member of the base struct.
+no keys in common). The 'discriminator' member must be the name of a
+non-optional enum-typed member of the base struct.
The following example enhances the above simple union example by
-adding a common member 'readonly', renaming the discriminator to
-something more applicable, and reducing the number of {} required on
-the wire:
+adding an optional common member 'read-only', renaming the
+discriminator to something more applicable than the simple union's
+default of 'type', and reducing the number of {} required on the wire:
{ 'enum': 'BlockdevDriver', 'data': [ 'file', 'qcow2' ] }
- { 'struct': 'BlockdevCommonOptions',
- 'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } }
+ { 'struct': 'BlockdevOptionsBase',
+ 'data': { 'driver': 'BlockdevDriver', '*read-only': 'bool' } }
{ 'union': 'BlockdevOptions',
- 'base': 'BlockdevCommonOptions',
+ 'base': 'BlockdevOptionsBase',
'discriminator': 'driver',
- 'data': { 'file': 'FileOptions',
- 'qcow2': 'Qcow2Options' } }
+ 'data': { 'file': 'BlockdevOptionsFile',
+ 'qcow2': 'BlockdevOptionsQcow2' } }
Resulting in these JSON objects:
- { "driver": "file", "readonly": true,
+ { "driver": "file", "read-only": true,
"filename": "/some/place/my-image" }
- { "driver": "qcow2", "readonly": false,
- "backing-file": "/some/place/my-image", "lazy-refcounts": true }
+ { "driver": "qcow2", "read-only": false,
+ "backing": "/some/place/my-image", "lazy-refcounts": true }
Notice that in a flat union, the discriminator name is controlled by
the user, but because it must map to a base member with enum type, the
@@ -382,7 +382,7 @@ data types (string, integer, number, or object, but currently not
array) on the wire. The definition is similar to a simple union type,
where each branch of the union names a QAPI type. For example:
- { 'alternate': 'BlockRef',
+ { 'alternate': 'BlockdevRef',
'data': { 'definition': 'BlockdevOptions',
'reference': 'str' } }
@@ -403,7 +403,7 @@ following example objects:
{ "file": "my_existing_block_device_id" }
{ "file": { "driver": "file",
- "readonly": false,
+ "read-only": false,
"filename": "/tmp/mydisk.qcow2" } }
@@ -637,11 +637,11 @@ Union types
{ "name": "BlockdevOptions", "meta-type": "object",
"members": [
{ "name": "driver", "type": "BlockdevDriver" },
- { "name": "readonly", "type": "bool"} ],
+ { "name": "read-only", "type": "bool", "default": null } ],
"tag": "driver",
"variants": [
- { "case": "file", "type": "FileOptions" },
- { "case": "qcow2", "type": "Qcow2Options" } ] }
+ { "case": "file", "type": "BlockdevOptionsFile" },
+ { "case": "qcow2", "type": "BlockdevOptionsQcow2" } ] }
Note that base types are "flattened": its members are included in the
"members" array.
@@ -652,20 +652,20 @@ discriminator (called "type" on the wire, see section Union types).
A simple union implicitly defines an object type for each of its
variants.
-Example: the SchemaInfo for simple union BlockdevOptions from section
+Example: the SchemaInfo for simple union BlockdevOptionsSimple from section
Union types
- { "name": "BlockdevOptions", "meta-type": "object",
+ { "name": "BlockdevOptionsSimple", "meta-type": "object",
"members": [
- { "name": "type", "type": "BlockdevOptionsKind" } ],
+ { "name": "type", "type": "BlockdevOptionsSimpleKind" } ],
"tag": "type",
"variants": [
- { "case": "file", "type": "q_obj-FileOptions-wrapper" },
- { "case": "qcow2", "type": "q_obj-Qcow2Options-wrapper" } ] }
+ { "case": "file", "type": "q_obj-BlockdevOptionsFile-wrapper" },
+ { "case": "qcow2", "type": "q_obj-BlockdevOptionsQcow2-wrapper" } ] }
- Enumeration type "BlockdevOptionsKind" and the object types
- "q_obj-FileOptions-wrapper", "q_obj-Qcow2Options-wrapper" are
- implicitly defined.
+ Enumeration type "BlockdevOptionsSimpleKind" and the object types
+ "q_obj-BlockdevOptionsFile-wrapper", "q_obj-BlockdevOptionsQcow2-wrapper"
+ are implicitly defined.
The SchemaInfo for an alternate type has meta-type "alternate", and
variant member "members". "members" is a JSON array. Each element is
@@ -673,9 +673,9 @@ a JSON object with member "type", which names a type. Values of the
alternate type conform to exactly one of its member types. There is
no guarantee on the order in which "members" will be listed.
-Example: the SchemaInfo for BlockRef from section Alternate types
+Example: the SchemaInfo for BlockdevRef from section Alternate types
- { "name": "BlockRef", "meta-type": "alternate",
+ { "name": "BlockdevRef", "meta-type": "alternate",
"members": [
{ "type": "BlockdevOptions" },
{ "type": "str" } ] }
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v5 13/14] qapi: Allow anonymous base for flat union
2016-03-10 0:55 [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Eric Blake
` (11 preceding siblings ...)
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 12/14] qapi: Make BlockdevOptions doc example closer to reality Eric Blake
@ 2016-03-10 0:55 ` Eric Blake
2016-03-10 20:22 ` Markus Armbruster
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 14/14] qapi: Use anonymous bases in QMP flat unions Eric Blake
2016-03-10 20:36 ` [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Markus Armbruster
14 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2016-03-10 0:55 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Rather than requiring all flat unions to explicitly create
a separate base struct, we can allow the qapi schema to specify
the common members via an inline dictionary. This is similar to
how commands can specify an inline anonymous type for its 'data'.
We already have several struct types that only exist to serve as
a single flat union's base; the next commit will clean them up
(in particular, the doc change to the BlockdevOptions example in
this patch will be reflected to QMP in the next).
Now that anonymous bases are legal, we need to rework the
flat-union-bad-base negative test (as previously written, it
forms what is now valid QAPI; tweak it to now provide coverage
of a new error message path), and add a positive test in
qapi-schema-test to use an anonymous base (making the integer
argument optional, for even more coverage).
Note that this patch only allows anonymous bases for flat unions;
simple unions are already enough syntactic sugar that we do not
want to burden them further. Meanwhile, while it would be easy
to also allow an anonymous base for structs, that would be quite
redundant, as the members can be put right into the struct
instead.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v5: rebase to earlier changes, improve commit message, split off
unrelated doc tweaks
v4: rebase to use implicit type, improve commit message, allow optional
members in anonymous type
[no v3]
v2: rebase onto s/fields/members/ change
v1: rebase and rework to use gen_visit_fields_call(), test new error
Previously posted as part of qapi cleanup subset F:
v6: avoid redundant error check in gen_visit_union(), rebase to
earlier gen_err_check() improvements
---
scripts/qapi.py | 12 ++++++++++--
scripts/qapi-types.py | 10 ++++++----
docs/qapi-code-gen.txt | 26 +++++++++++++-------------
tests/qapi-schema/flat-union-bad-base.err | 2 +-
tests/qapi-schema/flat-union-bad-base.json | 5 ++---
tests/qapi-schema/qapi-schema-test.json | 6 +-----
tests/qapi-schema/qapi-schema-test.out | 10 +++++-----
7 files changed, 38 insertions(+), 33 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index d91af94..a38ef52 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -327,6 +327,8 @@ class QAPISchemaParser(object):
def find_base_members(base):
+ if isinstance(base, dict):
+ return base
base_struct_define = find_struct(base)
if not base_struct_define:
return None
@@ -561,9 +563,10 @@ def check_union(expr, expr_info):
# Else, it's a flat union.
else:
- # The object must have a string member 'base'.
+ # The object must have a string or dictionary 'base'.
check_type(expr_info, "'base' for union '%s'" % name,
- base, allow_metas=['struct'])
+ base, allow_dict=True, allow_optional=True,
+ allow_metas=['struct'])
if not base:
raise QAPIExprError(expr_info,
"Flat union '%s' must have a base"
@@ -1039,6 +1042,8 @@ class QAPISchemaMember(object):
owner = owner[6:]
if owner.endswith('-arg'):
return '(parameter of %s)' % owner[:-4]
+ elif owner.endswith('-base'):
+ return '(base of %s)' % owner[:-5]
else:
assert owner.endswith('-wrapper')
# Unreachable and not implemented
@@ -1325,6 +1330,9 @@ class QAPISchema(object):
base = expr.get('base')
tag_name = expr.get('discriminator')
tag_member = None
+ if isinstance(base, dict):
+ base = (self._make_implicit_object_type(
+ name, info, 'base', self._make_members(base, info)))
if tag_name:
variants = [self._make_variant(key, value)
for (key, value) in data.iteritems()]
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 185b33e..cafedc4 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -72,12 +72,14 @@ struct %(c_name)s {
c_name=c_name(name))
if base:
- ret += mcgen('''
+ if not base.is_implicit():
+ ret += mcgen('''
/* Members inherited from %(c_name)s: */
''',
- c_name=base.c_name())
+ c_name=base.c_name())
ret += gen_struct_members(base.members)
- ret += mcgen('''
+ if not base.is_implicit():
+ ret += mcgen('''
/* Own members: */
''')
ret += gen_struct_members(members)
@@ -223,7 +225,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
def visit_object_type(self, name, info, base, members, variants):
self._fwdecl += gen_fwd_object_or_array(name)
self.decl += gen_object(name, base, members, variants)
- if base:
+ if base and not base.is_implicit():
self.decl += gen_upcast(name, base)
# FIXME Worth changing the visitor signature, so we could
# directly use rather than repeat type.is_implicit()?
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 12af1b8..0e4baff 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -284,7 +284,7 @@ better than open-coding the member to be type 'str'.
=== Union types ===
Usage: { 'union': STRING, 'data': DICT }
-or: { 'union': STRING, 'data': DICT, 'base': STRUCT-NAME,
+or: { 'union': STRING, 'data': DICT, 'base': STRUCT-NAME-OR-DICT,
'discriminator': ENUM-MEMBER-OF-BASE }
Union types are used to let the user choose between several different
@@ -320,13 +320,16 @@ an implicit C enum 'NameKind' is created, corresponding to the union
the union can be named 'max', as this would collide with the implicit
enum. The value for each branch can be of any type.
-A flat union definition specifies a struct as its base, and
-avoids nesting on the wire. All branches of the union must be
-complex types, and the top-level members of the union dictionary on
-the wire will be combination of members from both the base type and the
-appropriate branch type (when merging two dictionaries, there must be
-no keys in common). The 'discriminator' member must be the name of a
-non-optional enum-typed member of the base struct.
+A flat union definition avoids nesting on the wire, and specifies a
+set of common members that occur in all variants of the union. The
+'base' key must specifiy either a type name (the type must be a
+struct, not a union), or a dictionary representing an anonymous type.
+All branches of the union must be complex types, and the top-level
+members of the union dictionary on the wire will be combination of
+members from both the base type and the appropriate branch type (when
+merging two dictionaries, there must be no keys in common). The
+'discriminator' member must be the name of a non-optional enum-typed
+member of the base struct.
The following example enhances the above simple union example by
adding an optional common member 'read-only', renaming the
@@ -334,10 +337,8 @@ discriminator to something more applicable than the simple union's
default of 'type', and reducing the number of {} required on the wire:
{ 'enum': 'BlockdevDriver', 'data': [ 'file', 'qcow2' ] }
- { 'struct': 'BlockdevOptionsBase',
- 'data': { 'driver': 'BlockdevDriver', '*read-only': 'bool' } }
{ 'union': 'BlockdevOptions',
- 'base': 'BlockdevOptionsBase',
+ 'base': { 'driver': 'BlockdevDriver', '*read-only': 'bool' },
'discriminator': 'driver',
'data': { 'file': 'BlockdevOptionsFile',
'qcow2': 'BlockdevOptionsQcow2' } }
@@ -366,10 +367,9 @@ union has a struct with a single member named 'data'. That is,
is identical on the wire to:
{ 'enum': 'Enum', 'data': ['one', 'two'] }
- { 'struct': 'Base', 'data': { 'type': 'Enum' } }
{ 'struct': 'Branch1', 'data': { 'data': 'str' } }
{ 'struct': 'Branch2', 'data': { 'data': 'int' } }
- { 'union': 'Flat', 'base': 'Base', 'discriminator': 'type',
+ { 'union': 'Flat': 'base': { 'type': 'Enum' }, 'discriminator': 'type',
'data': { 'one': 'Branch1', 'two': 'Branch2' } }
diff --git a/tests/qapi-schema/flat-union-bad-base.err b/tests/qapi-schema/flat-union-bad-base.err
index 79b8a71..bee24a2 100644
--- a/tests/qapi-schema/flat-union-bad-base.err
+++ b/tests/qapi-schema/flat-union-bad-base.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-bad-base.json:9: 'base' for union 'TestUnion' should be a type name
+tests/qapi-schema/flat-union-bad-base.json:8: 'string' (member of TestTypeA) collides with 'string' (base of TestUnion)
diff --git a/tests/qapi-schema/flat-union-bad-base.json b/tests/qapi-schema/flat-union-bad-base.json
index e2e622b..74dd421 100644
--- a/tests/qapi-schema/flat-union-bad-base.json
+++ b/tests/qapi-schema/flat-union-bad-base.json
@@ -1,5 +1,4 @@
-# we require the base to be an existing struct
-# TODO: should we allow an anonymous inline base type?
+# we allow anonymous base, but enforce no duplicate keys
{ 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] }
{ 'struct': 'TestTypeA',
@@ -7,7 +6,7 @@
{ 'struct': 'TestTypeB',
'data': { 'integer': 'int' } }
{ 'union': 'TestUnion',
- 'base': { 'enum1': 'TestEnum', 'kind': 'str' },
+ 'base': { 'enum1': 'TestEnum', 'string': 'str' },
'discriminator': 'enum1',
'data': { 'value1': 'TestTypeA',
'value2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index e722748..f571e1b 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -75,14 +75,10 @@
'base': 'UserDefZero',
'data': { 'string': 'str', 'enum1': 'EnumOne' } }
-{ 'struct': 'UserDefUnionBase2',
- 'base': 'UserDefZero',
- 'data': { 'string': 'str', 'enum1': 'QEnumTwo' } }
-
# this variant of UserDefFlatUnion defaults to a union that uses members with
# allocated types to test corner cases in the cleanup/dealloc visitor
{ 'union': 'UserDefFlatUnion2',
- 'base': 'UserDefUnionBase2',
+ 'base': { '*integer': 'int', 'string': 'str', 'enum1': 'QEnumTwo' },
'discriminator': 'enum1',
'data': { 'value1' : 'UserDefC', # intentional forward reference
'value2' : 'UserDefB' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index d49fe1d..19cd214 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -66,7 +66,7 @@ object UserDefFlatUnion
case value2: UserDefB
case value3: UserDefB
object UserDefFlatUnion2
- base UserDefUnionBase2
+ base q_obj_UserDefFlatUnion2-base
tag enum1
case value1: UserDefC
case value2: UserDefB
@@ -111,10 +111,6 @@ object UserDefUnionBase
base UserDefZero
member string: str optional=False
member enum1: EnumOne optional=False
-object UserDefUnionBase2
- base UserDefZero
- member string: str optional=False
- member enum1: QEnumTwo optional=False
object UserDefZero
member integer: int optional=False
object WrapAlternate
@@ -156,6 +152,10 @@ object q_obj_EVENT_D-arg
member b: str optional=False
member c: str optional=True
member enum3: EnumOne optional=True
+object q_obj_UserDefFlatUnion2-base
+ member integer: int optional=True
+ member string: str optional=False
+ member enum1: QEnumTwo optional=False
object q_obj___org.qemu_x-command-arg
member a: __org.qemu_x-EnumList optional=False
member b: __org.qemu_x-StructList optional=False
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v5 14/14] qapi: Use anonymous bases in QMP flat unions
2016-03-10 0:55 [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Eric Blake
` (12 preceding siblings ...)
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 13/14] qapi: Allow anonymous base for flat union Eric Blake
@ 2016-03-10 0:55 ` Eric Blake
2016-03-10 20:36 ` [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Markus Armbruster
14 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-10 0:55 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Now that the generator supports it, we might as well use an
anonymous base rather than breaking out a single-use Base
structure, for all three of our current QMP flat unions.
Oddly enough, this change does not affect the resulting
introspection output (because we already inline the members of
a base type into an object, and had no independent use of the
base type reachable from a command).
The case_whitelist now has to list the name of an implicit
type; which is not too bad (consider it a feature if it makes
it harder for developers to make the whitelist grow :)
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v5: rebase to implicit type name change
v4: retitle, merge, add BlockdevOptions
[no v3]
v2: no change
v1: no change
Previously posted as part of qapi cleanup subset F:
v6: new patch
---
scripts/qapi.py | 2 +-
qapi-schema.json | 20 ++++-------
qapi/block-core.json | 98 ++++++++++++++++++++++++----------------------------
qapi/introspect.json | 12 +------
4 files changed, 53 insertions(+), 79 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index a38ef52..b13ae47 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -63,12 +63,12 @@ returns_whitelist = [
case_whitelist = [
# From QMP:
'ACPISlotType', # DIMM, visible through query-acpi-ospm-status
- 'CpuInfoBase', # CPU, visible through query-cpu
'CpuInfoMIPS', # PC, visible through query-cpu
'CpuInfoTricore', # PC, visible through query-cpu
'QapiErrorClass', # all members, visible through errors
'UuidInfo', # UUID, visible through query-uuid
'X86CPURegister32', # all members, visible indirectly through qom-get
+ 'q_obj_CpuInfo-base', # CPU, visible through query-cpu
]
enum_types = []
diff --git a/qapi-schema.json b/qapi-schema.json
index 362c9d8..bd03bcc 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -753,9 +753,9 @@
'data': ['x86', 'sparc', 'ppc', 'mips', 'tricore', 'other' ] }
##
-# @CpuInfoBase:
+# @CpuInfo:
#
-# Common information about a virtual CPU
+# Information about a virtual CPU
#
# @CPU: the index of the virtual CPU
#
@@ -776,18 +776,10 @@
# Notes: @halted is a transient state that changes frequently. By the time the
# data is sent to the client, the guest may no longer be halted.
##
-{ 'struct': 'CpuInfoBase',
- 'data': {'CPU': 'int', 'current': 'bool', 'halted': 'bool',
- 'qom_path': 'str', 'thread_id': 'int', 'arch': 'CpuInfoArch' } }
-
-##
-# @CpuInfo:
-#
-# Information about a virtual CPU
-#
-# Since: 0.14.0
-##
-{ 'union': 'CpuInfo', 'base': 'CpuInfoBase', 'discriminator': 'arch',
+{ 'union': 'CpuInfo',
+ 'base': {'CPU': 'int', 'current': 'bool', 'halted': 'bool',
+ 'qom_path': 'str', 'thread_id': 'int', 'arch': 'CpuInfoArch' },
+ 'discriminator': 'arch',
'data': { 'x86': 'CpuInfoX86',
'sparc': 'CpuInfoSPARC',
'ppc': 'CpuInfoPPC',
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9bf1b22..b1cf77d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1644,57 +1644,6 @@
'vmdk', 'vpc', 'vvfat' ] }
##
-# @BlockdevOptionsBase
-#
-# Options that are available for all block devices, independent of the block
-# driver.
-#
-# @driver: block driver name
-# @id: #optional id by which the new block device can be referred to.
-# This option is only allowed on the top level of blockdev-add.
-# A BlockBackend will be created by blockdev-add if and only if
-# this option is given.
-# @node-name: #optional the name of a block driver state node (Since 2.0).
-# This option is required on the top level of blockdev-add if
-# the @id option is not given there.
-# @discard: #optional discard-related options (default: ignore)
-# @cache: #optional cache-related options
-# @aio: #optional AIO backend (default: threads)
-# @rerror: #optional how to handle read errors on the device
-# (default: report)
-# @werror: #optional how to handle write errors on the device
-# (default: enospc)
-# @read-only: #optional whether the block device should be read-only
-# (default: false)
-# @stats-account-invalid: #optional whether to include invalid
-# operations when computing last access statistics
-# (default: true) (Since 2.5)
-# @stats-account-failed: #optional whether to include failed
-# operations when computing latency and last
-# access statistics (default: true) (Since 2.5)
-# @stats-intervals: #optional list of intervals for collecting I/O
-# statistics, in seconds (default: none) (Since 2.5)
-# @detect-zeroes: #optional detect and optimize zero writes (Since 2.1)
-# (default: off)
-#
-# Since: 1.7
-##
-{ 'struct': 'BlockdevOptionsBase',
- 'data': { 'driver': 'BlockdevDriver',
- '*id': 'str',
- '*node-name': 'str',
- '*discard': 'BlockdevDiscardOptions',
- '*cache': 'BlockdevCacheOptions',
- '*aio': 'BlockdevAioOptions',
- '*rerror': 'BlockdevOnError',
- '*werror': 'BlockdevOnError',
- '*read-only': 'bool',
- '*stats-account-invalid': 'bool',
- '*stats-account-failed': 'bool',
- '*stats-intervals': ['int'],
- '*detect-zeroes': 'BlockdevDetectZeroesOptions' } }
-
-##
# @BlockdevOptionsFile
#
# Driver specific block device options for the file backend and similar
@@ -2070,12 +2019,55 @@
##
# @BlockdevOptions
#
-# Options for creating a block device.
+# Options for creating a block device. Many options are available for all
+# block devices, independent of the block driver:
+#
+# @driver: block driver name
+# @id: #optional id by which the new block device can be referred to.
+# This option is only allowed on the top level of blockdev-add.
+# A BlockBackend will be created by blockdev-add if and only if
+# this option is given.
+# @node-name: #optional the name of a block driver state node (Since 2.0).
+# This option is required on the top level of blockdev-add if
+# the @id option is not given there.
+# @discard: #optional discard-related options (default: ignore)
+# @cache: #optional cache-related options
+# @aio: #optional AIO backend (default: threads)
+# @rerror: #optional how to handle read errors on the device
+# (default: report)
+# @werror: #optional how to handle write errors on the device
+# (default: enospc)
+# @read-only: #optional whether the block device should be read-only
+# (default: false)
+# @stats-account-invalid: #optional whether to include invalid
+# operations when computing last access statistics
+# (default: true) (Since 2.5)
+# @stats-account-failed: #optional whether to include failed
+# operations when computing latency and last
+# access statistics (default: true) (Since 2.5)
+# @stats-intervals: #optional list of intervals for collecting I/O
+# statistics, in seconds (default: none) (Since 2.5)
+# @detect-zeroes: #optional detect and optimize zero writes (Since 2.1)
+# (default: off)
+#
+# Remaining options are determined by the block driver.
#
# Since: 1.7
##
{ 'union': 'BlockdevOptions',
- 'base': 'BlockdevOptionsBase',
+ 'base': { 'driver': 'BlockdevDriver',
+ '*id': 'str',
+ '*node-name': 'str',
+ '*discard': 'BlockdevDiscardOptions',
+ '*cache': 'BlockdevCacheOptions',
+ '*aio': 'BlockdevAioOptions',
+ '*rerror': 'BlockdevOnError',
+ '*werror': 'BlockdevOnError',
+ '*read-only': 'bool',
+ '*stats-account-invalid': 'bool',
+ '*stats-account-failed': 'bool',
+ '*stats-intervals': ['int'],
+ '*detect-zeroes': 'BlockdevDetectZeroesOptions' },
'discriminator': 'driver',
'data': {
'archipelago':'BlockdevOptionsArchipelago',
diff --git a/qapi/introspect.json b/qapi/introspect.json
index 9e9369e..3fd81fb 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -75,16 +75,6 @@
'command', 'event' ] }
##
-# @SchemaInfoBase
-#
-# Members common to any @SchemaInfo.
-#
-# Since: 2.5
-##
-{ 'struct': 'SchemaInfoBase',
- 'data': { 'name': 'str', 'meta-type': 'SchemaMetaType' } }
-
-##
# @SchemaInfo
#
# @name: the entity's name, inherited from @base.
@@ -103,7 +93,7 @@
# Since: 2.5
##
{ 'union': 'SchemaInfo',
- 'base': 'SchemaInfoBase',
+ 'base': { 'name': 'str', 'meta-type': 'SchemaMetaType' },
'discriminator': 'meta-type',
'data': {
'builtin': 'SchemaInfoBuiltin',
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v5 04/14] qapi: Adjust names of implicit types
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 04/14] qapi: Adjust names of implicit types Eric Blake
@ 2016-03-10 13:39 ` Markus Armbruster
2016-03-10 16:11 ` Eric Blake
0 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2016-03-10 13:39 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> The original choice of ':obj-' as the prefix for implicit types
> made it obvious that we weren't going to clash with any user-defined
> names. But now we want to create structs for implicit types.
Why? I know, but the commit message should still give a hint. Perhaps:
"to get rid of special cases in the generators"?
> We
> could transliterate ':' to '_', except that C99 says that a leading
> underscore and lower-case letter should be used only for file scope
> identifiers, while we would be exposing it in qapi-types.h. So it's
Misunderstanding! When the standard says "identifiers that X are
reserved for Y use", it reserves these identifiers for itself and the
implementation. You shouldn't use them for Y then.
Suggest to simply quote the standard instead of interpreting it:
... except that C99 mandates that "identifiers that begin with an
underscore are always reserved for use as identifiers with file scope in
both the ordinary and tag name spaces"
> time to change our naming convention; we can instead use the 'q_'
> prefix that we reserved for ourselves back in commit 9fb081e0. As
> long as we don't declare 'empty' or 'obj' ticklish, it shouldn't
> clash with c_name() prepending 'q_' to the user's ticklish names.
Do we really want to rename :empty? We're not going to generate C for
it, are we?
> Signed-off-by: Eric Blake <eblake@redhat.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v5 05/14] qapi: Emit implicit structs in generated C
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 05/14] qapi: Emit implicit structs in generated C Eric Blake
@ 2016-03-10 14:25 ` Markus Armbruster
0 siblings, 0 replies; 32+ messages in thread
From: Markus Armbruster @ 2016-03-10 14:25 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> We already have several places that want to visit all the members
> of an implicit object within a larger context (simple union variant,
> event with anonymous data, command with anonymous arguments struct);
> and will be adding another one soon (the ability to declare an
> anonymous base for a flat union). Having a C struct declared for
> these implicit types, along with a visit_type_FOO_members() helper
> function, will make for fewer special cases in our generator.
>
> We do not, however, need qapi_free_FOO() or visit_type_FOO()
> functions for implicit types, because they should not be used
> directly outside of the generated code. This is done by adding a
> conditional in visit_object_type() for both qapi-types.py and
> qapi-visit.py based on the object name. The comparison of
> "name.startswith('q_')" is a bit hacky (it's basically duplicating
> what .is_implicit() already uses), but beats changing the signature
> of the visit_object_type() callback to pass a new 'implicit' flag.
> The hack should be temporary: we are considering adding a future
> patch that consolidates the narrow visit_object_type() and
> visit_object_type_flat() [with different pieces already broken out]
> into a broader visit_object_type() [where the visitor can query the
> type object directly].
>
> Also, now that we WANT to output C code for implicits, we have to
> change the condition in the visit_needed() filter. We have to
> special-case 'q_empty' in that filter as something that does not
> get output: because it is a built-in type, it would be emitted in
> multiple files (the main qapi-types.h and in qga-qapi-types.h)
> causing compilation failure due to redefinition. But since it
> has no members, it's easier to just avoid an attempt to visit
> that particular type.
Not just easier. ':empty' is a genuinely internal thing so far (it
exists only to make qapi-introspect.py simpler), and generating C for it
would be weird.
> The patch relies on the changed naming of implicit types in the
> previous patch. It is a bit unfortunate that the generated struct
> names and visit_type_FOO_members() don't match normal naming
> conventions, but it's not too bad, since they will only be used in
> generated code.
>
> The generated code grows substantially in size: the implicit
> '-wrapper' types must be emitted in qapi-types.h before any union
> can include an unboxed member of that type. Arguably, the '-args'
> types could be emitted in a private header for just qapi-visit.c
> and qmp-marshal.c, rather than polluting qapi-types.h; but adding
> complexity to the generator to split the output location according
> to role doesn't seem worth the maintenance costs. Another idea
> for a later patch would be reworking error.h to not need to
> include all of qapi-types.h.
I got that in my tree. If it goes in before this patch, the last
sentence should be dropped.
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v5: rebase onto different naming scheme, document future improvements
> v4: new patch
> ---
> scripts/qapi.py | 2 --
> scripts/qapi-types.py | 14 ++++++++------
> scripts/qapi-visit.py | 20 ++++++++++----------
> 3 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index f6701f5..96fb216 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -1000,7 +1000,6 @@ class QAPISchemaObjectType(QAPISchemaType):
> return self.name.startswith('q_')
>
> def c_name(self):
> - assert not self.is_implicit()
> return QAPISchemaType.c_name(self)
>
> def c_type(self):
> @@ -1008,7 +1007,6 @@ class QAPISchemaObjectType(QAPISchemaType):
> return c_name(self.name) + pointer_suffix
>
> def c_unboxed_type(self):
> - assert not self.is_implicit()
> return c_name(self.name)
>
> def json_type(self):
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index f194bea..107eabe 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -61,8 +61,7 @@ def gen_object(name, base, members, variants):
> ret = ''
> if variants:
> for v in variants.variants:
> - if (isinstance(v.type, QAPISchemaObjectType) and
> - not v.type.is_implicit()):
> + if isinstance(v.type, QAPISchemaObjectType):
> ret += gen_object(v.type.name, v.type.base,
> v.type.local_members, v.type.variants)
>
> @@ -197,9 +196,8 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
> self._btin = None
>
> def visit_needed(self, entity):
> - # Visit everything except implicit objects
> - return not (entity.is_implicit() and
> - isinstance(entity, QAPISchemaObjectType))
> + # Visit everything except the special empty builtin
> + return entity.name != 'q_empty'
>
> def _gen_type_cleanup(self, name):
> self.decl += gen_type_cleanup_decl(name)
Before: we visit all types except for implicit object types in "def
before use" order. visit_needed() filters out implicit object types.
gen_object() sorts by recursing, but filters out non-object types and
implicit object types. Note the shared "filters out implicit object
types" part.
After: we visit all types escept for the empty type in "def before use"
order. visit_needed() filters out the empty type. gen_object filters
out non-object types. It lost its shared part. If we somehow recurse
into the empty type, we visit it. Oops.
Actually, we overcomplicated things. visit_needed() exists so we can
filter out different kinds of entities in one condition. Really
convenient in qapi-introspect.py.
But if you want to filter out just one kind (here: object types), like
we do here, it's simpler to filter in that kind's visit_ method (here:
visit_object_type(). More efficient, too, but that doesn't matter.
Since we recurse, we need to additionally filter there. An easy way to
do that would be to start with objects_seen set to
set(schema.the_empty_object_type) instead of set().
> @@ -233,7 +231,11 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
> self.decl += gen_object(name, base, members, variants)
> if base:
> self.decl += gen_upcast(name, base)
> - self._gen_type_cleanup(name)
> + # FIXME Worth changing the visitor signature, so we could
> + # directly use rather than repeat type.is_implicit()?
Nitpick: this is a TODO, because it's not about fixing something that's
broken.
> + if not name.startswith('q_'):
> + # implicit types won't be directly allocated/freed
> + self._gen_type_cleanup(name)
>
> def visit_alternate_type(self, name, info, variants):
> self._fwdecl += gen_fwd_object_or_array(name)
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index a712e9a..c486aaa 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -215,13 +215,11 @@ out:
>
>
> def gen_visit_object(name, base, members, variants):
> - ret = gen_visit_object_members(name, base, members, variants)
> -
See visit_object_type() below.
> # FIXME: if *obj is NULL on entry, and visit_start_struct() assigns to
> # *obj, but then visit_type_FOO_members() fails, we should clean up *obj
> # rather than leaving it non-NULL. As currently written, the caller must
> # call qapi_free_FOO() to avoid a memory leak of the partial FOO.
> - ret += mcgen('''
> + return mcgen('''
>
> void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error **errp)
> {
> @@ -245,8 +243,6 @@ out:
> ''',
> c_name=c_name(name))
>
> - return ret
> -
>
> class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
> def __init__(self):
> @@ -269,9 +265,8 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
> self._btin = None
>
> def visit_needed(self, entity):
> - # Visit everything except implicit objects
> - return not (entity.is_implicit() and
> - isinstance(entity, QAPISchemaObjectType))
> + # Visit everything except the special empty builtin
> + return entity.name != 'q_empty'
As in qapi-types.py, visit_needed() isn't necessary even before the
patch.
> def visit_enum_type(self, name, info, values, prefix):
> # Special case for our lone builtin enum type
> @@ -297,8 +292,13 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
>
> def visit_object_type(self, name, info, base, members, variants):
> self.decl += gen_visit_members_decl(name)
> - self.decl += gen_visit_decl(name)
> - self.defn += gen_visit_object(name, base, members, variants)
> + self.defn += gen_visit_object_members(name, base, members, variants)
Moved from gen_visit_object(), the rest can be called conditionally:
> + # FIXME Worth changing the visitor signature, so we could
> + # directly use rather than repeat type.is_implicit()?
> + if not name.startswith('q_'):
> + # only explicit types need an allocating visit
> + self.decl += gen_visit_decl(name)
> + self.defn += gen_visit_object(name, base, members, variants)
>
> def visit_alternate_type(self, name, info, variants):
> self.decl += gen_visit_decl(name)
Okay.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v5 04/14] qapi: Adjust names of implicit types
2016-03-10 13:39 ` Markus Armbruster
@ 2016-03-10 16:11 ` Eric Blake
2016-03-11 7:48 ` Markus Armbruster
0 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2016-03-10 16:11 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 1904 bytes --]
On 03/10/2016 06:39 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
>> The original choice of ':obj-' as the prefix for implicit types
>> made it obvious that we weren't going to clash with any user-defined
>> names. But now we want to create structs for implicit types.
>
> Why? I know, but the commit message should still give a hint. Perhaps:
> "to get rid of special cases in the generators"?
>
>> We
>> could transliterate ':' to '_', except that C99 says that a leading
>> underscore and lower-case letter should be used only for file scope
>> identifiers, while we would be exposing it in qapi-types.h. So it's
>
> Misunderstanding! When the standard says "identifiers that X are
> reserved for Y use", it reserves these identifiers for itself and the
> implementation. You shouldn't use them for Y then.
>
> Suggest to simply quote the standard instead of interpreting it:
> ... except that C99 mandates that "identifiers that begin with an
> underscore are always reserved for use as identifiers with file scope in
> both the ordinary and tag name spaces"
Both those changes sound fine.
>
>> time to change our naming convention; we can instead use the 'q_'
>> prefix that we reserved for ourselves back in commit 9fb081e0. As
>> long as we don't declare 'empty' or 'obj' ticklish, it shouldn't
>> clash with c_name() prepending 'q_' to the user's ticklish names.
>
> Do we really want to rename :empty? We're not going to generate C for
> it, are we?
No, but it was easier to implement .is_implicit() as
"name.startswith('q_')" than as "name == ':empty' or
name.startswith('q_obj')". I can stick with :empty if you want a
respin, though.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v5 06/14] qapi-event: Slightly shrink generated code
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 06/14] qapi-event: Slightly shrink generated code Eric Blake
@ 2016-03-10 18:50 ` Markus Armbruster
2016-03-10 20:14 ` Eric Blake
2016-03-16 14:41 ` Markus Armbruster
1 sibling, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2016-03-10 18:50 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> Slightly rearrange the code in gen_event_send() for less generated
> output, by initializing 'emit' sooner, deferring an assertion
> to qdict_put_obj, and dropping a now-unused 'obj' local variable.
>
> While at it, document a FIXME related to the potential for a
> compiler naming collision - if the user ever creates a QAPI event
> whose name matches 'errp' or one of our local variables (like
> 'emit'), we'll have to revisit how we generate functions to
> avoid the problem.
>
> |@@ -25,16 +25,13 @@ void qapi_event_send_acpi_device_ost(ACP
> | {
> | QDict *qmp;
> | Error *err = NULL;
> |- QMPEventFuncEmit emit;
> |+ QMPEventFuncEmit emit = qmp_event_get_func_emit();
> | QmpOutputVisitor *qov;
> | Visitor *v;
> |- QObject *obj;
> |
> |- emit = qmp_event_get_func_emit();
> | if (!emit) {
> | return;
> | }
> |-
> | qmp = qmp_event_build_dict("ACPI_DEVICE_OST");
> |
> | qov = qmp_output_visitor_new();
> |@@ -53,11 +50,7 @@ out_obj:
> | if (err) {
> | goto out;
> | }
> |-
> |- obj = qmp_output_get_qobject(qov);
> |- g_assert(obj);
We're not "deferring an assertion to qdict_put_obj()", we're dropping a
dead one: qmp_output_get_qobject() never returns null.
I figure the assertion dates back to the time when it still did. Back
then, getting null here meant we screwed up.
I just searched the code for similarly dead assertions. Found one in
qapi_clone_InputEvent(), and serveral more in test-qmp-output-visitor.c.
There's also an error return in qapi_copy_SocketAddress(). Useless?
Should check for qnull instead?
> |-
> |- qdict_put_obj(qmp, "data", obj);
> |+ qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
> | emit(QAPI_EVENT_ACPI_DEVICE_OST, qmp, &err);
> |
> | out:
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v5: new patch
> ---
> scripts/qapi-event.py | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
> index c03cb78..02c9556 100644
> --- a/scripts/qapi-event.py
> +++ b/scripts/qapi-event.py
> @@ -29,13 +29,19 @@ def gen_event_send_decl(name, arg_type):
>
>
> def gen_event_send(name, arg_type):
> + # FIXME: Our declaration of local variables (and of 'errp' in the
> + # parameter list) can collide with exploded members of the event's
> + # data type passed in as parameters. If this collision ever hits in
> + # practice, we can rename our local variables with a leading _ prefix,
> + # or split the code into a wrapper function that creates a boxed
> + # 'param' object then calls another to do the real work.
> ret = mcgen('''
>
> %(proto)s
> {
> QDict *qmp;
> Error *err = NULL;
> - QMPEventFuncEmit emit;
> + QMPEventFuncEmit emit = qmp_event_get_func_emit();
> ''',
> proto=gen_event_send_proto(name, arg_type))
>
> @@ -43,16 +49,13 @@ def gen_event_send(name, arg_type):
> ret += mcgen('''
> QmpOutputVisitor *qov;
> Visitor *v;
> - QObject *obj;
> -
Please keep the blank line here...
> ''')
>
> ret += mcgen('''
> - emit = qmp_event_get_func_emit();
> +
... instead of adding it here.
> if (!emit) {
> return;
> }
> -
Let's keep this one.
> qmp = qmp_event_build_dict("%(name)s");
>
> ''',
> @@ -76,11 +79,7 @@ out_obj:
> if (err) {
> goto out;
> }
> -
> - obj = qmp_output_get_qobject(qov);
> - g_assert(obj);
> -
> - qdict_put_obj(qmp, "data", obj);
> + qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
> ''')
>
> ret += mcgen('''
Small improvements are welcome, too :)
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v5 07/14] qapi: Utilize implicit struct visits
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 07/14] qapi: Utilize implicit struct visits Eric Blake
@ 2016-03-10 19:05 ` Markus Armbruster
2016-03-10 20:16 ` Eric Blake
2016-03-16 14:45 ` Markus Armbruster
1 sibling, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2016-03-10 19:05 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> Rather than generate inline per-member visits, take advantage
> of the 'visit_type_FOO_members()' function for both event and
> command marshalling. This is possible now that implicit
> structs can be visited like any other.
>
> Generated code shrinks accordingly; events initialize a struct
> based on parameters, through a new gen_param_var() helper, like:
>
> |@@ -338,6 +250,9 @@ void qapi_event_send_block_job_error(con
> | QMPEventFuncEmit emit = qmp_event_get_func_emit();
> | QmpOutputVisitor *qov;
> | Visitor *v;
> |+ q_obj_BLOCK_JOB_ERROR_arg param = {
> |+ (char *)device, operation, action
> |+ };
> |
> | if (!emit) {
> | return;
> @@ -351,19 +266,7 @@ void qapi_event_send_block_job_error(con
> | if (err) {
> | goto out;
> | }
> |- visit_type_str(v, "device", (char **)&device, &err);
> |- if (err) {
> |- goto out_obj;
> |- }
> |- visit_type_IoOperationType(v, "operation", &operation, &err);
> |- if (err) {
> |- goto out_obj;
> |- }
> |- visit_type_BlockErrorAction(v, "action", &action, &err);
> |- if (err) {
> |- goto out_obj;
> |- }
> |-out_obj:
> |+ visit_type_q_obj_BLOCK_JOB_ERROR_arg_members(v, ¶m, &err);
> | visit_end_struct(v, err ? NULL : &err);
>
> Notice that the initialization of 'param' has to cast away const
> (just as the old gen_visit_members() had to do): we can't change
> the signature of the user function (which uses 'const char *'), but
> have to assign it to a non-const QAPI object (which requires
> 'char *').
>
> Likewise, command marshalling generates call arguments from a
> stack-allocated struct, rather than a list of local variables:
>
> |@@ -57,26 +57,15 @@ void qmp_marshal_add_fd(QDict *args, QOb
> | QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
> | QapiDeallocVisitor *qdv;
> | Visitor *v;
> |- bool has_fdset_id = false;
> |- int64_t fdset_id = 0;
> |- bool has_opaque = false;
> |- char *opaque = NULL;
> |-
> |- v = qmp_input_get_visitor(qiv);
> |- if (visit_optional(v, "fdset-id", &has_fdset_id)) {
> |- visit_type_int(v, "fdset-id", &fdset_id, &err);
> |- if (err) {
> |- goto out;
> |- }
> |- }
> |- if (visit_optional(v, "opaque", &has_opaque)) {
> |- visit_type_str(v, "opaque", &opaque, &err);
> |- if (err) {
> |- goto out;
> |- }
> |+ q_obj_add_fd_arg qapi = {0};
Let's calls this arg.
> |+
> |+ v = qmp_input_get_visitor(qiv);
> |+ visit_type_q_obj_add_fd_arg_members(v, &qapi, &err);
> |+ if (err) {
> |+ goto out;
> | }
> |
> |- retval = qmp_add_fd(has_fdset_id, fdset_id, has_opaque, opaque, &err);
> |+ retval = qmp_add_fd(qapi.has_fdset_id, qapi.fdset_id, qapi.has_opaque, qapi.opaque, &err);
> | if (err) {
> | goto out;
> | }
> |@@ -88,12 +77,7 @@ out:
> | qmp_input_visitor_cleanup(qiv);
> | qdv = qapi_dealloc_visitor_new();
> | v = qapi_dealloc_get_visitor(qdv);
> |- if (visit_optional(v, "fdset-id", &has_fdset_id)) {
> |- visit_type_int(v, "fdset-id", &fdset_id, NULL);
> |- }
> |- if (visit_optional(v, "opaque", &has_opaque)) {
> |- visit_type_str(v, "opaque", &opaque, NULL);
> |- }
> |+ visit_type_q_obj_add_fd_arg_members(v, &qapi, NULL);
> | qapi_dealloc_visitor_cleanup(qdv);
> | }
>
> For the marshaller, it has the nice side effect of eliminating a
> chance of collision between argument QMP names and local variables.
>
> This patch also paves the way for some followup simplifications
> in the generator, in subsequent patches.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v5: move qapi.py:gen_struct_init() to qapi-event.py:gen_param_var(),
> improve commit message
> v4: new patch
> ---
> scripts/qapi-commands.py | 28 ++++++++++++----------------
> scripts/qapi-event.py | 40 +++++++++++++++++++++++++++++++---------
> 2 files changed, 43 insertions(+), 25 deletions(-)
>
> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> index 3784f33..5ffc381 100644
> --- a/scripts/qapi-commands.py
> +++ b/scripts/qapi-commands.py
> @@ -33,8 +33,8 @@ def gen_call(name, arg_type, ret_type):
> assert not arg_type.variants
> for memb in arg_type.members:
> if memb.optional:
> - argstr += 'has_%s, ' % c_name(memb.name)
> - argstr += '%s, ' % c_name(memb.name)
> + argstr += 'qapi.has_%s, ' % c_name(memb.name)
> + argstr += 'qapi.%s, ' % c_name(memb.name)
>
> lhs = ''
> if ret_type:
> @@ -71,21 +71,10 @@ def gen_marshal_vars(arg_type, ret_type):
> QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
> QapiDeallocVisitor *qdv;
> Visitor *v;
> -''')
> + %(c_name)s qapi = {0};
>
> - for memb in arg_type.members:
> - if memb.optional:
> - ret += mcgen('''
> - bool has_%(c_name)s = false;
> ''',
> - c_name=c_name(memb.name))
> - ret += mcgen('''
> - %(c_type)s %(c_name)s = %(c_null)s;
> -''',
> - c_name=c_name(memb.name),
> - c_type=memb.type.c_type(),
> - c_null=memb.type.c_null())
> - ret += '\n'
> + c_name=arg_type.c_name())
> else:
> ret += mcgen('''
>
> @@ -107,17 +96,24 @@ def gen_marshal_input_visit(arg_type, dealloc=False):
> qdv = qapi_dealloc_visitor_new();
> v = qapi_dealloc_get_visitor(qdv);
> ''')
> + errp = 'NULL'
> else:
> ret += mcgen('''
> v = qmp_input_get_visitor(qiv);
> ''')
> + errp = '&err'
>
> - ret += gen_visit_members(arg_type.members, skiperr=dealloc)
> + ret += mcgen('''
> + visit_type_%(c_name)s_members(v, &qapi, %(errp)s);
> +''',
> + c_name=arg_type.c_name(), errp=errp)
>
> if dealloc:
> ret += mcgen('''
> qapi_dealloc_visitor_cleanup(qdv);
> ''')
> + else:
> + ret += gen_err_check()
> return ret
>
>
> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
> index 02c9556..c125ecb 100644
> --- a/scripts/qapi-event.py
> +++ b/scripts/qapi-event.py
> @@ -28,6 +28,30 @@ def gen_event_send_decl(name, arg_type):
> proto=gen_event_send_proto(name, arg_type))
>
>
> +# Declare and initialize an object 'qapi' using parameters from gen_params()
> +def gen_param_var(typ):
> + assert not typ.variants
> + ret = mcgen('''
> + %(c_name)s param = {
> +''',
> + c_name=typ.c_name())
> + sep = ' '
> + for memb in typ.members:
> + ret += sep
> + sep = ', '
> + if memb.optional:
> + ret += 'has_' + c_name(memb.name) + sep
> + if memb.type.name == 'str':
> + # Cast away const added in gen_params()
> + ret += '(char *)'
> + ret += c_name(memb.name)
> + ret += mcgen('''
> +
> + };
> +''')
> + return ret
> +
> +
> def gen_event_send(name, arg_type):
> # FIXME: Our declaration of local variables (and of 'errp' in the
> # parameter list) can collide with exploded members of the event's
> @@ -50,6 +74,7 @@ def gen_event_send(name, arg_type):
> QmpOutputVisitor *qov;
> Visitor *v;
> ''')
> + ret += gen_param_var(arg_type)
>
> ret += mcgen('''
>
> @@ -62,25 +87,22 @@ def gen_event_send(name, arg_type):
> name=name)
>
> if arg_type and arg_type.members:
> - assert not arg_type.variants
Already asserted by new gen_param_var(). Thanks for paying attention to
details.
> ret += mcgen('''
> qov = qmp_output_visitor_new();
> v = qmp_output_get_visitor(qov);
>
> visit_start_struct(v, "%(name)s", NULL, 0, &err);
> -''',
> - name=name)
> - ret += gen_err_check()
> - ret += gen_visit_members(arg_type.members, need_cast=True,
> - label='out_obj')
> - ret += mcgen('''
> -out_obj:
> + if (err) {
> + goto out;
> + }
> + visit_type_%(c_name)s_members(v, ¶m, &err);
> visit_end_struct(v, err ? NULL : &err);
> if (err) {
> goto out;
> }
> qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
> -''')
> +''',
> + name=name, c_name=arg_type.c_name())
>
> ret += mcgen('''
> emit(%(c_enum)s, qmp, &err);
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v5 06/14] qapi-event: Slightly shrink generated code
2016-03-10 18:50 ` Markus Armbruster
@ 2016-03-10 20:14 ` Eric Blake
0 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-10 20:14 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 3203 bytes --]
On 03/10/2016 11:50 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
>> Slightly rearrange the code in gen_event_send() for less generated
>> output, by initializing 'emit' sooner, deferring an assertion
>> to qdict_put_obj, and dropping a now-unused 'obj' local variable.
>>
>> While at it, document a FIXME related to the potential for a
>> compiler naming collision - if the user ever creates a QAPI event
>> whose name matches 'errp' or one of our local variables (like
>> 'emit'), we'll have to revisit how we generate functions to
>> avoid the problem.
>>
>
> We're not "deferring an assertion to qdict_put_obj()", we're dropping a
> dead one: qmp_output_get_qobject() never returns null.
Oh, good point; I can improve the commit message.
>
> I figure the assertion dates back to the time when it still did. Back
> then, getting null here meant we screwed up.
>
> I just searched the code for similarly dead assertions. Found one in
> qapi_clone_InputEvent(), and serveral more in test-qmp-output-visitor.c.
Speaking of that, I have a patch pending (but not yet posted) that adds
a clone visitor, so that we don't need qapi_clone_InputEvent() (it's
rather wasteful to convert into and back out of QObject when you can
just directly clone).
>
> There's also an error return in qapi_copy_SocketAddress(). Useless?
And that's the other hand-rolled clone that also gets nuked by my patch.
Some obvious copy-and-paste between the two.
> Should check for qnull instead?
Not necessary; we can't return qnull unless we visit nothing (or, when
my visit_type_null() lands, if we explicitly ask for it), but these
callers are visiting something that is not null.
>> %(proto)s
>> {
>> QDict *qmp;
>> Error *err = NULL;
>> - QMPEventFuncEmit emit;
>> + QMPEventFuncEmit emit = qmp_event_get_func_emit();
>> ''',
>> proto=gen_event_send_proto(name, arg_type))
>>
>> @@ -43,16 +49,13 @@ def gen_event_send(name, arg_type):
>> ret += mcgen('''
>> QmpOutputVisitor *qov;
>> Visitor *v;
>> - QObject *obj;
>> -
>
> Please keep the blank line here...
>
>> ''')
>>
>> ret += mcgen('''
>> - emit = qmp_event_get_func_emit();
>> +
>
> ... instead of adding it here.
Except that the next patch added one more declaration after Visitor *v,
but not in direct text, where keeping the blank line unmoved would
require splitting the mcgen() call into two parts. Or I could do ret +=
'\n'.
>
>> if (!emit) {
>> return;
>> }
>> -
>
> Let's keep this one.
Okay.
>
>> qmp = qmp_event_build_dict("%(name)s");
>>
>> ''',
>> @@ -76,11 +79,7 @@ out_obj:
>> if (err) {
>> goto out;
>> }
>> -
>> - obj = qmp_output_get_qobject(qov);
>> - g_assert(obj);
>> -
>> - qdict_put_obj(qmp, "data", obj);
>> + qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
>> ''')
>>
>> ret += mcgen('''
>
> Small improvements are welcome, too :)
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v5 07/14] qapi: Utilize implicit struct visits
2016-03-10 19:05 ` Markus Armbruster
@ 2016-03-10 20:16 ` Eric Blake
0 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-10 20:16 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 1894 bytes --]
On 03/10/2016 12:05 PM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
>> Rather than generate inline per-member visits, take advantage
>> of the 'visit_type_FOO_members()' function for both event and
>> command marshalling. This is possible now that implicit
>> structs can be visited like any other.
>>
>> Likewise, command marshalling generates call arguments from a
>> stack-allocated struct, rather than a list of local variables:
>>
>> |- goto out;
>> |- }
>> |+ q_obj_add_fd_arg qapi = {0};
>
> Let's calls this arg.
Sure.
>
>> |+
>> |+ v = qmp_input_get_visitor(qiv);
>> |+ visit_type_q_obj_add_fd_arg_members(v, &qapi, &err);
>> |+ if (err) {
>> |+ goto out;
>> | }
>> |
>> |- retval = qmp_add_fd(has_fdset_id, fdset_id, has_opaque, opaque, &err);
>> |+ retval = qmp_add_fd(qapi.has_fdset_id, qapi.fdset_id, qapi.has_opaque, qapi.opaque, &err);
and this line then gets a bit shorter.
>> +++ b/scripts/qapi-event.py
>> @@ -28,6 +28,30 @@ def gen_event_send_decl(name, arg_type):
>> proto=gen_event_send_proto(name, arg_type))
>> @@ -50,6 +74,7 @@ def gen_event_send(name, arg_type):
>> QmpOutputVisitor *qov;
>> Visitor *v;
>> ''')
>> + ret += gen_param_var(arg_type)
>>
>> ret += mcgen('''
>>
This is why I moved the blank line in 6/14. But I can rearrange things
as you requested.
I'm also wondering if this should be split into two patches (one for
qapi-visit, one for qapi-commands); when I first started writing it, I
thought there would be some code sharing between the two with edits to
qapi.py (see the v4 posting); but now they are distinct enough that two
commits is just as easy to do.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v5 13/14] qapi: Allow anonymous base for flat union
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 13/14] qapi: Allow anonymous base for flat union Eric Blake
@ 2016-03-10 20:22 ` Markus Armbruster
2016-03-10 20:50 ` Eric Blake
0 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2016-03-10 20:22 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> Rather than requiring all flat unions to explicitly create
> a separate base struct, we can allow the qapi schema to specify
> the common members via an inline dictionary. This is similar to
> how commands can specify an inline anonymous type for its 'data'.
> We already have several struct types that only exist to serve as
> a single flat union's base; the next commit will clean them up
> (in particular, the doc change to the BlockdevOptions example in
> this patch will be reflected to QMP in the next).
The parenthesis is a bit cryptic. "Reflected"?
> Now that anonymous bases are legal, we need to rework the
> flat-union-bad-base negative test (as previously written, it
> forms what is now valid QAPI; tweak it to now provide coverage
> of a new error message path), and add a positive test in
> qapi-schema-test to use an anonymous base (making the integer
> argument optional, for even more coverage).
>
> Note that this patch only allows anonymous bases for flat unions;
> simple unions are already enough syntactic sugar that we do not
> want to burden them further. Meanwhile, while it would be easy
> to also allow an anonymous base for structs, that would be quite
> redundant, as the members can be put right into the struct
> instead.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
Patch looks good.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types
2016-03-10 0:55 [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Eric Blake
` (13 preceding siblings ...)
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 14/14] qapi: Use anonymous bases in QMP flat unions Eric Blake
@ 2016-03-10 20:36 ` Markus Armbruster
2016-03-16 15:24 ` Markus Armbruster
14 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2016-03-10 20:36 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Eduardo Habkost
I finished review. I'll look over it tomorrow to decide whether I can
take it with minor tweaks, or whether we need v6. I'm hopeful :)
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v5 13/14] qapi: Allow anonymous base for flat union
2016-03-10 20:22 ` Markus Armbruster
@ 2016-03-10 20:50 ` Eric Blake
2016-03-16 15:21 ` Markus Armbruster
0 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2016-03-10 20:50 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 1662 bytes --]
On 03/10/2016 01:22 PM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
>> Rather than requiring all flat unions to explicitly create
>> a separate base struct, we can allow the qapi schema to specify
>> the common members via an inline dictionary. This is similar to
>> how commands can specify an inline anonymous type for its 'data'.
>> We already have several struct types that only exist to serve as
>> a single flat union's base; the next commit will clean them up
>> (in particular, the doc change to the BlockdevOptions example in
>> this patch will be reflected to QMP in the next).
>
> The parenthesis is a bit cryptic. "Reflected"?
Maybe s/reflected to/implemented in/ would read better.
>
>> Now that anonymous bases are legal, we need to rework the
>> flat-union-bad-base negative test (as previously written, it
>> forms what is now valid QAPI; tweak it to now provide coverage
>> of a new error message path), and add a positive test in
>> qapi-schema-test to use an anonymous base (making the integer
>> argument optional, for even more coverage).
>>
>> Note that this patch only allows anonymous bases for flat unions;
>> simple unions are already enough syntactic sugar that we do not
>> want to burden them further. Meanwhile, while it would be easy
>> to also allow an anonymous base for structs, that would be quite
>> redundant, as the members can be put right into the struct
>> instead.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> Patch looks good.
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v5 04/14] qapi: Adjust names of implicit types
2016-03-10 16:11 ` Eric Blake
@ 2016-03-11 7:48 ` Markus Armbruster
2016-03-11 17:29 ` Eric Blake
0 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2016-03-11 7:48 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> On 03/10/2016 06:39 AM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>>
>>> The original choice of ':obj-' as the prefix for implicit types
>>> made it obvious that we weren't going to clash with any user-defined
>>> names. But now we want to create structs for implicit types.
>>
>> Why? I know, but the commit message should still give a hint. Perhaps:
>> "to get rid of special cases in the generators"?
>>
>>> We
>>> could transliterate ':' to '_', except that C99 says that a leading
>>> underscore and lower-case letter should be used only for file scope
>>> identifiers, while we would be exposing it in qapi-types.h. So it's
>>
>> Misunderstanding! When the standard says "identifiers that X are
>> reserved for Y use", it reserves these identifiers for itself and the
>> implementation. You shouldn't use them for Y then.
>>
>> Suggest to simply quote the standard instead of interpreting it:
>> ... except that C99 mandates that "identifiers that begin with an
>> underscore are always reserved for use as identifiers with file scope in
>> both the ordinary and tag name spaces"
>
> Both those changes sound fine.
>
>>
>>> time to change our naming convention; we can instead use the 'q_'
>>> prefix that we reserved for ourselves back in commit 9fb081e0. As
>>> long as we don't declare 'empty' or 'obj' ticklish, it shouldn't
>>> clash with c_name() prepending 'q_' to the user's ticklish names.
>>
>> Do we really want to rename :empty? We're not going to generate C for
>> it, are we?
>
> No, but it was easier to implement .is_implicit() as
> "name.startswith('q_')" than as "name == ':empty' or
> name.startswith('q_obj')". I can stick with :empty if you want a
> respin, though.
You avoid complicating .is_implicit() slightly, and you pay for that
with a bit of patch churn elsewhere. Sounds justified.
Is ':empty' the last use of the ':' prefix?
General maxims:
1. Keep the generator simple while generating something reasonable. No
need to overcomplicate things when we can rely on the optimizer to do
its job. Keeping headers lean is worth more complexity than keeping .c
files lean, because headers get compiled much, much more.
2. When all else is equal, avoid patch churn.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v5 04/14] qapi: Adjust names of implicit types
2016-03-11 7:48 ` Markus Armbruster
@ 2016-03-11 17:29 ` Eric Blake
0 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-11 17:29 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]
On 03/11/2016 12:48 AM, Markus Armbruster wrote:
>>>> time to change our naming convention; we can instead use the 'q_'
>>>> prefix that we reserved for ourselves back in commit 9fb081e0. As
>>>> long as we don't declare 'empty' or 'obj' ticklish, it shouldn't
>>>> clash with c_name() prepending 'q_' to the user's ticklish names.
>>>
>>> Do we really want to rename :empty? We're not going to generate C for
>>> it, are we?
>>
>> No, but it was easier to implement .is_implicit() as
>> "name.startswith('q_')" than as "name == ':empty' or
>> name.startswith('q_obj')". I can stick with :empty if you want a
>> respin, though.
>
> You avoid complicating .is_implicit() slightly, and you pay for that
> with a bit of patch churn elsewhere. Sounds justified.
>
> Is ':empty' the last use of the ':' prefix?
Yes. And renaming it to 'q_empty' meant I didn't have to add ':' to the
set of characters to be transliterated in c_name().
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v5 06/14] qapi-event: Slightly shrink generated code
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 06/14] qapi-event: Slightly shrink generated code Eric Blake
2016-03-10 18:50 ` Markus Armbruster
@ 2016-03-16 14:41 ` Markus Armbruster
1 sibling, 0 replies; 32+ messages in thread
From: Markus Armbruster @ 2016-03-16 14:41 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> Slightly rearrange the code in gen_event_send() for less generated
> output, by initializing 'emit' sooner, deferring an assertion
> to qdict_put_obj, and dropping a now-unused 'obj' local variable.
>
> While at it, document a FIXME related to the potential for a
> compiler naming collision - if the user ever creates a QAPI event
> whose name matches 'errp' or one of our local variables (like
> 'emit'), we'll have to revisit how we generate functions to
> avoid the problem.
I think this should go into the next patch, where we actually fix the
same collision in command marshallers.
> |@@ -25,16 +25,13 @@ void qapi_event_send_acpi_device_ost(ACP
> | {
> | QDict *qmp;
> | Error *err = NULL;
> |- QMPEventFuncEmit emit;
> |+ QMPEventFuncEmit emit = qmp_event_get_func_emit();
> | QmpOutputVisitor *qov;
> | Visitor *v;
> |- QObject *obj;
> |
> |- emit = qmp_event_get_func_emit();
> | if (!emit) {
> | return;
> | }
> |-
> | qmp = qmp_event_build_dict("ACPI_DEVICE_OST");
> |
> | qov = qmp_output_visitor_new();
> |@@ -53,11 +50,7 @@ out_obj:
> | if (err) {
> | goto out;
> | }
> |-
> |- obj = qmp_output_get_qobject(qov);
> |- g_assert(obj);
> |-
> |- qdict_put_obj(qmp, "data", obj);
> |+ qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
> | emit(QAPI_EVENT_ACPI_DEVICE_OST, qmp, &err);
> |
> | out:
This is actually two separate simplifications. One is initializing emit
instead of assigning to it:
> |@@ -25,16 +25,13 @@ void qapi_event_send_acpi_device_ost(ACP
> | {
> | QDict *qmp;
> | Error *err = NULL;
> |- QMPEventFuncEmit emit;
> |+ QMPEventFuncEmit emit = qmp_event_get_func_emit();
> | QmpOutputVisitor *qov;
> | Visitor *v;
> | QObject *obj;
> |
> |- emit = qmp_event_get_func_emit();
> | if (!emit) {
> | return;
> | }
> |-
> | qmp = qmp_event_build_dict("ACPI_DEVICE_OST");
> |
> | qov = qmp_output_visitor_new();
I'm afraid I don't like it, because it separates
qmp_event_get_func_emit() from its check.
A more likable simplification would be dropping the indirection
outright: emit is always monitor_qapi_event_queue(), except in
test-qmp-event. That's a rather weak excuse for complicating things
with an indirection. Let's not upset this series with it, though.
The other simplification is burying the dead check of
qmp_output_get_qobject()'s value:
> |@@ -25,16 +25,13 @@ void qapi_event_send_acpi_device_ost(ACP
> | {
> | QDict *qmp;
> | Error *err = NULL;
> | QMPEventFuncEmit emit;
> | QmpOutputVisitor *qov;
> | Visitor *v;
> |- QObject *obj;
> |
> | emit = qmp_event_get_func_emit();
> |@@ -53,11 +50,7 @@ out_obj:
> | if (err) {
> | goto out;
> | }
> |-
> |- obj = qmp_output_get_qobject(qov);
> |- g_assert(obj);
> |-
> |- qdict_put_obj(qmp, "data", obj);
> |+ qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
> | emit(QAPI_EVENT_ACPI_DEVICE_OST, qmp, &err);
> |
> | out:
Yes, please.
Patch shrinks to just this:
>From bfaa962b04f5365289c46dadb0c078a13d52eae8 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 9 Mar 2016 17:55:27 -0700
Subject: [PATCH 06/14] qapi-event: Drop qmp_output_get_qobject() null check
qmp_output_get_qobject() was changed never to return null some time
ago, but the qapi_event_send_FOO() functions still check. Clean that
up:
|@@ -28,7 +28,6 @@ void qapi_event_send_acpi_device_ost(ACP
| QMPEventFuncEmit emit;
| QmpOutputVisitor *qov;
| Visitor *v;
|- QObject *obj;
|
| emit = qmp_event_get_func_emit();
| if (!emit) {
|@@ -54,10 +53,7 @@ out_obj:
| goto out;
| }
|
|- obj = qmp_output_get_qobject(qov);
|- g_assert(obj);
|-
|- qdict_put_obj(qmp, "data", obj);
|+ qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
| emit(QAPI_EVENT_ACPI_DEVICE_OST, qmp, &err);
|
| out:
Signed-off-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi-event.py | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index c03cb78..27af206 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -43,7 +43,6 @@ def gen_event_send(name, arg_type):
ret += mcgen('''
QmpOutputVisitor *qov;
Visitor *v;
- QObject *obj;
''')
@@ -77,10 +76,7 @@ out_obj:
goto out;
}
- obj = qmp_output_get_qobject(qov);
- g_assert(obj);
-
- qdict_put_obj(qmp, "data", obj);
+ qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
''')
ret += mcgen('''
--
2.4.3
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v5 07/14] qapi: Utilize implicit struct visits
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 07/14] qapi: Utilize implicit struct visits Eric Blake
2016-03-10 19:05 ` Markus Armbruster
@ 2016-03-16 14:45 ` Markus Armbruster
1 sibling, 0 replies; 32+ messages in thread
From: Markus Armbruster @ 2016-03-16 14:45 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Here's how I'd rebase this on the simplified PATCH 06 I just posted as a
reply to yours:
>From e35449172828351f0acd85f236add45d3eb575a7 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 9 Mar 2016 17:55:28 -0700
Subject: [PATCH 07/14] qapi: Utilize implicit struct visits
Rather than generate inline per-member visits, take advantage
of the 'visit_type_FOO_members()' function for both event and
command marshalling. This is possible now that implicit
structs can be visited like any other.
Generated code shrinks accordingly; events initialize a struct
based on parameters, through a new gen_param_var() helper, like:
|@@ -338,6 +250,9 @@ void qapi_event_send_block_job_error(con
| QMPEventFuncEmit emit = qmp_event_get_func_emit();
| QmpOutputVisitor *qov;
| Visitor *v;
|+ q_obj_BLOCK_JOB_ERROR_arg param = {
|+ (char *)device, operation, action
|+ };
|
| if (!emit) {
| return;
@@ -351,19 +266,7 @@ void qapi_event_send_block_job_error(con
| if (err) {
| goto out;
| }
|- visit_type_str(v, "device", (char **)&device, &err);
|- if (err) {
|- goto out_obj;
|- }
|- visit_type_IoOperationType(v, "operation", &operation, &err);
|- if (err) {
|- goto out_obj;
|- }
|- visit_type_BlockErrorAction(v, "action", &action, &err);
|- if (err) {
|- goto out_obj;
|- }
|-out_obj:
|+ visit_type_q_obj_BLOCK_JOB_ERROR_arg_members(v, ¶m, &err);
| visit_end_struct(v, err ? NULL : &err);
Notice that the initialization of 'param' has to cast away const
(just as the old gen_visit_members() had to do): we can't change
the signature of the user function (which uses 'const char *'), but
have to assign it to a non-const QAPI object (which requires
'char *').
Likewise, command marshalling generates call arguments from a
stack-allocated struct, rather than a list of local variables:
|@@ -57,26 +57,15 @@ void qmp_marshal_add_fd(QDict *args, QOb
| QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
| QapiDeallocVisitor *qdv;
| Visitor *v;
|- bool has_fdset_id = false;
|- int64_t fdset_id = 0;
|- bool has_opaque = false;
|- char *opaque = NULL;
|-
|- v = qmp_input_get_visitor(qiv);
|- if (visit_optional(v, "fdset-id", &has_fdset_id)) {
|- visit_type_int(v, "fdset-id", &fdset_id, &err);
|- if (err) {
|- goto out;
|- }
|- }
|- if (visit_optional(v, "opaque", &has_opaque)) {
|- visit_type_str(v, "opaque", &opaque, &err);
|- if (err) {
|- goto out;
|- }
|+ q_obj_add_fd_arg qapi = {0};
|+
|+ v = qmp_input_get_visitor(qiv);
|+ visit_type_q_obj_add_fd_arg_members(v, &qapi, &err);
|+ if (err) {
|+ goto out;
| }
|
|- retval = qmp_add_fd(has_fdset_id, fdset_id, has_opaque, opaque, &err);
|+ retval = qmp_add_fd(qapi.has_fdset_id, qapi.fdset_id, qapi.has_opaque, qapi.opaque, &err);
| if (err) {
| goto out;
| }
|@@ -88,12 +77,7 @@ out:
| qmp_input_visitor_cleanup(qiv);
| qdv = qapi_dealloc_visitor_new();
| v = qapi_dealloc_get_visitor(qdv);
|- if (visit_optional(v, "fdset-id", &has_fdset_id)) {
|- visit_type_int(v, "fdset-id", &fdset_id, NULL);
|- }
|- if (visit_optional(v, "opaque", &has_opaque)) {
|- visit_type_str(v, "opaque", &opaque, NULL);
|- }
|+ visit_type_q_obj_add_fd_arg_members(v, &qapi, NULL);
| qapi_dealloc_visitor_cleanup(qdv);
| }
For the marshaller, it has the nice side effect of eliminating a
chance of collision between argument QMP names and local variables.
A similar collision still exists in the qapi_event_send_FOO().
Document it with a FIXME comment.
This patch also paves the way for some followup simplifications
in the generator, in subsequent patches.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1457571335-10938-8-git-send-email-eblake@redhat.com>
---
scripts/qapi-commands.py | 28 ++++++++++++----------------
scripts/qapi-event.py | 48 ++++++++++++++++++++++++++++++++++++++----------
2 files changed, 50 insertions(+), 26 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 3784f33..5ffc381 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -33,8 +33,8 @@ def gen_call(name, arg_type, ret_type):
assert not arg_type.variants
for memb in arg_type.members:
if memb.optional:
- argstr += 'has_%s, ' % c_name(memb.name)
- argstr += '%s, ' % c_name(memb.name)
+ argstr += 'qapi.has_%s, ' % c_name(memb.name)
+ argstr += 'qapi.%s, ' % c_name(memb.name)
lhs = ''
if ret_type:
@@ -71,21 +71,10 @@ def gen_marshal_vars(arg_type, ret_type):
QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
QapiDeallocVisitor *qdv;
Visitor *v;
-''')
+ %(c_name)s qapi = {0};
- for memb in arg_type.members:
- if memb.optional:
- ret += mcgen('''
- bool has_%(c_name)s = false;
''',
- c_name=c_name(memb.name))
- ret += mcgen('''
- %(c_type)s %(c_name)s = %(c_null)s;
-''',
- c_name=c_name(memb.name),
- c_type=memb.type.c_type(),
- c_null=memb.type.c_null())
- ret += '\n'
+ c_name=arg_type.c_name())
else:
ret += mcgen('''
@@ -107,17 +96,24 @@ def gen_marshal_input_visit(arg_type, dealloc=False):
qdv = qapi_dealloc_visitor_new();
v = qapi_dealloc_get_visitor(qdv);
''')
+ errp = 'NULL'
else:
ret += mcgen('''
v = qmp_input_get_visitor(qiv);
''')
+ errp = '&err'
- ret += gen_visit_members(arg_type.members, skiperr=dealloc)
+ ret += mcgen('''
+ visit_type_%(c_name)s_members(v, &qapi, %(errp)s);
+''',
+ c_name=arg_type.c_name(), errp=errp)
if dealloc:
ret += mcgen('''
qapi_dealloc_visitor_cleanup(qdv);
''')
+ else:
+ ret += gen_err_check()
return ret
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 27af206..9b5c5b5 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -28,7 +28,37 @@ def gen_event_send_decl(name, arg_type):
proto=gen_event_send_proto(name, arg_type))
+# Declare and initialize an object 'qapi' using parameters from gen_params()
+def gen_param_var(typ):
+ assert not typ.variants
+ ret = mcgen('''
+ %(c_name)s param = {
+''',
+ c_name=typ.c_name())
+ sep = ' '
+ for memb in typ.members:
+ ret += sep
+ sep = ', '
+ if memb.optional:
+ ret += 'has_' + c_name(memb.name) + sep
+ if memb.type.name == 'str':
+ # Cast away const added in gen_params()
+ ret += '(char *)'
+ ret += c_name(memb.name)
+ ret += mcgen('''
+
+ };
+''')
+ return ret
+
+
def gen_event_send(name, arg_type):
+ # FIXME: Our declaration of local variables (and of 'errp' in the
+ # parameter list) can collide with exploded members of the event's
+ # data type passed in as parameters. If this collision ever hits in
+ # practice, we can rename our local variables with a leading _ prefix,
+ # or split the code into a wrapper function that creates a boxed
+ # 'param' object then calls another to do the real work.
ret = mcgen('''
%(proto)s
@@ -43,10 +73,11 @@ def gen_event_send(name, arg_type):
ret += mcgen('''
QmpOutputVisitor *qov;
Visitor *v;
-
''')
+ ret += gen_param_var(arg_type)
ret += mcgen('''
+
emit = qmp_event_get_func_emit();
if (!emit) {
return;
@@ -58,26 +89,23 @@ def gen_event_send(name, arg_type):
name=name)
if arg_type and arg_type.members:
- assert not arg_type.variants
ret += mcgen('''
qov = qmp_output_visitor_new();
v = qmp_output_get_visitor(qov);
visit_start_struct(v, "%(name)s", NULL, 0, &err);
-''',
- name=name)
- ret += gen_err_check()
- ret += gen_visit_members(arg_type.members, need_cast=True,
- label='out_obj')
- ret += mcgen('''
-out_obj:
+ if (err) {
+ goto out;
+ }
+ visit_type_%(c_name)s_members(v, ¶m, &err);
visit_end_struct(v, err ? NULL : &err);
if (err) {
goto out;
}
qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
-''')
+''',
+ name=name, c_name=arg_type.c_name())
ret += mcgen('''
emit(%(c_enum)s, qmp, &err);
--
2.4.3
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v5 13/14] qapi: Allow anonymous base for flat union
2016-03-10 20:50 ` Eric Blake
@ 2016-03-16 15:21 ` Markus Armbruster
0 siblings, 0 replies; 32+ messages in thread
From: Markus Armbruster @ 2016-03-16 15:21 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> On 03/10/2016 01:22 PM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>>
>>> Rather than requiring all flat unions to explicitly create
>>> a separate base struct, we can allow the qapi schema to specify
>>> the common members via an inline dictionary. This is similar to
>>> how commands can specify an inline anonymous type for its 'data'.
>>> We already have several struct types that only exist to serve as
>>> a single flat union's base; the next commit will clean them up
>>> (in particular, the doc change to the BlockdevOptions example in
>>> this patch will be reflected to QMP in the next).
>>
>> The parenthesis is a bit cryptic. "Reflected"?
>
> Maybe s/reflected to/implemented in/ would read better.
What about:
a single flat union's base; the next commit will clean them up.
In particular, this patch's change to the BlockdevOptions example
in qapi-code-gen.txt will actually be done in the real QAPI schema.
[...]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types
2016-03-10 20:36 ` [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Markus Armbruster
@ 2016-03-16 15:24 ` Markus Armbruster
2016-03-17 15:36 ` Eric Blake
0 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2016-03-16 15:24 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Eduardo Habkost
I think the additional changes I'd like to see are small enough to not
require a full respin. I took the liberty to work them in, and pushed
the result to my qapi-not-next branch. Let me know whether you like
them.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types
2016-03-16 15:24 ` Markus Armbruster
@ 2016-03-17 15:36 ` Eric Blake
0 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-17 15:36 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Eduardo Habkost
[-- Attachment #1: Type: text/plain, Size: 595 bytes --]
On 03/16/2016 09:24 AM, Markus Armbruster wrote:
> I think the additional changes I'd like to see are small enough to not
> require a full respin. I took the liberty to work them in, and pushed
> the result to my qapi-not-next branch. Let me know whether you like
> them.
The code changes looked okay, but I think there were still some tweaks
to commit messages that you pointed out during review that might be
worth making; I'll go ahead and post a v6 to make it easier.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2016-03-17 15:36 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-10 0:55 [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 01/14] qapi: Assert in places where variants are not handled Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 02/14] qapi: Fix command with named empty argument type Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 03/14] qapi: Make c_type() more OO-like Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 04/14] qapi: Adjust names of implicit types Eric Blake
2016-03-10 13:39 ` Markus Armbruster
2016-03-10 16:11 ` Eric Blake
2016-03-11 7:48 ` Markus Armbruster
2016-03-11 17:29 ` Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 05/14] qapi: Emit implicit structs in generated C Eric Blake
2016-03-10 14:25 ` Markus Armbruster
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 06/14] qapi-event: Slightly shrink generated code Eric Blake
2016-03-10 18:50 ` Markus Armbruster
2016-03-10 20:14 ` Eric Blake
2016-03-16 14:41 ` Markus Armbruster
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 07/14] qapi: Utilize implicit struct visits Eric Blake
2016-03-10 19:05 ` Markus Armbruster
2016-03-10 20:16 ` Eric Blake
2016-03-16 14:45 ` Markus Armbruster
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 08/14] qapi-commands: Inline single-use helpers of gen_marshal() Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 09/14] qapi: Inline gen_visit_members() into lone caller Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 10/14] qapi: Drop unused c_null() Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 11/14] qapi: Don't special-case simple union wrappers Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 12/14] qapi: Make BlockdevOptions doc example closer to reality Eric Blake
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 13/14] qapi: Allow anonymous base for flat union Eric Blake
2016-03-10 20:22 ` Markus Armbruster
2016-03-10 20:50 ` Eric Blake
2016-03-16 15:21 ` Markus Armbruster
2016-03-10 0:55 ` [Qemu-devel] [PATCH v5 14/14] qapi: Use anonymous bases in QMP flat unions Eric Blake
2016-03-10 20:36 ` [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types Markus Armbruster
2016-03-16 15:24 ` Markus Armbruster
2016-03-17 15:36 ` Eric Blake
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.