* [Qemu-devel] [PATCH v4 00/10] easier unboxed visits/qapi implicit types
@ 2016-03-05 16:16 Eric Blake
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 01/10] qapi: Assert in places where variants are not handled Eric Blake
` (9 more replies)
0 siblings, 10 replies; 36+ messages in thread
From: Eric Blake @ 2016-03-05 16:16 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Eduardo Habkost
This is patches 10-19 of v2:
https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg06079.html
Depends on Markus' qapi-next branch (which includes v3 of this
series mapping to the first half of v2).
Also available as a tag at this location:
git fetch git://repo.or.cz/qemu/ericb.git qapi-implicit-v4
and will soon be part of my branch at:
http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/qapi
The big change here is a refactoring of how to visit implicit types:
I bit the bullet, and exposed the types in qapi-types.h, along with
visit_type_FOO_members() in qapi-visit.h. It made a lot of the
earlier series pointless (I no longer needed patch 10, 11, 13, or
15), at the expense of a couple new patches. The generated code
gets larger, but I reclaimed some of the growth in qapi-types.h by
using the visit_type_FOO_members() functions in more places in
qapi-commands.py and qapi-events.py. This series also rearranges
some patches posted earlier, and squashed 17-18 as part of
converting one more flat union.
I suspect there may be some conversation on how best to detect
implicit types (I hacked in "name[0] == ':'", which is not the
prettiest), but am overall pleased with how this turned out in
comparison to v2.
backport-diff gets confused by a patch split and some renames:
001/10:[down] 'qapi: Assert in places where variants are not handled'
002/10:[0006] [FC] 'qapi: Fix command with named empty argument type'
003/10:[0016] [FC] 'qapi: Make c_type() more OO-like'
004/10:[down] 'qapi: Emit implicit structs in generated C'
005/10:[down] 'qapi: Utilize implicit struct visits'
006/10:[down] 'qapi-commands: Inline single-use helpers of gen_marshal()'
007/10:[0098] [FC] 'qapi: Don't special-case simple union wrappers'
008/10:[0038] [FC] 'qapi: Allow anonymous base for flat union'
009/10:[down] 'qapi: Use anonymous bases in QMP flat unions'
010/10:[----] [-C] 'qapi: Populate info['name'] for each entity'
Eric Blake (10):
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: Emit implicit structs in generated C
qapi: Utilize implicit struct visits
qapi-commands: Inline single-use helpers of gen_marshal()
qapi: Don't special-case simple union wrappers
qapi: Allow anonymous base for flat union
qapi: Use anonymous bases in QMP flat unions
qapi: Populate info['name'] for each entity
scripts/qapi.py | 105 ++++++++++++++++++--------
scripts/qapi-commands.py | 117 +++++++++++------------------
scripts/qapi-event.py | 17 ++---
scripts/qapi-types.py | 27 ++++---
scripts/qapi-visit.py | 40 +++-------
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 | 30 ++++----
qapi-schema.json | 20 ++---
qapi/block-core.json | 98 +++++++++++-------------
qapi/introspect.json | 12 +--
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 | 8 +-
tests/qapi-schema/qapi-schema-test.out | 12 +--
53 files changed, 476 insertions(+), 496 deletions(-)
--
2.5.0
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v4 01/10] qapi: Assert in places where variants are not handled
2016-03-05 16:16 [Qemu-devel] [PATCH v4 00/10] easier unboxed visits/qapi implicit types Eric Blake
@ 2016-03-05 16:16 ` Eric Blake
2016-03-08 10:12 ` Markus Armbruster
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 02/10] qapi: Fix command with named empty argument type Eric Blake
` (8 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Eric Blake @ 2016-03-05 16:16 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: base types must still be plain structs, and anywhere we
explode a struct into a parameter list (events and command
marshalling), we don't support variants in that explosion.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v4: new patch, split out from .is_empty() patch
---
scripts/qapi.py | 1 +
scripts/qapi-commands.py | 3 ++-
scripts/qapi-event.py | 1 +
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 6b2aa6e..dc26ef9 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -960,6 +960,7 @@ class QAPISchemaObjectType(QAPISchemaType):
assert isinstance(self.base, QAPISchemaObjectType)
self.base.check(schema)
self.base.check_clash(schema, self.info, seen)
+ assert not self.base.variants
for m in self.local_members:
m.check(schema)
m.check_clash(self.info, seen)
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] 36+ messages in thread
* [Qemu-devel] [PATCH v4 02/10] qapi: Fix command with named empty argument type
2016-03-05 16:16 [Qemu-devel] [PATCH v4 00/10] easier unboxed visits/qapi implicit types Eric Blake
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 01/10] qapi: Assert in places where variants are not handled Eric Blake
@ 2016-03-05 16:16 ` Eric Blake
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 03/10] qapi: Make c_type() more OO-like Eric Blake
` (7 subsequent siblings)
9 siblings, 0 replies; 36+ messages in thread
From: Eric Blake @ 2016-03-05 16:16 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>
---
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] 36+ messages in thread
* [Qemu-devel] [PATCH v4 03/10] qapi: Make c_type() more OO-like
2016-03-05 16:16 [Qemu-devel] [PATCH v4 00/10] easier unboxed visits/qapi implicit types Eric Blake
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 01/10] qapi: Assert in places where variants are not handled Eric Blake
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 02/10] qapi: Fix command with named empty argument type Eric Blake
@ 2016-03-05 16:16 ` Eric Blake
2016-03-08 10:54 ` Markus Armbruster
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 04/10] qapi: Emit implicit structs in generated C Eric Blake
` (6 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Eric Blake @ 2016-03-05 16:16 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
QAPISchemaType.c_type() was a bit awkward. Rather than having two
optional orthogonal boolean flags that should never both be true,
and where all callers should pass a compile-time constant (well,
our use of is_unboxed wasn't constant, but a future patch is about
to remove the special case for simple unions, so we can live with
the churn of refactoring the code in the meantime), the more
object-oriented approach uses different method names that can be
overridden as needed, and which convey the intent by the method
name. The caller just makes sure to use the right variant, rather
than having to worry about boolean flags.
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>
---
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 dc26ef9..b1b87ee 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'
@@ -986,12 +1002,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'
@@ -1140,6 +1158,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'
@@ -1631,7 +1652,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] 36+ messages in thread
* [Qemu-devel] [PATCH v4 04/10] qapi: Emit implicit structs in generated C
2016-03-05 16:16 [Qemu-devel] [PATCH v4 00/10] easier unboxed visits/qapi implicit types Eric Blake
` (2 preceding siblings ...)
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 03/10] qapi: Make c_type() more OO-like Eric Blake
@ 2016-03-05 16:16 ` Eric Blake
2016-03-08 14:24 ` Markus Armbruster
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 05/10] qapi: Utilize implicit struct visits Eric Blake
` (5 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Eric Blake @ 2016-03-05 16:16 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[0] == ':'" feels a bit hacky, but beats changing the
signature of the visit_object_type() callback to pass a new
'implicit' flag. 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 ':empty' 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 fact that all our implicit objects start
with a leading ':', which can be transliteratated to a leading
single underscore, and we've already documented and enforce that
the user can't create QAPI names with a leading underscore, so
exposing the types won't create any collisions. It is a bit
unfortunate that the generated struct names 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; in part because
it was easier to output every implicit type rather than adding
generator complexity to try and only output types as needed.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v4: new patch
---
scripts/qapi.py | 3 +--
scripts/qapi-types.py | 12 ++++++------
scripts/qapi-visit.py | 18 ++++++++----------
3 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index b1b87ee..6cf0a75 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -999,7 +999,6 @@ class QAPISchemaObjectType(QAPISchemaType):
return self.name[0] == ':'
def c_name(self):
- assert not self.is_implicit()
return QAPISchemaType.c_name(self)
def c_type(self):
@@ -1476,7 +1475,7 @@ def c_enum_const(type_name, const_name, prefix=None):
type_name = prefix
return camel_to_upper(type_name) + '_' + c_name(const_name, False).upper()
-c_name_trans = string.maketrans('.-', '__')
+c_name_trans = string.maketrans('.-:', '___')
# Map @name to a valid C identifier.
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index f194bea..fa2d6af 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 != ':empty'
def _gen_type_cleanup(self, name):
self.decl += gen_type_cleanup_decl(name)
@@ -233,7 +231,9 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
self.decl += gen_object(name, base, members, variants)
if base:
self.decl += gen_upcast(name, base)
- self._gen_type_cleanup(name)
+ if name[0] != ':':
+ # 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..b4303a7 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 != ':empty'
def visit_enum_type(self, name, info, values, prefix):
# Special case for our lone builtin enum type
@@ -297,8 +292,11 @@ 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)
+ if name[0] != ':':
+ # 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] 36+ messages in thread
* [Qemu-devel] [PATCH v4 05/10] qapi: Utilize implicit struct visits
2016-03-05 16:16 [Qemu-devel] [PATCH v4 00/10] easier unboxed visits/qapi implicit types Eric Blake
` (3 preceding siblings ...)
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 04/10] qapi: Emit implicit structs in generated C Eric Blake
@ 2016-03-05 16:16 ` Eric Blake
2016-03-08 15:10 ` Markus Armbruster
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 06/10] qapi-commands: Inline single-use helpers of gen_marshal() Eric Blake
` (4 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Eric Blake @ 2016-03-05 16:16 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, such as:
|@@ -381,6 +293,9 @@ void qapi_event_send_block_job_error(con
| QmpOutputVisitor *qov;
| Visitor *v;
| QObject *obj;
|+ _obj_BLOCK_JOB_ERROR_arg qapi = {
|+ (char *)device, operation, action
|+ };
|
| emit = qmp_event_get_func_emit();
| if (!emit) {
|@@ -396,19 +311,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__obj_BLOCK_JOB_ERROR_arg_members(v, &qapi, &err);
| visit_end_struct(v, err ? NULL : &err);
And command marshalling generates call arguments from a stack-allocated
struct:
|@@ -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;
|- }
|+ _obj_add_fd_arg qapi = {0};
|+
|+ v = qmp_input_get_visitor(qiv);
|+ visit_type__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__obj_add_fd_arg_members(v, &qapi, NULL);
| qapi_dealloc_visitor_cleanup(qdv);
| }
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v4: new patch
---
scripts/qapi.py | 25 +++++++++++++++++++++++++
scripts/qapi-commands.py | 28 ++++++++++++----------------
scripts/qapi-event.py | 16 +++++++---------
3 files changed, 44 insertions(+), 25 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 6cf0a75..797ac7a 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1640,6 +1640,7 @@ extern const char *const %(c_name)s_lookup[];
return ret
+# Explode arg_type into a parameter list, along with extra parameters
def gen_params(arg_type, extra):
if not arg_type:
return extra
@@ -1657,6 +1658,30 @@ def gen_params(arg_type, extra):
return ret
+# Declare and initialize an object 'qapi' using parameters from gen_params()
+def gen_struct_init(typ):
+ assert not typ.variants
+ ret = mcgen('''
+ %(c_name)s qapi = {
+''',
+ 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_err_check(label='out', skiperr=False):
if skiperr:
return ''
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 c03cb78..627e9a0 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -44,8 +44,8 @@ def gen_event_send(name, arg_type):
QmpOutputVisitor *qov;
Visitor *v;
QObject *obj;
-
''')
+ ret += gen_struct_init(arg_type) + '\n'
ret += mcgen('''
emit = qmp_event_get_func_emit();
@@ -65,13 +65,10 @@ def gen_event_send(name, arg_type):
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, &qapi, &err);
visit_end_struct(v, err ? NULL : &err);
if (err) {
goto out;
@@ -81,7 +78,8 @@ out_obj:
g_assert(obj);
qdict_put_obj(qmp, "data", obj);
-''')
+''',
+ 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] 36+ messages in thread
* [Qemu-devel] [PATCH v4 06/10] qapi-commands: Inline single-use helpers of gen_marshal()
2016-03-05 16:16 [Qemu-devel] [PATCH v4 00/10] easier unboxed visits/qapi implicit types Eric Blake
` (4 preceding siblings ...)
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 05/10] qapi: Utilize implicit struct visits Eric Blake
@ 2016-03-05 16:16 ` Eric Blake
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 07/10] qapi: Don't special-case simple union wrappers Eric Blake
` (3 subsequent siblings)
9 siblings, 0 replies; 36+ messages in thread
From: Eric Blake @ 2016-03-05 16:16 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>
---
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] 36+ messages in thread
* [Qemu-devel] [PATCH v4 07/10] qapi: Don't special-case simple union wrappers
2016-03-05 16:16 [Qemu-devel] [PATCH v4 00/10] easier unboxed visits/qapi implicit types Eric Blake
` (5 preceding siblings ...)
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 06/10] qapi-commands: Inline single-use helpers of gen_marshal() Eric Blake
@ 2016-03-05 16:16 ` Eric Blake
2016-03-08 15:59 ` Markus Armbruster
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 08/10] qapi: Allow anonymous base for flat union Eric Blake
` (2 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Eric Blake @ 2016-03-05 16:16 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 _obj_ImageInfoSpecificQCow2_wrapper {
| ImageInfoSpecificQCow2 *data;
| };
|
| struct _obj_ImageInfoSpecificVmdk_wrapper {
| ImageInfoSpecificVmdk *data;
| };
...
| struct ImageInfoSpecific {
| ImageInfoSpecificKind type;
| union { /* union tag is @type */
| void *data;
|- ImageInfoSpecificQCow2 *qcow2;
|- ImageInfoSpecificVmdk *vmdk;
|+ _obj_ImageInfoSpecificQCow2_wrapper qcow2;
|+ _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__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__obj_ImageInfoSpecificVmdk_wrapper_members(v, &obj->u.vmdk, &err);
| break;
| default:
| abort();
Signed-off-by: Eric Blake <eblake@redhat.com>
---
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 | 11 ------
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(+), 269 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 797ac7a..0574b8b 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1006,7 +1006,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):
@@ -1126,16 +1125,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 fa2d6af..b8499ac 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 b4303a7..afbaeeb 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -60,29 +60,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 9710321..9f7dc53 100644
--- a/net/netmap.c
+++ b/net/netmap.c
@@ -403,7 +403,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 cfb6831..b477352 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] 36+ messages in thread
* [Qemu-devel] [PATCH v4 08/10] qapi: Allow anonymous base for flat union
2016-03-05 16:16 [Qemu-devel] [PATCH v4 00/10] easier unboxed visits/qapi implicit types Eric Blake
` (6 preceding siblings ...)
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 07/10] qapi: Don't special-case simple union wrappers Eric Blake
@ 2016-03-05 16:16 ` Eric Blake
2016-03-08 16:23 ` Markus Armbruster
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 09/10] qapi: Use anonymous bases in QMP flat unions Eric Blake
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 10/10] qapi: Populate info['name'] for each entity Eric Blake
9 siblings, 1 reply; 36+ messages in thread
From: Eric Blake @ 2016-03-05 16:16 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.
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 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>
---
v4: rebase to use implicit type, improve commit message
[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 | 30 +++++++++++++++---------------
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, 40 insertions(+), 35 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 0574b8b..227f47d 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
@@ -560,9 +562,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"
@@ -1049,6 +1052,8 @@ class QAPISchemaMember(object):
owner = owner[5:]
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
@@ -1336,6 +1341,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 b8499ac..c003721 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)
if name[0] != ':':
# implicit types won't be directly allocated/freed
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index e0b2ef1..a92d4da 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,24 +320,25 @@ 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 an
-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 a common member 'readonly', renaming the discriminator to
+adding a common member 'read-only', renaming the discriminator to
something more applicable, and reducing the number of {} required on
the wire:
{ 'enum': 'BlockdevDriver', 'data': [ 'file', 'qcow2' ] }
- { 'struct': 'BlockdevCommonOptions',
- 'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } }
{ 'union': 'BlockdevOptions',
- 'base': 'BlockdevCommonOptions',
+ 'base': { 'driver': 'BlockdevDriver', 'read-only': 'bool' },
'discriminator': 'driver',
'data': { 'file': 'FileOptions',
'qcow2': 'Qcow2Options' } }
@@ -346,7 +347,7 @@ Resulting in these JSON objects:
{ "driver": "file", "readonly": true,
"filename": "/some/place/my-image" }
- { "driver": "qcow2", "readonly": false,
+ { "driver": "qcow2", "read-only": false,
"backing-file": "/some/place/my-image", "lazy-refcounts": true }
Notice that in a flat union, the discriminator name is controlled by
@@ -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 f531961..a67d375 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -8,6 +8,10 @@ object :obj-EVENT_D-arg
member b: str optional=False
member c: str optional=True
member enum3: EnumOne optional=True
+object :obj-UserDefFlatUnion2-base
+ member integer: int optional=True
+ member string: str optional=False
+ member enum1: QEnumTwo optional=False
object :obj-__org.qemu_x-command-arg
member a: __org.qemu_x-EnumList optional=False
member b: __org.qemu_x-StructList optional=False
@@ -121,7 +125,7 @@ object UserDefFlatUnion
case value2: UserDefB
case value3: UserDefB
object UserDefFlatUnion2
- base UserDefUnionBase2
+ base :obj-UserDefFlatUnion2-base
tag enum1
case value1: UserDefC
case value2: UserDefB
@@ -166,10 +170,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
--
2.5.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v4 09/10] qapi: Use anonymous bases in QMP flat unions
2016-03-05 16:16 [Qemu-devel] [PATCH v4 00/10] easier unboxed visits/qapi implicit types Eric Blake
` (7 preceding siblings ...)
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 08/10] qapi: Allow anonymous base for flat union Eric Blake
@ 2016-03-05 16:16 ` Eric Blake
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 10/10] qapi: Populate info['name'] for each entity Eric Blake
9 siblings, 0 replies; 36+ messages in thread
From: Eric Blake @ 2016-03-05 16:16 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; the next patch will address that wart.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
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 227f47d..ebcd207 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -62,8 +62,8 @@ returns_whitelist = [
# Whitelist of entities allowed to violate case conventions
case_whitelist = [
# From QMP:
+ ':obj-CpuInfo-base', # CPU, visible through query-cpu
'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
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] 36+ messages in thread
* [Qemu-devel] [PATCH v4 10/10] qapi: Populate info['name'] for each entity
2016-03-05 16:16 [Qemu-devel] [PATCH v4 00/10] easier unboxed visits/qapi implicit types Eric Blake
` (8 preceding siblings ...)
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 09/10] qapi: Use anonymous bases in QMP flat unions Eric Blake
@ 2016-03-05 16:16 ` Eric Blake
2016-03-08 16:46 ` Markus Armbruster
9 siblings, 1 reply; 36+ messages in thread
From: Eric Blake @ 2016-03-05 16:16 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Every non-implicit entity is associated with an 'info'
dictionary, but it is not easy to reverse-engineer the name of
the top-most entity associated with that 'info'. Our use of
'case_whitelist' (added in commit 893e1f2) is thus currently
tied to the owner of a member instead; but as the previous patch
showed with CpuInfo, this requires whitelist exceptions to know
how an implicit name will be generated.
While we have a ._pretty_owner() that maps from implicit names
back to a human readable phrase, that produces more than just a
plain top-level entity name. What's more, the current use of
._pretty_owner() is via .check_clash(), which can be called on
the same member object more than once (once through the
standalone type, and a second time when used as a base class of
a derived tpye); if a clash is only introduced in the derived
class, using ._pretty_owner() to report the error on behalf of
the base class named in member.owner seems wrong. Therefore,
we need a new mechanism.
Add a new info['name'] field to track the information we need,
allowing us to change 'case_whitelist' to use only names present
in the qapi files.
Unfortunately, there is no one good place to add the mapping:
at the point 'info' is created in QAPISchemaParser.__init__(),
we don't know the name. Info is then stored into
QAPISchemaParser.exprs, which then then gets fed to
QAPISchema.__init__() via the global check_exprs(), but we want
check_exprs() to go away. And QAPISchema._def_exprs() sees
every entity, except that the various _def_FOO() helpers don't
return anything. So we have to modify all of the _def_FOO()
methods.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v4: Change series again :)
[previously posted in subset F]:
v6: sink later in series, and rework commit message
[previously posted in subset D]:
v14: rearrange assignment, improve commit message
v13: new patch
---
scripts/qapi.py | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index ebcd207..6c991c3 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -62,8 +62,8 @@ returns_whitelist = [
# Whitelist of entities allowed to violate case conventions
case_whitelist = [
# From QMP:
- ':obj-CpuInfo-base', # CPU, visible through query-cpu
'ACPISlotType', # DIMM, visible through query-acpi-ospm-status
+ 'CpuInfo', # CPU and PC, visible through query-cpu
'CpuInfoMIPS', # PC, visible through query-cpu
'CpuInfoTricore', # PC, visible through query-cpu
'QapiErrorClass', # all members, visible through errors
@@ -1035,7 +1035,7 @@ class QAPISchemaMember(object):
def check_clash(self, info, seen):
cname = c_name(self.name)
- if cname.lower() != cname and self.owner not in case_whitelist:
+ if cname.lower() != cname and info['name'] not in case_whitelist:
raise QAPIExprError(info,
"%s should not use uppercase" % self.describe())
if cname in seen:
@@ -1296,7 +1296,7 @@ class QAPISchema(object):
return name
def _def_enum_type(self, expr, info):
- name = expr['enum']
+ name = info['name'] = expr['enum']
data = expr['data']
prefix = expr.get('prefix')
self._def_entity(QAPISchemaEnumType(
@@ -1317,7 +1317,7 @@ class QAPISchema(object):
for (key, value) in data.iteritems()]
def _def_struct_type(self, expr, info):
- name = expr['struct']
+ name = info['name'] = expr['struct']
base = expr.get('base')
data = expr['data']
self._def_entity(QAPISchemaObjectType(name, info, base,
@@ -1336,7 +1336,7 @@ class QAPISchema(object):
return QAPISchemaObjectTypeVariant(case, typ)
def _def_union_type(self, expr, info):
- name = expr['union']
+ name = info['name'] = expr['union']
data = expr['data']
base = expr.get('base')
tag_name = expr.get('discriminator')
@@ -1362,7 +1362,7 @@ class QAPISchema(object):
variants)))
def _def_alternate_type(self, expr, info):
- name = expr['alternate']
+ name = info['name'] = expr['alternate']
data = expr['data']
variants = [self._make_variant(key, value)
for (key, value) in data.iteritems()]
@@ -1374,7 +1374,7 @@ class QAPISchema(object):
variants)))
def _def_command(self, expr, info):
- name = expr['command']
+ name = info['name'] = expr['command']
data = expr.get('data')
rets = expr.get('returns')
gen = expr.get('gen', True)
@@ -1389,7 +1389,7 @@ class QAPISchema(object):
success_response))
def _def_event(self, expr, info):
- name = expr['event']
+ name = info['name'] = expr['event']
data = expr.get('data')
if isinstance(data, OrderedDict):
data = self._make_implicit_object_type(
--
2.5.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 01/10] qapi: Assert in places where variants are not handled
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 01/10] qapi: Assert in places where variants are not handled Eric Blake
@ 2016-03-08 10:12 ` Markus Armbruster
2016-03-08 15:49 ` Eric Blake
0 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2016-03-08 10:12 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> 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: base types must still be plain structs, and anywhere we
> explode a struct into a parameter list (events and command
> marshalling), we don't support variants in that explosion.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v4: new patch, split out from .is_empty() patch
> ---
> scripts/qapi.py | 1 +
> scripts/qapi-commands.py | 3 ++-
> scripts/qapi-event.py | 1 +
> 3 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 6b2aa6e..dc26ef9 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -960,6 +960,7 @@ class QAPISchemaObjectType(QAPISchemaType):
> assert isinstance(self.base, QAPISchemaObjectType)
> self.base.check(schema)
> self.base.check_clash(schema, self.info, seen)
> + assert not self.base.variants
I'd move this two lines up, so it's next to the isinstance.
Assertions in .check() are place-holders for semantic checks that
haven't been moved from the old semantic analysis to the classes.
Whenever we add one, we should double-check the old semantic analysis
catches whatever we assert. For object types, that's check_struct() and
check_union(). Both check_type() the base with allow_metas=['struct']),
so we're good.
Inconsistency: you add the check for base, but not for variants.
On closer look, adding it for either is actually redundant, because
se.f.base.check_clash() already asserts it, with a nice "not
implemented" comment.
If we think asserting twice is useful for base, then it's useful for
variants, too. But I think asserting once suffices.
While reviewing use of .check_clash(), I got quite confused by
QAPISchemaAlternateType().check():
def check(self, schema):
self.variants.tag_member.check(schema)
# Not calling self.variants.check_clash(), because there's nothing
# to clash with
self.variants.check(schema, {})
# Alternate branch names have no relation to the tag enum values;
# so we have to check for potential name collisions ourselves.
seen = {}
for v in self.variants.variants:
v.check_clash(self.info, seen)
Commit b807a1e added the "Not calling self.variants.check_clash()"
comment.
Commit 0426d53 added the loop with its comment. Which on first glance
looks like the loop in self.variants.check_clash()! But it's really
different, namely:
for v in self.variants.variants:
# Reset seen map for each variant, since qapi names from one
# branch do not affect another branch
assert isinstance(v.type, QAPISchemaObjectType)
v.type.check_clash(schema, info, dict(seen))
i.e. it calls QAPISchemaObjectType.check_clash() to check for clashes
between each variant's members and the common members in seen, whereas
the other loop calls QAPISchemaObjectTypeVariant.check_clash(), which is
really QAPISchemaMember.check_clash(), to check for clashes between
variant names.
Nice little OO maze we've built there %-}
> for m in self.local_members:
> m.check(schema)
> m.check_clash(self.info, seen)
> 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)
Assertions in generators need to be protected by semantic checks in
.check(). The .check() can either do it themselves, or assert and
delegate to the old semantic analysis. Whenever we add an assertion to
a generator, we should double-check it's protected properly.
gen_call() is used only by QAPISchemaGenCommandVisitor.visit_command()
via gen_marshal(), so QAPISchemaCommand.check() is responsible for
protecting. It has a "not implemented" assertion, i.e. it delegates to
check_command(). That one check_type()s the argument with
allow_metas=['struct'], so we're good.
> 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);
gen_event_send() is used only by
QAPISchemaGenEventVisitor.visit_event(), so QAPISchemaEvent.check() is
responsible for protecting. It has a "not implemented" assertion,
i.e. it delegates to check_event(). That one check_type()s the argument
with allow_metas=['struct'], so we're good.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 03/10] qapi: Make c_type() more OO-like
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 03/10] qapi: Make c_type() more OO-like Eric Blake
@ 2016-03-08 10:54 ` Markus Armbruster
2016-03-08 15:50 ` Eric Blake
0 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2016-03-08 10:54 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> QAPISchemaType.c_type() was a bit awkward. Rather than having two
> optional orthogonal boolean flags that should never both be true,
> and where all callers should pass a compile-time constant (well,
> our use of is_unboxed wasn't constant, but a future patch is about
> to remove the special case for simple unions, so we can live with
> the churn of refactoring the code in the meantime), the more
> object-oriented approach uses different method names that can be
> overridden as needed, and which convey the intent by the method
> name. The caller just makes sure to use the right variant, rather
> than having to worry about boolean flags.
>
> It requires slightly more Python, but is arguably easier to read.
The second sentence is rather long. Suggest:
QAPISchemaType.c_type() is a bit awkward: it takes two optional
boolean flags is_param and is_unboxed that should never both be
true.
Add a new method for each of the flags, and drop the flags from
c_type().
Most calls pass no flags. They remain unchanged.
One call passes is_param=True. Call new .c_param_type() instead.
One call passes is_unboxed=True except for simple union types. This
is actually an ugly special case that should go away soon. Until
then, we now have to call either .c_type() or the new
.c_unboxed_type(). Tolerable.
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
Patch looks good.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 04/10] qapi: Emit implicit structs in generated C
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 04/10] qapi: Emit implicit structs in generated C Eric Blake
@ 2016-03-08 14:24 ` Markus Armbruster
2016-03-08 16:03 ` Eric Blake
0 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2016-03-08 14:24 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.
Yes.
> 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[0] == ':'" feels a bit hacky, but beats changing the
> signature of the visit_object_type() callback to pass a new
> 'implicit' flag.
PRO: it matches what QAPISchemaObjectType.is_implicit() does.
CON: it duplicates what QAPISchemaObjectType.is_implicit() does.
Ways to adhere to DRY:
(1) Add a flag to visit_object_type() and, for consistency, to
visit_object_type_flat().
(2) Change the QAPISchemaVisitor.visit_FOO() to take a full FOO instead
of FOO.name, FOO.info and selected other members.
We've discussed (2) elsewhere. The QAPISchemaEntity.visit() shrink to a
mere double-dispatch. The QAPISchemaVisitor gain full access to the
things they visit. The interface between the generators and the
internal representation changes from a narrow, explicit and inflexible
one to a much wider "anything goes" one. Both the narrow and the wide
interface have advantages and disadvantages. Have we outgrown the
narrow one?
> 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 ':empty' 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 fact that all our implicit objects start
> with a leading ':', which can be transliteratated to a leading
transliterated
> single underscore, and we've already documented and enforce that
Uh, these are "always reserved for use as identifiers with file scope"
(C99 7.1.3). I'm afraid we need to use the q_ prefix.
> the user can't create QAPI names with a leading underscore, so
> exposing the types won't create any collisions. It is a bit
> unfortunate that the generated struct names don't match normal
> naming conventions, but it's not too bad, since they will only
> be used in generated code.
The problem is self-inflicted: we make up these names in
_make_implicit_object_type(). We could just as well make up names that
come out prettier.
In fact, my first versions of the code had names starting with ':' for
*all* implicit types. I abandoned that for enum and array types when I
realized I need C names for them, and the easiest way to get them making
up names so that a trivial .c_name() works. We can do the same for
object types.
> The generated code grows substantially in size; in part because
> it was easier to output every implicit type rather than adding
> generator complexity to try and only output types as needed.
I happily trade larger .c files the optimizer can reduce for a simpler
generator. I'm less sanguine about larger .h files when they get
included a lot. qapi-types.h gets included basically everywhere, and
grows from ~4000 to ~5250 lines. How much of this is avoidable?
> Signed-off-by: Eric Blake <eblake@redhat.com>
Can't find fault with the patch itself.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 05/10] qapi: Utilize implicit struct visits
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 05/10] qapi: Utilize implicit struct visits Eric Blake
@ 2016-03-08 15:10 ` Markus Armbruster
2016-03-08 16:11 ` Eric Blake
0 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2016-03-08 15:10 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, such as:
>
> |@@ -381,6 +293,9 @@ void qapi_event_send_block_job_error(con
> | QmpOutputVisitor *qov;
> | Visitor *v;
> | QObject *obj;
> |+ _obj_BLOCK_JOB_ERROR_arg qapi = {
> |+ (char *)device, operation, action
> |+ };
> |
> | emit = qmp_event_get_func_emit();
> | if (!emit) {
> |@@ -396,19 +311,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__obj_BLOCK_JOB_ERROR_arg_members(v, &qapi, &err);
> | visit_end_struct(v, err ? NULL : &err);
>
> And command marshalling generates call arguments from a stack-allocated
> struct:
I see qmp-marshal.c shrink from ~5700 lines to ~4300. Neat!
>
> |@@ -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;
> |- }
> |+ _obj_add_fd_arg qapi = {0};
> |+
> |+ v = qmp_input_get_visitor(qiv);
> |+ visit_type__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__obj_add_fd_arg_members(v, &qapi, NULL);
> | qapi_dealloc_visitor_cleanup(qdv);
> | }
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v4: new patch
> ---
> scripts/qapi.py | 25 +++++++++++++++++++++++++
> scripts/qapi-commands.py | 28 ++++++++++++----------------
> scripts/qapi-event.py | 16 +++++++---------
> 3 files changed, 44 insertions(+), 25 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 6cf0a75..797ac7a 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -1640,6 +1640,7 @@ extern const char *const %(c_name)s_lookup[];
> return ret
>
>
> +# Explode arg_type into a parameter list, along with extra parameters
> def gen_params(arg_type, extra):
> if not arg_type:
> return extra
> @@ -1657,6 +1658,30 @@ def gen_params(arg_type, extra):
> return ret
>
>
> +# Declare and initialize an object 'qapi' using parameters from gen_params()
> +def gen_struct_init(typ):
It's not just a "struct init", it's a variable declaration with an
initializer. gen_param_var()?
Name the variable param rather than qapi?
> + assert not typ.variants
> + ret = mcgen('''
> + %(c_name)s qapi = {
> +''',
> + 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 *)'
Why is that useful?
> + ret += c_name(memb.name)
> + ret += mcgen('''
> +
> + };
> +''')
> + return ret
Odd: you use this only in qapi-event.py, not in qapi-commands.py.
Should it live in qapi-event.py then?
> +
> +
> def gen_err_check(label='out', skiperr=False):
> if skiperr:
> return ''
> 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:
This is necessary, because...
> @@ -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('''
... you wrap the parameter variables in a struct here. Okay.
>
> @@ -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)
Unless I'm mistaken, this is the only use of skiperr. Follow up with a
patch to drop the parameter and simplify?
> + 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 c03cb78..627e9a0 100644
> --- a/scripts/qapi-event.py
> +++ b/scripts/qapi-event.py
> @@ -44,8 +44,8 @@ def gen_event_send(name, arg_type):
> QmpOutputVisitor *qov;
> Visitor *v;
> QObject *obj;
> -
> ''')
> + ret += gen_struct_init(arg_type) + '\n'
>
> ret += mcgen('''
> emit = qmp_event_get_func_emit();
> @@ -65,13 +65,10 @@ def gen_event_send(name, arg_type):
> 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, &qapi, &err);
> visit_end_struct(v, err ? NULL : &err);
> if (err) {
> goto out;
> @@ -81,7 +78,8 @@ out_obj:
> g_assert(obj);
>
> qdict_put_obj(qmp, "data", obj);
> -''')
> +''',
> + name=name, c_name=arg_type.c_name())
>
> ret += mcgen('''
> emit(%(c_enum)s, qmp, &err);
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 01/10] qapi: Assert in places where variants are not handled
2016-03-08 10:12 ` Markus Armbruster
@ 2016-03-08 15:49 ` Eric Blake
2016-03-08 17:46 ` Markus Armbruster
0 siblings, 1 reply; 36+ messages in thread
From: Eric Blake @ 2016-03-08 15:49 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 2041 bytes --]
On 03/08/2016 03:12 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
>> 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: base types must still be plain structs, and anywhere we
>> explode a struct into a parameter list (events and command
>> marshalling), we don't support variants in that explosion.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>
>> +++ b/scripts/qapi.py
>> @@ -960,6 +960,7 @@ class QAPISchemaObjectType(QAPISchemaType):
>> assert isinstance(self.base, QAPISchemaObjectType)
>> self.base.check(schema)
>> self.base.check_clash(schema, self.info, seen)
>> + assert not self.base.variants
>
> I'd move this two lines up, so it's next to the isinstance.
>
> Assertions in .check() are place-holders for semantic checks that
> haven't been moved from the old semantic analysis to the classes.
> Whenever we add one, we should double-check the old semantic analysis
> catches whatever we assert. For object types, that's check_struct() and
> check_union(). Both check_type() the base with allow_metas=['struct']),
> so we're good.
>
> Inconsistency: you add the check for base, but not for variants.
>
> On closer look, adding it for either is actually redundant, because
> se.f.base.check_clash() already asserts it, with a nice "not
> implemented" comment.
>
> If we think asserting twice is useful for base, then it's useful for
> variants, too. But I think asserting once suffices.
So basically, we can drop this hunk, right?
--
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] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 03/10] qapi: Make c_type() more OO-like
2016-03-08 10:54 ` Markus Armbruster
@ 2016-03-08 15:50 ` Eric Blake
0 siblings, 0 replies; 36+ messages in thread
From: Eric Blake @ 2016-03-08 15:50 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 1813 bytes --]
On 03/08/2016 03:54 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
>> QAPISchemaType.c_type() was a bit awkward. Rather than having two
>> optional orthogonal boolean flags that should never both be true,
>> and where all callers should pass a compile-time constant (well,
>> our use of is_unboxed wasn't constant, but a future patch is about
>> to remove the special case for simple unions, so we can live with
>> the churn of refactoring the code in the meantime), the more
>> object-oriented approach uses different method names that can be
>> overridden as needed, and which convey the intent by the method
>> name. The caller just makes sure to use the right variant, rather
>> than having to worry about boolean flags.
>>
>> It requires slightly more Python, but is arguably easier to read.
>
> The second sentence is rather long. Suggest:
>
> QAPISchemaType.c_type() is a bit awkward: it takes two optional
> boolean flags is_param and is_unboxed that should never both be
> true.
>
> Add a new method for each of the flags, and drop the flags from
> c_type().
>
> Most calls pass no flags. They remain unchanged.
>
> One call passes is_param=True. Call new .c_param_type() instead.
>
> One call passes is_unboxed=True except for simple union types. This
> is actually an ugly special case that should go away soon. Until
> then, we now have to call either .c_type() or the new
> .c_unboxed_type(). Tolerable.
Yes, that works for me.
>
>> Suggested-by: Markus Armbruster <armbru@redhat.com>
>> 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] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 07/10] qapi: Don't special-case simple union wrappers
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 07/10] qapi: Don't special-case simple union wrappers Eric Blake
@ 2016-03-08 15:59 ` Markus Armbruster
2016-03-08 16:16 ` Eric Blake
0 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2016-03-08 15:59 UTC (permalink / raw)
To: Eric Blake
Cc: Kevin Wolf, Fam Zheng, Eduardo Habkost,
open list:Block layer core, Michael S. Tsirkin, Michael Roth,
Jan Kiszka, Jason Wang, qemu-devel, Vincenzo Maffione,
Luiz Capitulino, Gerd Hoffmann, Paolo Bonzini, Igor Mammedov,
Giuseppe Lettieri, Luigi Rizzo, Samuel Thibault
Eric Blake <eblake@redhat.com> writes:
> 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 _obj_ImageInfoSpecificQCow2_wrapper {
> | ImageInfoSpecificQCow2 *data;
> | };
> |
> | struct _obj_ImageInfoSpecificVmdk_wrapper {
> | ImageInfoSpecificVmdk *data;
> | };
> ...
> | struct ImageInfoSpecific {
> | ImageInfoSpecificKind type;
> | union { /* union tag is @type */
> | void *data;
> |- ImageInfoSpecificQCow2 *qcow2;
> |- ImageInfoSpecificVmdk *vmdk;
> |+ _obj_ImageInfoSpecificQCow2_wrapper qcow2;
> |+ _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__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__obj_ImageInfoSpecificVmdk_wrapper_members(v, &obj->u.vmdk, &err);
> | break;
> | default:
> | abort();
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> 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 | 11 ------
> 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(+), 269 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 797ac7a..0574b8b 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -1006,7 +1006,6 @@ class QAPISchemaObjectType(QAPISchemaType):
> return c_name(self.name) + pointer_suffix
>
> def c_unboxed_type(self):
> - assert not self.is_implicit()
Doesn't this belong into PATCH 04?
> return c_name(self.name)
>
> def json_type(self):
> @@ -1126,16 +1125,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 fa2d6af..b8499ac 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 b4303a7..afbaeeb 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -60,29 +60,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:
This stupid special case has given us enough trouble, good to see it
gone!
[Tedious part snipped, it looks good to me...]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 04/10] qapi: Emit implicit structs in generated C
2016-03-08 14:24 ` Markus Armbruster
@ 2016-03-08 16:03 ` Eric Blake
2016-03-08 19:09 ` Markus Armbruster
0 siblings, 1 reply; 36+ messages in thread
From: Eric Blake @ 2016-03-08 16:03 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 5651 bytes --]
On 03/08/2016 07:24 AM, Markus Armbruster wrote:
> 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.
>
> Yes.
>
>> 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[0] == ':'" feels a bit hacky, but beats changing the
>> signature of the visit_object_type() callback to pass a new
>> 'implicit' flag.
>
> PRO: it matches what QAPISchemaObjectType.is_implicit() does.
>
> CON: it duplicates what QAPISchemaObjectType.is_implicit() does.
>
> Ways to adhere to DRY:
>
> (1) Add a flag to visit_object_type() and, for consistency, to
> visit_object_type_flat().
>
> (2) Change the QAPISchemaVisitor.visit_FOO() to take a full FOO instead
> of FOO.name, FOO.info and selected other members.
>
> We've discussed (2) elsewhere. The QAPISchemaEntity.visit() shrink to a
> mere double-dispatch. The QAPISchemaVisitor gain full access to the
> things they visit. The interface between the generators and the
> internal representation changes from a narrow, explicit and inflexible
> one to a much wider "anything goes" one. Both the narrow and the wide
> interface have advantages and disadvantages. Have we outgrown the
> narrow one?
Your argument that the narrow view forces us to think about things may
still hold. I'm border-line on whether the switch is worth it, vs.
adding flags as the visitors want to learn more and more about what they
are visiting without having the actual Python object in hand.
I think what would sway me over the fence is looking at some of our
constructs: for example, qapi-types.py has gen_object() which it now
calls recursively. When called directly from visit_object_type(), we
have all the pieces; when called from visit_alternate_type(), we have to
create a 1-element members array; and when called recursively, we have
to manually explode base into the four pieces.
>
>> 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 ':empty' 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 fact that all our implicit objects start
>> with a leading ':', which can be transliteratated to a leading
>
> transliterated
>
>> single underscore, and we've already documented and enforce that
>
> Uh, these are "always reserved for use as identifiers with file scope"
> (C99 7.1.3). I'm afraid we need to use the q_ prefix.
>
>> the user can't create QAPI names with a leading underscore, so
>> exposing the types won't create any collisions. It is a bit
>> unfortunate that the generated struct names don't match normal
>> naming conventions, but it's not too bad, since they will only
>> be used in generated code.
>
> The problem is self-inflicted: we make up these names in
> _make_implicit_object_type(). We could just as well make up names that
> come out prettier.
Any suggestions? It seems easy enough to change, if we have something
that we like.
>
> In fact, my first versions of the code had names starting with ':' for
> *all* implicit types. I abandoned that for enum and array types when I
> realized I need C names for them, and the easiest way to get them making
> up names so that a trivial .c_name() works. We can do the same for
> object types.
>
>> The generated code grows substantially in size; in part because
>> it was easier to output every implicit type rather than adding
>> generator complexity to try and only output types as needed.
>
> I happily trade larger .c files the optimizer can reduce for a simpler
> generator. I'm less sanguine about larger .h files when they get
> included a lot. qapi-types.h gets included basically everywhere, and
> grows from ~4000 to ~5250 lines. How much of this is avoidable?
Not much: remember, because we use unboxed types in unions, all -wrapper
structs have to be present in the .h for the compiler to not complain
about incomplete types.
For -arg types (and for upcoming -base types in 8/10), we could get away
with only forward declarations of the type in the .h: the
visit_type_ARG_members() function uses only a pointer so it can live
with an incomplete type in the header and a complete type in a private
helper file or in the .c. But we have fewer -arg classes in comparison
to the -wrapper classes, which means splitting on those lines would add
a lot of complexity to the generator for very little .h savings.
--
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] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 05/10] qapi: Utilize implicit struct visits
2016-03-08 15:10 ` Markus Armbruster
@ 2016-03-08 16:11 ` Eric Blake
2016-03-08 18:09 ` Markus Armbruster
2016-03-09 23:28 ` Eric Blake
0 siblings, 2 replies; 36+ messages in thread
From: Eric Blake @ 2016-03-08 16:11 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 3607 bytes --]
On 03/08/2016 08:10 AM, 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.
>>
>> Generated code shrinks accordingly; events initialize a struct
>> based on parameters, such as:
>>
>>
>> And command marshalling generates call arguments from a stack-allocated
>> struct:
>
> I see qmp-marshal.c shrink from ~5700 lines to ~4300. Neat!
Yeah, it nicely balances out the .h getting so much larger, except that
the .h gets parsed a lot more by the compiler.
>>
>> +# Declare and initialize an object 'qapi' using parameters from gen_params()
>> +def gen_struct_init(typ):
>
> It's not just a "struct init", it's a variable declaration with an
> initializer. gen_param_var()?
>
> Name the variable param rather than qapi?
Sure, I'm not tied to a specific name. I will point out that we have a
potential collision point - any local variable we create here can
collide with members of the QAPI struct passed to the event. We haven't
hit the collision on any events we've created so far, and it's easy to
rename our local variables at such time if we do run into the collision
down the road, so I won't worry about it now.
>
>> + assert not typ.variants
>> + ret = mcgen('''
>> + %(c_name)s qapi = {
>> +''',
>> + 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 *)'
>
> Why is that useful?
The compiler complains if you try to initialize a 'char *' member of a
QAPI C struct with a 'const char *' parameter. It's no different than
the fact that the gen_visit_members() call we are getting rid of also
has to cast away const.
>
>> + ret += c_name(memb.name)
>> + ret += mcgen('''
>> +
>> + };
>> +''')
>> + return ret
>
> Odd: you use this only in qapi-event.py, not in qapi-commands.py.
> Should it live in qapi-event.py then?
Yeah, I guess so. I originally put it in qapi.py thinking that I could
reuse it in qapi-commands.py, then realized that the two are opposite
(event collects parameters into a struct, commands parses a QObject into
parameters), so I couldn't share it after all.
>> @@ -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)
>
> Unless I'm mistaken, this is the only use of skiperr. Follow up with a
> patch to drop the parameter and simplify?
Oh, nice. I noticed some cleanups in patch 6/10, but missed this one as
a reasonable improvement.
In fact, gen_visit_members() is now only used in qapi-visits.py, so
maybe I can move it back there (it used to live there before commit
82ca8e469 moved it for sharing with the two files simplified here).
--
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] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 07/10] qapi: Don't special-case simple union wrappers
2016-03-08 15:59 ` Markus Armbruster
@ 2016-03-08 16:16 ` Eric Blake
2016-03-08 18:10 ` Markus Armbruster
0 siblings, 1 reply; 36+ messages in thread
From: Eric Blake @ 2016-03-08 16:16 UTC (permalink / raw)
To: Markus Armbruster
Cc: Kevin Wolf, Fam Zheng, Eduardo Habkost,
open list:Block layer core, Michael S. Tsirkin, Michael Roth,
Jan Kiszka, Jason Wang, qemu-devel, Vincenzo Maffione,
Luiz Capitulino, Gerd Hoffmann, Paolo Bonzini, Igor Mammedov,
Giuseppe Lettieri, Luigi Rizzo, Samuel Thibault
[-- Attachment #1: Type: text/plain, Size: 1918 bytes --]
On 03/08/2016 08:59 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
>> 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:
>>
>> +++ b/scripts/qapi.py
>> @@ -1006,7 +1006,6 @@ class QAPISchemaObjectType(QAPISchemaType):
>> return c_name(self.name) + pointer_suffix
>>
>> def c_unboxed_type(self):
>> - assert not self.is_implicit()
>
> Doesn't this belong into PATCH 04?
>
>> return c_name(self.name)
Maybe. Patch 3 kept the assertion out of straight code refactoring, and
patch 4 didn't use c_unboxed_type(), so this was the first place where I
had to weaken the assertion. But moving it into patch 4 doesn't seem
like it would hurt, as it is still semantically related to the fact that
we are planning on allowing an unboxed implicit type.
>> - 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:
>
> This stupid special case has given us enough trouble, good to see it
> gone!
Yeah, it was a nice feeling to get to this point!
--
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] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 08/10] qapi: Allow anonymous base for flat union
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 08/10] qapi: Allow anonymous base for flat union Eric Blake
@ 2016-03-08 16:23 ` Markus Armbruster
2016-03-08 16:29 ` Eric Blake
0 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2016-03-08 16:23 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.
>
> 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 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>
>
> ---
> v4: rebase to use implicit type, improve commit message
> [no v3]
I think you also
* fixed a missing allow_optional=True in check_union()
* fixed a missing "non-optional" in qapi-code-gen.txt (mention in commit
message? separate patch?)
* renamed readonly to read-only in an example in qapi-code-gen.txt to be
closer to the code (separate patch?)
> 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 | 30 +++++++++++++++---------------
> 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, 40 insertions(+), 35 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 0574b8b..227f47d 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
> @@ -560,9 +562,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'])
This is where you added allow_optional=True.
> if not base:
> raise QAPIExprError(expr_info,
> "Flat union '%s' must have a base"
> @@ -1049,6 +1052,8 @@ class QAPISchemaMember(object):
> owner = owner[5:]
> 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
> @@ -1336,6 +1341,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 b8499ac..c003721 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)
> if name[0] != ':':
> # implicit types won't be directly allocated/freed
> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index e0b2ef1..a92d4da 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,24 +320,25 @@ 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 an
> -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
This is where you supplied the missing "non-optional".
> +member of the base struct.
>
And below, you rename readonly to read-only.
> The following example enhances the above simple union example by
> -adding a common member 'readonly', renaming the discriminator to
> +adding a common member 'read-only', renaming the discriminator to
> something more applicable, and reducing the number of {} required on
> the wire:
>
> { 'enum': 'BlockdevDriver', 'data': [ 'file', 'qcow2' ] }
> - { 'struct': 'BlockdevCommonOptions',
> - 'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } }
> { 'union': 'BlockdevOptions',
> - 'base': 'BlockdevCommonOptions',
> + 'base': { 'driver': 'BlockdevDriver', 'read-only': 'bool' },
> 'discriminator': 'driver',
> 'data': { 'file': 'FileOptions',
> 'qcow2': 'Qcow2Options' } }
> @@ -346,7 +347,7 @@ Resulting in these JSON objects:
>
> { "driver": "file", "readonly": true,
> "filename": "/some/place/my-image" }
> - { "driver": "qcow2", "readonly": false,
> + { "driver": "qcow2", "read-only": false,
> "backing-file": "/some/place/my-image", "lazy-refcounts": true }
>
> Notice that in a flat union, the discriminator name is controlled by
> @@ -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' } }
>
>
[Tests look good...]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 08/10] qapi: Allow anonymous base for flat union
2016-03-08 16:23 ` Markus Armbruster
@ 2016-03-08 16:29 ` Eric Blake
2016-03-08 18:14 ` Markus Armbruster
0 siblings, 1 reply; 36+ messages in thread
From: Eric Blake @ 2016-03-08 16:29 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 2791 bytes --]
On 03/08/2016 09:23 AM, 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.
>>
>
> I think you also
>
> * fixed a missing allow_optional=True in check_union()
More on that below.
>
> * fixed a missing "non-optional" in qapi-code-gen.txt (mention in commit
> message? separate patch?)
>
> * renamed readonly to read-only in an example in qapi-code-gen.txt to be
> closer to the code (separate patch?)
Could separate those two cleanups if it helps.
>> @@ -560,9 +562,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'])
>
> This is where you added allow_optional=True.
allow_optional only matters if allow_dict is True. We have places where
we allow a dict but do not allow optionals (namely, simple unions); but
where we are creating an anonymous type, we want to allow optionals at
the same time we allow a dict. I think this is just a case where the
commit message needs to be beefed up.
>> +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
>
> This is where you supplied the missing "non-optional".
>
>> +member of the base struct.
>>
>
> And below, you rename readonly to read-only.
They touch the same paragraph, but I can separate them if it would make
this patch feel cleaner.
--
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] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 10/10] qapi: Populate info['name'] for each entity
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 10/10] qapi: Populate info['name'] for each entity Eric Blake
@ 2016-03-08 16:46 ` Markus Armbruster
2016-03-08 16:59 ` Eric Blake
0 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2016-03-08 16:46 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> Every non-implicit entity is associated with an 'info'
> dictionary, but it is not easy to reverse-engineer the name of
> the top-most entity associated with that 'info'. Our use of
> 'case_whitelist' (added in commit 893e1f2) is thus currently
> tied to the owner of a member instead; but as the previous patch
> showed with CpuInfo, this requires whitelist exceptions to know
> how an implicit name will be generated.
Why is that a problem?
> While we have a ._pretty_owner() that maps from implicit names
> back to a human readable phrase, that produces more than just a
> plain top-level entity name. What's more, the current use of
> ._pretty_owner() is via .check_clash(), which can be called on
> the same member object more than once (once through the
> standalone type, and a second time when used as a base class of
> a derived tpye); if a clash is only introduced in the derived
> class, using ._pretty_owner() to report the error on behalf of
> the base class named in member.owner seems wrong. Therefore,
> we need a new mechanism.
Now I'm confused. Are you fixing suboptimal error messages?
If yes, can you show the message before & after the patch?
> Add a new info['name'] field to track the information we need,
> allowing us to change 'case_whitelist' to use only names present
> in the qapi files.
>
> Unfortunately, there is no one good place to add the mapping:
> at the point 'info' is created in QAPISchemaParser.__init__(),
> we don't know the name. Info is then stored into
> QAPISchemaParser.exprs, which then then gets fed to
> QAPISchema.__init__() via the global check_exprs(), but we want
> check_exprs() to go away. And QAPISchema._def_exprs() sees
> every entity, except that the various _def_FOO() helpers don't
> return anything. So we have to modify all of the _def_FOO()
> methods.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v4: Change series again :)
> [previously posted in subset F]:
> v6: sink later in series, and rework commit message
> [previously posted in subset D]:
> v14: rearrange assignment, improve commit message
> v13: new patch
> ---
> scripts/qapi.py | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index ebcd207..6c991c3 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -62,8 +62,8 @@ returns_whitelist = [
> # Whitelist of entities allowed to violate case conventions
> case_whitelist = [
> # From QMP:
> - ':obj-CpuInfo-base', # CPU, visible through query-cpu
> 'ACPISlotType', # DIMM, visible through query-acpi-ospm-status
> + 'CpuInfo', # CPU and PC, visible through query-cpu
> 'CpuInfoMIPS', # PC, visible through query-cpu
> 'CpuInfoTricore', # PC, visible through query-cpu
> 'QapiErrorClass', # all members, visible through errors
> @@ -1035,7 +1035,7 @@ class QAPISchemaMember(object):
>
> def check_clash(self, info, seen):
> cname = c_name(self.name)
> - if cname.lower() != cname and self.owner not in case_whitelist:
> + if cname.lower() != cname and info['name'] not in case_whitelist:
> raise QAPIExprError(info,
> "%s should not use uppercase" % self.describe())
> if cname in seen:
Doesn't look like you're touching the error message: QAPIExprError()
doesn't use info['name'].
> @@ -1296,7 +1296,7 @@ class QAPISchema(object):
> return name
>
> def _def_enum_type(self, expr, info):
> - name = expr['enum']
> + name = info['name'] = expr['enum']
> data = expr['data']
> prefix = expr.get('prefix')
> self._def_entity(QAPISchemaEnumType(
> @@ -1317,7 +1317,7 @@ class QAPISchema(object):
> for (key, value) in data.iteritems()]
>
> def _def_struct_type(self, expr, info):
> - name = expr['struct']
> + name = info['name'] = expr['struct']
> base = expr.get('base')
> data = expr['data']
> self._def_entity(QAPISchemaObjectType(name, info, base,
> @@ -1336,7 +1336,7 @@ class QAPISchema(object):
> return QAPISchemaObjectTypeVariant(case, typ)
>
> def _def_union_type(self, expr, info):
> - name = expr['union']
> + name = info['name'] = expr['union']
> data = expr['data']
> base = expr.get('base')
> tag_name = expr.get('discriminator')
> @@ -1362,7 +1362,7 @@ class QAPISchema(object):
> variants)))
>
> def _def_alternate_type(self, expr, info):
> - name = expr['alternate']
> + name = info['name'] = expr['alternate']
> data = expr['data']
> variants = [self._make_variant(key, value)
> for (key, value) in data.iteritems()]
> @@ -1374,7 +1374,7 @@ class QAPISchema(object):
> variants)))
>
> def _def_command(self, expr, info):
> - name = expr['command']
> + name = info['name'] = expr['command']
> data = expr.get('data')
> rets = expr.get('returns')
> gen = expr.get('gen', True)
> @@ -1389,7 +1389,7 @@ class QAPISchema(object):
> success_response))
>
> def _def_event(self, expr, info):
> - name = expr['event']
> + name = info['name'] = expr['event']
> data = expr.get('data')
> if isinstance(data, OrderedDict):
> data = self._make_implicit_object_type(
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 10/10] qapi: Populate info['name'] for each entity
2016-03-08 16:46 ` Markus Armbruster
@ 2016-03-08 16:59 ` Eric Blake
2016-03-08 19:14 ` Markus Armbruster
0 siblings, 1 reply; 36+ messages in thread
From: Eric Blake @ 2016-03-08 16:59 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 2709 bytes --]
On 03/08/2016 09:46 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
>> Every non-implicit entity is associated with an 'info'
>> dictionary, but it is not easy to reverse-engineer the name of
>> the top-most entity associated with that 'info'. Our use of
>> 'case_whitelist' (added in commit 893e1f2) is thus currently
>> tied to the owner of a member instead; but as the previous patch
>> showed with CpuInfo, this requires whitelist exceptions to know
>> how an implicit name will be generated.
>
> Why is that a problem?
Not necessarily a bad problem, but a bit annoying. If a developer
modifies a .json file and adds an improper name, then they will get an
error message that tells them that they need to fix their naming
conventions (hmm, the error message doesn't even point them to the
whitelist - see args-member-case.err). If the developer figures out
that the whitelist will let them avoid the error, they still have to
figure _what_ name to add to the whitelist, and without this patch, they
have to determine the generated name for the implicit struct (which may
not be constant, since we are discussing about alternative names earlier
in the series other than something that flattens to a public 'struct
_obj_...' in violation of file-local naming scope). The goal is thus to
make the whitelist tied only to names mentioned in the .json file,
rather than dragging generated implicit names into the mix.
But it is cosmetic; we could live without the patch and stick to
generated names in the whitelist, just as easily.
>
>> While we have a ._pretty_owner() that maps from implicit names
>> back to a human readable phrase, that produces more than just a
>> plain top-level entity name. What's more, the current use of
>> ._pretty_owner() is via .check_clash(), which can be called on
>> the same member object more than once (once through the
>> standalone type, and a second time when used as a base class of
>> a derived tpye); if a clash is only introduced in the derived
>> class, using ._pretty_owner() to report the error on behalf of
>> the base class named in member.owner seems wrong. Therefore,
>> we need a new mechanism.
>
> Now I'm confused. Are you fixing suboptimal error messages?
No, I was trying to document why ._pretty_owner() (which is our only
pre-exisiting map from Python objects back to pretty type names) is
insufficient for the task at hand (namely, what is the pretty name of
the type to add to the whitelist, if we don't want generated implicit
names in the whitelist).
--
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] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 01/10] qapi: Assert in places where variants are not handled
2016-03-08 15:49 ` Eric Blake
@ 2016-03-08 17:46 ` Markus Armbruster
0 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2016-03-08 17:46 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> On 03/08/2016 03:12 AM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>>
>>> 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: base types must still be plain structs, and anywhere we
>>> explode a struct into a parameter list (events and command
>>> marshalling), we don't support variants in that explosion.
>>>
>>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>>
>
>>> +++ b/scripts/qapi.py
>>> @@ -960,6 +960,7 @@ class QAPISchemaObjectType(QAPISchemaType):
>>> assert isinstance(self.base, QAPISchemaObjectType)
>>> self.base.check(schema)
>>> self.base.check_clash(schema, self.info, seen)
>>> + assert not self.base.variants
>>
>> I'd move this two lines up, so it's next to the isinstance.
>>
>> Assertions in .check() are place-holders for semantic checks that
>> haven't been moved from the old semantic analysis to the classes.
>> Whenever we add one, we should double-check the old semantic analysis
>> catches whatever we assert. For object types, that's check_struct() and
>> check_union(). Both check_type() the base with allow_metas=['struct']),
>> so we're good.
>>
>> Inconsistency: you add the check for base, but not for variants.
>>
>> On closer look, adding it for either is actually redundant, because
>> se.f.base.check_clash() already asserts it, with a nice "not
>> implemented" comment.
>>
>> If we think asserting twice is useful for base, then it's useful for
>> variants, too. But I think asserting once suffices.
>
> So basically, we can drop this hunk, right?
Yes.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 05/10] qapi: Utilize implicit struct visits
2016-03-08 16:11 ` Eric Blake
@ 2016-03-08 18:09 ` Markus Armbruster
2016-03-08 18:28 ` Eric Blake
2016-03-09 23:28 ` Eric Blake
1 sibling, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2016-03-08 18:09 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> On 03/08/2016 08:10 AM, 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.
>>>
>>> Generated code shrinks accordingly; events initialize a struct
>>> based on parameters, such as:
>>>
>
>>>
>>> And command marshalling generates call arguments from a stack-allocated
>>> struct:
>>
>> I see qmp-marshal.c shrink from ~5700 lines to ~4300. Neat!
>
> Yeah, it nicely balances out the .h getting so much larger, except that
> the .h gets parsed a lot more by the compiler.
>
>
>>>
>>> +# Declare and initialize an object 'qapi' using parameters from gen_params()
>>> +def gen_struct_init(typ):
>>
>> It's not just a "struct init", it's a variable declaration with an
>> initializer. gen_param_var()?
>>
>> Name the variable param rather than qapi?
>
> Sure, I'm not tied to a specific name. I will point out that we have a
> potential collision point - any local variable we create here can
> collide with members of the QAPI struct passed to the event. We haven't
> hit the collision on any events we've created so far, and it's easy to
> rename our local variables at such time if we do run into the collision
> down the road, so I won't worry about it now.
This patch actually fixes a similar issue in the qmp_marshal_FOO()
functions.
To keep ignoring it in the qapi_event_send_BAR() functions is okay.
It's fairly easy to fix now, though: split them into two, so that the
outer half does nothing but parameter wrapping. For instance,
void qapi_event_send_block_io_error(const char *device, IoOperationType operation, BlockErrorAction action, bool has_nospace, bool nospace, const char *reason, Error **errp)
{
QDict *qmp;
Error *err = NULL;
QMPEventFuncEmit emit;
QmpOutputVisitor *qov;
Visitor *v;
QObject *obj;
_obj_BLOCK_IO_ERROR_arg param = {
(char *)device, operation, action, has_nospace, nospace, (char *)reason
};
[do stuff...]
}
becomes
static inline void do_event_send_block_io_error(_obj_BLOCK_IO_ERROR_arg param, Error **errp)
{
QDict *qmp;
Error *err = NULL;
QMPEventFuncEmit emit;
QmpOutputVisitor *qov;
Visitor *v;
QObject *obj;
[do stuff...]
}
void qapi_event_send_block_io_error(const char *device, IoOperationType operation, BlockErrorAction action, bool has_nospace, bool nospace, const char *reason, Error **errp)
{
do_event_send_block_io_error((_obj_BLOCK_IO_ERROR_arg){
(char *)device, operation, action, has_nospace, nospace, (char *)reason
}, errp);
};
}
Feel free not to do that now, but mark the spot with a comment then.
Since it's technically wrong, we could even mark it FIXME.
>>
>>> + assert not typ.variants
>>> + ret = mcgen('''
>>> + %(c_name)s qapi = {
>>> +''',
>>> + 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 *)'
>>
>> Why is that useful?
>
> The compiler complains if you try to initialize a 'char *' member of a
> QAPI C struct with a 'const char *' parameter. It's no different than
> the fact that the gen_visit_members() call we are getting rid of also
> has to cast away const.
I see. Const never fails to annoy.
[...]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 07/10] qapi: Don't special-case simple union wrappers
2016-03-08 16:16 ` Eric Blake
@ 2016-03-08 18:10 ` Markus Armbruster
0 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2016-03-08 18:10 UTC (permalink / raw)
To: Eric Blake
Cc: Kevin Wolf, Fam Zheng, Eduardo Habkost,
open list:Block layer core, Michael S. Tsirkin, Jan Kiszka,
Jason Wang, Michael Roth, Vincenzo Maffione, qemu-devel,
Gerd Hoffmann, Igor Mammedov, Paolo Bonzini, Giuseppe Lettieri,
Luiz Capitulino, Luigi Rizzo, Samuel Thibault
Eric Blake <eblake@redhat.com> writes:
> On 03/08/2016 08:59 AM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>>
>>> 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:
>>>
>
>>> +++ b/scripts/qapi.py
>>> @@ -1006,7 +1006,6 @@ class QAPISchemaObjectType(QAPISchemaType):
>>> return c_name(self.name) + pointer_suffix
>>>
>>> def c_unboxed_type(self):
>>> - assert not self.is_implicit()
>>
>> Doesn't this belong into PATCH 04?
>>
>>> return c_name(self.name)
>
> Maybe. Patch 3 kept the assertion out of straight code refactoring, and
> patch 4 didn't use c_unboxed_type(), so this was the first place where I
> had to weaken the assertion. But moving it into patch 4 doesn't seem
> like it would hurt, as it is still semantically related to the fact that
> we are planning on allowing an unboxed implicit type.
PATCH 04 drops the assertion from c_name(). Let's keep the two
consistent.
>>> - 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:
>>
>> This stupid special case has given us enough trouble, good to see it
>> gone!
>
> Yeah, it was a nice feeling to get to this point!
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 08/10] qapi: Allow anonymous base for flat union
2016-03-08 16:29 ` Eric Blake
@ 2016-03-08 18:14 ` Markus Armbruster
0 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2016-03-08 18:14 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> On 03/08/2016 09:23 AM, 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.
>>>
>
>>
>> I think you also
>>
>> * fixed a missing allow_optional=True in check_union()
>
> More on that below.
>
>>
>> * fixed a missing "non-optional" in qapi-code-gen.txt (mention in commit
>> message? separate patch?)
>>
>> * renamed readonly to read-only in an example in qapi-code-gen.txt to be
>> closer to the code (separate patch?)
>
> Could separate those two cleanups if it helps.
I'd like the fix recorded in git-log. For that, a mention in the commit
message would suffice, but I think I'd rather separate them.
>>> @@ -560,9 +562,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'])
>>
>> This is where you added allow_optional=True.
>
> allow_optional only matters if allow_dict is True. We have places where
> we allow a dict but do not allow optionals (namely, simple unions); but
> where we are creating an anonymous type, we want to allow optionals at
> the same time we allow a dict. I think this is just a case where the
> commit message needs to be beefed up.
Perhaps not even that. I'm spoiled by your meticulous patch versioning,
and when once in a blue moon I spot something you didn't announce there,
I take note.
>>> +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
>>
>> This is where you supplied the missing "non-optional".
>>
>>> +member of the base struct.
>>>
>>
>> And below, you rename readonly to read-only.
>
> They touch the same paragraph, but I can separate them if it would make
> this patch feel cleaner.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 05/10] qapi: Utilize implicit struct visits
2016-03-08 18:09 ` Markus Armbruster
@ 2016-03-08 18:28 ` Eric Blake
2016-03-08 19:21 ` Markus Armbruster
0 siblings, 1 reply; 36+ messages in thread
From: Eric Blake @ 2016-03-08 18:28 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 2523 bytes --]
On 03/08/2016 11:09 AM, Markus Armbruster wrote:
> This patch actually fixes a similar issue in the qmp_marshal_FOO()
> functions.
Indeed, and I didn't even realize it. I'll add that to the commit message :)
>
> To keep ignoring it in the qapi_event_send_BAR() functions is okay.
> It's fairly easy to fix now, though: split them into two, so that the
> outer half does nothing but parameter wrapping. For instance,
>
> void qapi_event_send_block_io_error(const char *device, IoOperationType operation, BlockErrorAction action, bool has_nospace, bool nospace, const char *reason, Error **errp)
> {
> QDict *qmp;
> Error *err = NULL;
> QMPEventFuncEmit emit;
> QmpOutputVisitor *qov;
> Visitor *v;
> QObject *obj;
> _obj_BLOCK_IO_ERROR_arg param = {
> (char *)device, operation, action, has_nospace, nospace, (char *)reason
> };
>
> [do stuff...]
> }
>
> becomes
>
> static inline void do_event_send_block_io_error(_obj_BLOCK_IO_ERROR_arg param, Error **errp)
> {
> QDict *qmp;
> Error *err = NULL;
> QMPEventFuncEmit emit;
> QmpOutputVisitor *qov;
> Visitor *v;
> QObject *obj;
>
> [do stuff...]
> }
>
> void qapi_event_send_block_io_error(const char *device, IoOperationType operation, BlockErrorAction action, bool has_nospace, bool nospace, const char *reason, Error **errp)
Still means we can't have 'errp' as a QMP member of the error, without
some sort of renaming. Again, not worth worrying about until we
actually want to avoid the collision.
> {
> do_event_send_block_io_error((_obj_BLOCK_IO_ERROR_arg){
> (char *)device, operation, action, has_nospace, nospace, (char *)reason
> }, errp);
> };
> }
>
> Feel free not to do that now, but mark the spot with a comment then.
> Since it's technically wrong, we could even mark it FIXME.
In fact, I have a patch in a later series [1] that WANTS to let the user
supply a boxed parameter - at which point, the difference between two
vs. one function would be whether the user requested boxing. Sounds
like I add the FIXME here, and then that series can take care of the
possible split.
[1] https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04394.html
--
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] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 04/10] qapi: Emit implicit structs in generated C
2016-03-08 16:03 ` Eric Blake
@ 2016-03-08 19:09 ` Markus Armbruster
2016-03-09 5:42 ` Eric Blake
0 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2016-03-08 19:09 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> On 03/08/2016 07:24 AM, Markus Armbruster wrote:
>> 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.
>>
>> Yes.
>>
>>> 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[0] == ':'" feels a bit hacky, but beats changing the
>>> signature of the visit_object_type() callback to pass a new
>>> 'implicit' flag.
>>
>> PRO: it matches what QAPISchemaObjectType.is_implicit() does.
>>
>> CON: it duplicates what QAPISchemaObjectType.is_implicit() does.
>>
>> Ways to adhere to DRY:
>>
>> (1) Add a flag to visit_object_type() and, for consistency, to
>> visit_object_type_flat().
>>
>> (2) Change the QAPISchemaVisitor.visit_FOO() to take a full FOO instead
>> of FOO.name, FOO.info and selected other members.
>>
>> We've discussed (2) elsewhere. The QAPISchemaEntity.visit() shrink to a
>> mere double-dispatch. The QAPISchemaVisitor gain full access to the
>> things they visit. The interface between the generators and the
>> internal representation changes from a narrow, explicit and inflexible
>> one to a much wider "anything goes" one. Both the narrow and the wide
>> interface have advantages and disadvantages. Have we outgrown the
>> narrow one?
>
> Your argument that the narrow view forces us to think about things may
> still hold. I'm border-line on whether the switch is worth it, vs.
> adding flags as the visitors want to learn more and more about what they
> are visiting without having the actual Python object in hand.
I used to be on the "narrow" side of the fence, but I've come to sit on
it.
> I think what would sway me over the fence is looking at some of our
> constructs: for example, qapi-types.py has gen_object() which it now
> calls recursively. When called directly from visit_object_type(), we
> have all the pieces; when called from visit_alternate_type(), we have to
> create a 1-element members array; and when called recursively, we have
> to manually explode base into the four pieces.
What could be improved if we changed visit_object_type() to take a
QAPISchemaObjectType instead of name, info, base, members, variants?
I believe we'd leave gen_object() unchanged to keep
visit_alternate_type() simple.
Here's a different one: we could drop visit_object_type_flat().
>>> 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 ':empty' 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 fact that all our implicit objects start
>>> with a leading ':', which can be transliteratated to a leading
>>
>> transliterated
>>
>>> single underscore, and we've already documented and enforce that
>>
>> Uh, these are "always reserved for use as identifiers with file scope"
>> (C99 7.1.3). I'm afraid we need to use the q_ prefix.
>>
>>> the user can't create QAPI names with a leading underscore, so
>>> exposing the types won't create any collisions. It is a bit
>>> unfortunate that the generated struct names don't match normal
>>> naming conventions, but it's not too bad, since they will only
>>> be used in generated code.
>>
>> The problem is self-inflicted: we make up these names in
>> _make_implicit_object_type(). We could just as well make up names that
>> come out prettier.
>
> Any suggestions? It seems easy enough to change, if we have something
> that we like.
For an array of T, we use TList.
For a simple union U's implicit enum, we use UKind.
Both predate the application of rational thought to clashes in generated
code ;) Reserving these names was easier than changing them, so that's
what we did.
For implicit objects, we use :obj-NAME-ROLE. We can change that, but
need to keep QAPISchemaMember._pretty_owner() working.
We could use c_name(q_obj_NAME_ROLE).
The resulting types aren't in CamelCase. Making them CamelCase would be
easy enough when NAME is a type name (it is for ROLEs wrapper and base),
but for command and event names (ROLE arg) it involves playing stupid
games with case, dash and underscore. Let's not go there, and pray the
coding style police looks the other way.
>> In fact, my first versions of the code had names starting with ':' for
>> *all* implicit types. I abandoned that for enum and array types when I
>> realized I need C names for them, and the easiest way to get them making
>> up names so that a trivial .c_name() works. We can do the same for
>> object types.
>>
>>> The generated code grows substantially in size; in part because
>>> it was easier to output every implicit type rather than adding
>>> generator complexity to try and only output types as needed.
>>
>> I happily trade larger .c files the optimizer can reduce for a simpler
>> generator. I'm less sanguine about larger .h files when they get
>> included a lot. qapi-types.h gets included basically everywhere, and
>> grows from ~4000 to ~5250 lines. How much of this is avoidable?
>
> Not much: remember, because we use unboxed types in unions, all -wrapper
> structs have to be present in the .h for the compiler to not complain
> about incomplete types.
>
> For -arg types (and for upcoming -base types in 8/10), we could get away
> with only forward declarations of the type in the .h: the
> visit_type_ARG_members() function uses only a pointer so it can live
> with an incomplete type in the header and a complete type in a private
> helper file or in the .c. But we have fewer -arg classes in comparison
> to the -wrapper classes, which means splitting on those lines would add
> a lot of complexity to the generator for very little .h savings.
Hmm.
I feel awful generating >100KiB of code that gets included pretty much
every time we compile anything. Perhaps the proper fix for that is to
find out *why* we include qapi-types.h so much, then figure out how to
include it much, much less.
Here's a guilty one: error.h. I believe it includes qapi-types.h just
for QapiErrorClass. I guess it's only in the QAPI schema to have
QapiErrorClass_lookup[] generated. I'd gladly maintain those nine(!)
lines of code manually if it helps me drop >100KiB of useless code from
many (most?) compiles.
Another one are builtin QAPI types. A few headers want them. We could
put them into a separate header instead of generating them into
qapi-types.h. Could also enable getting rid of the ugly "spew builtins
into every header, but guard them with #ifdeffery" trick.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 10/10] qapi: Populate info['name'] for each entity
2016-03-08 16:59 ` Eric Blake
@ 2016-03-08 19:14 ` Markus Armbruster
0 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2016-03-08 19:14 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> On 03/08/2016 09:46 AM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>>
>>> Every non-implicit entity is associated with an 'info'
>>> dictionary, but it is not easy to reverse-engineer the name of
>>> the top-most entity associated with that 'info'. Our use of
>>> 'case_whitelist' (added in commit 893e1f2) is thus currently
>>> tied to the owner of a member instead; but as the previous patch
>>> showed with CpuInfo, this requires whitelist exceptions to know
>>> how an implicit name will be generated.
>>
>> Why is that a problem?
>
> Not necessarily a bad problem, but a bit annoying. If a developer
> modifies a .json file and adds an improper name, then they will get an
> error message that tells them that they need to fix their naming
> conventions (hmm, the error message doesn't even point them to the
> whitelist - see args-member-case.err).
I'd consider that a feature :) I don't want anyone adding to that white
list.
> If the developer figures out
> that the whitelist will let them avoid the error, they still have to
> figure _what_ name to add to the whitelist, and without this patch, they
> have to determine the generated name for the implicit struct
Still feature...
> (which may
> not be constant, since we are discussing about alternative names earlier
> in the series other than something that flattens to a public 'struct
> _obj_...' in violation of file-local naming scope).
Whoever messes with the generated names gets to update the whitelist.
> The goal is thus to
> make the whitelist tied only to names mentioned in the .json file,
> rather than dragging generated implicit names into the mix.
>
> But it is cosmetic; we could live without the patch and stick to
> generated names in the whitelist, just as easily.
Let's drop this patch.
If we find a truly compelling use for info['name'] later, we can revisit
it if you like, because then it's almost free.
[...]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 05/10] qapi: Utilize implicit struct visits
2016-03-08 18:28 ` Eric Blake
@ 2016-03-08 19:21 ` Markus Armbruster
0 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2016-03-08 19:21 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> On 03/08/2016 11:09 AM, Markus Armbruster wrote:
>
>> This patch actually fixes a similar issue in the qmp_marshal_FOO()
>> functions.
>
> Indeed, and I didn't even realize it. I'll add that to the commit message :)
>
>>
>> To keep ignoring it in the qapi_event_send_BAR() functions is okay.
>> It's fairly easy to fix now, though: split them into two, so that the
>> outer half does nothing but parameter wrapping. For instance,
>>
>> void qapi_event_send_block_io_error(const char *device, IoOperationType operation, BlockErrorAction action, bool has_nospace, bool nospace, const char *reason, Error **errp)
>> {
>> QDict *qmp;
>> Error *err = NULL;
>> QMPEventFuncEmit emit;
>> QmpOutputVisitor *qov;
>> Visitor *v;
>> QObject *obj;
>> _obj_BLOCK_IO_ERROR_arg param = {
>> (char *)device, operation, action, has_nospace, nospace, (char *)reason
>> };
>>
>> [do stuff...]
>> }
>>
>> becomes
>>
>> static inline void do_event_send_block_io_error(_obj_BLOCK_IO_ERROR_arg param, Error **errp)
>> {
>> QDict *qmp;
>> Error *err = NULL;
>> QMPEventFuncEmit emit;
>> QmpOutputVisitor *qov;
>> Visitor *v;
>> QObject *obj;
>>
>> [do stuff...]
>> }
>>
>> void qapi_event_send_block_io_error(const char *device, IoOperationType operation, BlockErrorAction action, bool has_nospace, bool nospace, const char *reason, Error **errp)
>
> Still means we can't have 'errp' as a QMP member of the error, without
> some sort of renaming. Again, not worth worrying about until we
> actually want to avoid the collision.
Rename it to q_errp.
>> {
>> do_event_send_block_io_error((_obj_BLOCK_IO_ERROR_arg){
>> (char *)device, operation, action, has_nospace, nospace, (char *)reason
>> }, errp);
>> };
>> }
>>
>> Feel free not to do that now, but mark the spot with a comment then.
>> Since it's technically wrong, we could even mark it FIXME.
>
> In fact, I have a patch in a later series [1] that WANTS to let the user
> supply a boxed parameter - at which point, the difference between two
> vs. one function would be whether the user requested boxing. Sounds
> like I add the FIXME here, and then that series can take care of the
> possible split.
>
> [1] https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04394.html
Works for me.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 04/10] qapi: Emit implicit structs in generated C
2016-03-08 19:09 ` Markus Armbruster
@ 2016-03-09 5:42 ` Eric Blake
2016-03-09 7:23 ` Markus Armbruster
0 siblings, 1 reply; 36+ messages in thread
From: Eric Blake @ 2016-03-09 5:42 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 2600 bytes --]
On 03/08/2016 12:09 PM, Markus Armbruster wrote:
>
>> I think what would sway me over the fence is looking at some of our
>> constructs: for example, qapi-types.py has gen_object() which it now
>> calls recursively. When called directly from visit_object_type(), we
>> have all the pieces; when called from visit_alternate_type(), we have to
>> create a 1-element members array; and when called recursively, we have
>> to manually explode base into the four pieces.
>
> What could be improved if we changed visit_object_type() to take a
> QAPISchemaObjectType instead of name, info, base, members, variants?
>
> I believe we'd leave gen_object() unchanged to keep
> visit_alternate_type() simple.
>
> Here's a different one: we could drop visit_object_type_flat().
Indeed. But I'd rather get v5 of this series out sooner rather than
later, so I'll save the change for a later day.
>>>
>>> Uh, these are "always reserved for use as identifiers with file scope"
>>> (C99 7.1.3). I'm afraid we need to use the q_ prefix.
Seems doable. We already reserved q_ for ourselves (commit 9fb081e0), so
far using it mainly by c_name. There's still the risk that c_name adds
q_ onto something ticklish which then clashes with our use of q_ on an
implicit type for something else; except that we haven't declared 'obj'
as ticklish.
> I feel awful generating >100KiB of code that gets included pretty much
> every time we compile anything. Perhaps the proper fix for that is to
> find out *why* we include qapi-types.h so much, then figure out how to
> include it much, much less.
>
> Here's a guilty one: error.h. I believe it includes qapi-types.h just
> for QapiErrorClass. I guess it's only in the QAPI schema to have
> QapiErrorClass_lookup[] generated. I'd gladly maintain those nine(!)
> lines of code manually if it helps me drop >100KiB of useless code from
> many (most?) compiles.
Commit 13f59ae predates my work on qapi, but I think you're right that
error.h including qapi-types.h is a big offender and appears to do so
solely for ErrorClass. But how about as a followup series, so that v5
gets out the door sooner.
>
> Another one are builtin QAPI types. A few headers want them. We could
> put them into a separate header instead of generating them into
> qapi-types.h. Could also enable getting rid of the ugly "spew builtins
> into every header, but guard them with #ifdeffery" trick.
In that same series.
--
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] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 04/10] qapi: Emit implicit structs in generated C
2016-03-09 5:42 ` Eric Blake
@ 2016-03-09 7:23 ` Markus Armbruster
0 siblings, 0 replies; 36+ messages in thread
From: Markus Armbruster @ 2016-03-09 7:23 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Michael Roth
Eric Blake <eblake@redhat.com> writes:
> On 03/08/2016 12:09 PM, Markus Armbruster wrote:
>
>>
>>> I think what would sway me over the fence is looking at some of our
>>> constructs: for example, qapi-types.py has gen_object() which it now
>>> calls recursively. When called directly from visit_object_type(), we
>>> have all the pieces; when called from visit_alternate_type(), we have to
>>> create a 1-element members array; and when called recursively, we have
>>> to manually explode base into the four pieces.
>>
>> What could be improved if we changed visit_object_type() to take a
>> QAPISchemaObjectType instead of name, info, base, members, variants?
>>
>> I believe we'd leave gen_object() unchanged to keep
>> visit_alternate_type() simple.
>>
>> Here's a different one: we could drop visit_object_type_flat().
>
> Indeed. But I'd rather get v5 of this series out sooner rather than
> later, so I'll save the change for a later day.
I agree that we've wandered beyond the scope of this series here. It's
not about the visitor, it merely struggles with the visitor's narrow
view on the visited objects. That led us to discuss whether that view
still makes sense. We're not sure.
>>>> Uh, these are "always reserved for use as identifiers with file scope"
>>>> (C99 7.1.3). I'm afraid we need to use the q_ prefix.
>
> Seems doable. We already reserved q_ for ourselves (commit 9fb081e0), so
> far using it mainly by c_name. There's still the risk that c_name adds
> q_ onto something ticklish which then clashes with our use of q_ on an
> implicit type for something else; except that we haven't declared 'obj'
> as ticklish.
As far as I can tell, c_name() is still the only user of q_. If we add
more, we need to update the "Reserve the entire 'q_' namespace for
c_name()" comment, and we should try to keep track of our use of q_
somehow.
Alternatively, reserve another chunk of the name space for implicit
object types. Might be easier to maintain.
>> I feel awful generating >100KiB of code that gets included pretty much
>> every time we compile anything. Perhaps the proper fix for that is to
>> find out *why* we include qapi-types.h so much, then figure out how to
>> include it much, much less.
>>
>> Here's a guilty one: error.h. I believe it includes qapi-types.h just
>> for QapiErrorClass. I guess it's only in the QAPI schema to have
>> QapiErrorClass_lookup[] generated. I'd gladly maintain those nine(!)
>> lines of code manually if it helps me drop >100KiB of useless code from
>> many (most?) compiles.
>
> Commit 13f59ae predates my work on qapi, but I think you're right that
> error.h including qapi-types.h is a big offender and appears to do so
> solely for ErrorClass. But how about as a followup series, so that v5
> gets out the door sooner.
Teaching error.h manners doesn't depend on anything in your pipe. I'm
invoking "Error Reporting" maintainer privileges: the task is mine :)
>> Another one are builtin QAPI types. A few headers want them. We could
>> put them into a separate header instead of generating them into
>> qapi-types.h. Could also enable getting rid of the ugly "spew builtins
>> into every header, but guard them with #ifdeffery" trick.
>
> In that same series.
I'd expect this to be a series of its own, going through the QAPI tree.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 05/10] qapi: Utilize implicit struct visits
2016-03-08 16:11 ` Eric Blake
2016-03-08 18:09 ` Markus Armbruster
@ 2016-03-09 23:28 ` Eric Blake
1 sibling, 0 replies; 36+ messages in thread
From: Eric Blake @ 2016-03-09 23:28 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 784 bytes --]
On 03/08/2016 09:11 AM, Eric Blake wrote:
>>>
>>> - ret += gen_visit_members(arg_type.members, skiperr=dealloc)
>>
>> Unless I'm mistaken, this is the only use of skiperr. Follow up with a
>> patch to drop the parameter and simplify?
>
> Oh, nice. I noticed some cleanups in patch 6/10, but missed this one as
> a reasonable improvement.
>
> In fact, gen_visit_members() is now only used in qapi-visits.py, so
> maybe I can move it back there (it used to live there before commit
> 82ca8e469 moved it for sharing with the two files simplified here).
In fact, this also nukes the only use of c_null(). I'll have a couple
of cleanup patches in v5.
--
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] 36+ messages in thread
end of thread, other threads:[~2016-03-09 23:28 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-05 16:16 [Qemu-devel] [PATCH v4 00/10] easier unboxed visits/qapi implicit types Eric Blake
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 01/10] qapi: Assert in places where variants are not handled Eric Blake
2016-03-08 10:12 ` Markus Armbruster
2016-03-08 15:49 ` Eric Blake
2016-03-08 17:46 ` Markus Armbruster
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 02/10] qapi: Fix command with named empty argument type Eric Blake
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 03/10] qapi: Make c_type() more OO-like Eric Blake
2016-03-08 10:54 ` Markus Armbruster
2016-03-08 15:50 ` Eric Blake
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 04/10] qapi: Emit implicit structs in generated C Eric Blake
2016-03-08 14:24 ` Markus Armbruster
2016-03-08 16:03 ` Eric Blake
2016-03-08 19:09 ` Markus Armbruster
2016-03-09 5:42 ` Eric Blake
2016-03-09 7:23 ` Markus Armbruster
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 05/10] qapi: Utilize implicit struct visits Eric Blake
2016-03-08 15:10 ` Markus Armbruster
2016-03-08 16:11 ` Eric Blake
2016-03-08 18:09 ` Markus Armbruster
2016-03-08 18:28 ` Eric Blake
2016-03-08 19:21 ` Markus Armbruster
2016-03-09 23:28 ` Eric Blake
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 06/10] qapi-commands: Inline single-use helpers of gen_marshal() Eric Blake
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 07/10] qapi: Don't special-case simple union wrappers Eric Blake
2016-03-08 15:59 ` Markus Armbruster
2016-03-08 16:16 ` Eric Blake
2016-03-08 18:10 ` Markus Armbruster
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 08/10] qapi: Allow anonymous base for flat union Eric Blake
2016-03-08 16:23 ` Markus Armbruster
2016-03-08 16:29 ` Eric Blake
2016-03-08 18:14 ` Markus Armbruster
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 09/10] qapi: Use anonymous bases in QMP flat unions Eric Blake
2016-03-05 16:16 ` [Qemu-devel] [PATCH v4 10/10] qapi: Populate info['name'] for each entity Eric Blake
2016-03-08 16:46 ` Markus Armbruster
2016-03-08 16:59 ` Eric Blake
2016-03-08 19:14 ` Markus Armbruster
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).