[Qemu-devel] [PATCH 0/2] qapi: allow flat unions with empty branches

Discussion:

Anton Nefedov

2018-05-11 09:05:32 UTC

hej,

inspired by this thread
http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg04802.html

Seems that nobody got around to this since (did anybody?),
so I thought I'd give a try.

There's a generator patch and usage example.

Anton Nefedov (2):
qapi: allow flat unions with empty branches
qapi: avoid empty CpuInfoOther type

qapi/misc.json | 48 ++++--------------------------------------
cpus.c | 2 --
scripts/qapi/common.py | 18 ++++++++++------
scripts/qapi/doc.py | 2 +-
scripts/qapi/types.py | 2 +-
scripts/qapi/visit.py | 12 ++++++-----
tests/qapi-schema/test-qapi.py | 2 +-
7 files changed, 26 insertions(+), 60 deletions(-)

--
2.7.4

Anton Nefedov

2018-05-11 09:05:33 UTC

Permalink

The patch makes possible to avoid introducing dummy empty types
for the flat union branches that have no data.

Signed-off-by: Anton Nefedov <***@virtuozzo.com>
---
scripts/qapi/common.py | 18 ++++++++++++------
scripts/qapi/doc.py | 2 +-
scripts/qapi/types.py | 2 +-
scripts/qapi/visit.py | 12 +++++++-----
tests/qapi-schema/test-qapi.py | 2 +-
5 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index a032cec..ec5cf28 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -721,6 +721,7 @@ def check_union(expr, info):
name = expr['union']
base = expr.get('base')
discriminator = expr.get('discriminator')
+ partial = expr.get('data-partial', False)
members = expr['data']

# Two types of unions, determined by discriminator.
@@ -783,7 +784,7 @@ def check_union(expr, info):
% (key, enum_define['enum']))

# If discriminator is user-defined, ensure all values are covered
- if enum_define:
+ if enum_define and not partial:
for value in enum_define['data']:
if value not in members.keys():
raise QAPISemError(info, "Union '%s' data missing '%s' branch"
@@ -909,7 +910,7 @@ def check_exprs(exprs):
elif 'union' in expr:
meta = 'union'
check_keys(expr_elem, 'union', ['data'],
- ['base', 'discriminator'])
+ ['base', 'discriminator', 'data-partial'])
union_types[expr[meta]] = expr
elif 'alternate' in expr:
meta = 'alternate'
@@ -1035,7 +1036,7 @@ class QAPISchemaVisitor(object):
def visit_array_type(self, name, info, element_type):
pass

- def visit_object_type(self, name, info, base, members, variants):
+ def visit_object_type(self, name, info, base, members, variants, partial):
pass

def visit_object_type_flat(self, name, info, members, variants):
@@ -1192,7 +1193,8 @@ class QAPISchemaArrayType(QAPISchemaType):

class QAPISchemaObjectType(QAPISchemaType):
- def __init__(self, name, info, doc, base, local_members, variants):
+ def __init__(self, name, info, doc, base, local_members, variants,
+ partial = False):
# struct has local_members, optional base, and no variants
# flat union has base, variants, and no local_members
# simple union has local_members, variants, and no base
@@ -1209,6 +1211,7 @@ class QAPISchemaObjectType(QAPISchemaType):
self.local_members = local_members
self.variants = variants
self.members = None
+ self.partial = partial

def check(self, schema):
if self.members is False: # check for cycles
@@ -1269,7 +1272,8 @@ class QAPISchemaObjectType(QAPISchemaType):

def visit(self, visitor):
visitor.visit_object_type(self.name, self.info,
- self.base, self.local_members, self.variants)
+ self.base, self.local_members, self.variants,
+ self.partial)
visitor.visit_object_type_flat(self.name, self.info,
self.members, self.variants)

@@ -1636,6 +1640,7 @@ class QAPISchema(object):
name = expr['union']
data = expr['data']
base = expr.get('base')
+ partial = expr.get('data-partial', False)
tag_name = expr.get('discriminator')
tag_member = None
if isinstance(base, dict):
@@ -1656,7 +1661,8 @@ class QAPISchema(object):
QAPISchemaObjectType(name, info, doc, base, members,
QAPISchemaObjectTypeVariants(tag_name,
tag_member,
- variants)))
+ variants),
+ partial))

def _def_alternate_type(self, expr, info, doc):
name = expr['alternate']
diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 9b312b2..40dffc4 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -211,7 +211,7 @@ class QAPISchemaGenDocVisitor(qapi.common.QAPISchemaVisitor):
body=texi_entity(doc, 'Values',
member_func=texi_enum_value)))

- def visit_object_type(self, name, info, base, members, variants):
+ def visit_object_type(self, name, info, base, members, variants, partial):
doc = self.cur_doc
if base and base.is_implicit():
base = None
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 64d9c0f..e805509 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -215,7 +215,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
self._genh.add(gen_array(name, element_type))
self._gen_type_cleanup(name)

- def visit_object_type(self, name, info, base, members, variants):
+ def visit_object_type(self, name, info, base, members, variants, partial):
# Nothing to do for the special empty builtin
if name == 'q_empty':
return
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 5d72d89..3ee64bb 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -34,7 +34,7 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp);
c_name=c_name(name))

-def gen_visit_object_members(name, base, members, variants):
+def gen_visit_object_members(name, base, members, variants, partial):
ret = mcgen('''

void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
@@ -93,9 +93,10 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)

ret += mcgen('''
default:
- abort();
+ %(action)s
}
-''')
+''',
+ action="break;" if partial else "abort();")

# 'goto out' produced for base, for each member, and if variants were
# present
@@ -309,12 +310,13 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
self._genh.add(gen_visit_decl(name))
self._genc.add(gen_visit_list(name, element_type))

- def visit_object_type(self, name, info, base, members, variants):
+ def visit_object_type(self, name, info, base, members, variants, partial):
# Nothing to do for the special empty builtin
if name == 'q_empty':
return
self._genh.add(gen_visit_members_decl(name))
- self._genc.add(gen_visit_object_members(name, base, members, variants))
+ self._genc.add(gen_visit_object_members(name, base, members, variants,
+ partial))
# TODO Worth changing the visitor signature, so we could
# directly use rather than repeat type.is_implicit()?
if not name.startswith('q_'):
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index c1a144b..95cd575 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -28,7 +28,7 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
if prefix:
print(' prefix %s' % prefix)

- def visit_object_type(self, name, info, base, members, variants):
+ def visit_object_type(self, name, info, base, members, variants, partial):
print('object %s' % name)
if base:
print(' base %s' % base.name)

--
2.7.4

Markus Armbruster

2018-05-14 18:08:18 UTC

Permalink

Post by Anton Nefedov
The patch makes possible to avoid introducing dummy empty types
for the flat union branches that have no data.
---
scripts/qapi/common.py | 18 ++++++++++++------
scripts/qapi/doc.py | 2 +-
scripts/qapi/types.py | 2 +-
scripts/qapi/visit.py | 12 +++++++-----
tests/qapi-schema/test-qapi.py | 2 +-
5 files changed, 22 insertions(+), 14 deletions(-)

Doesn't docs/devel/qapi-code-gen.txt need an update?

Post by Anton Nefedov
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index a032cec..ec5cf28 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
name = expr['union']
base = expr.get('base')
discriminator = expr.get('discriminator')
+ partial = expr.get('data-partial', False)

Yes, it does.

Discussing a proposed new QAPI language feature is much easier when the
patch starts with a documentation update specifying the new feature. If
you were a seasoned QAPI developer, my review would stop right here :)
Since you're not, I'll try to make sense of it.

Post by Anton Nefedov
members = expr['data']
# Two types of unions, determined by discriminator.
% (key, enum_define['enum']))
# If discriminator is user-defined, ensure all values are covered

data-partial supresses the check for "union lists all discriminator
values". Makes sense given your commit message.

Post by Anton Nefedov
raise QAPISemError(info, "Union '%s' data missing '%s' branch"
meta = 'union'
check_keys(expr_elem, 'union', ['data'],
- ['base', 'discriminator'])
+ ['base', 'discriminator', 'data-partial'])

This accepts key 'data-partial'. Missing: we also need to check its
value, in check_keys().

Post by Anton Nefedov
union_types[expr[meta]] = expr
meta = 'alternate'
pass

As we'll see below, @partial is needed just for qapi/visit.py's
gen_visit_object_members().

Post by Anton Nefedov
pass
+ def __init__(self, name, info, doc, base, local_members, variants,
# struct has local_members, optional base, and no variants
# flat union has base, variants, and no local_members
# simple union has local_members, variants, and no base
self.local_members = local_members
self.variants = variants
self.members = None
+ self.partial = partial
if self.members is False: # check for cycles
visitor.visit_object_type(self.name, self.info,
- self.base, self.local_members, self.variants)
+ self.base, self.local_members, self.variants,
+ self.partial)
visitor.visit_object_type_flat(self.name, self.info,
self.members, self.variants)
name = expr['union']
data = expr['data']
base = expr.get('base')
+ partial = expr.get('data-partial', False)
tag_name = expr.get('discriminator')
tag_member = None

Flat union; @partial applies.

base = (self._make_implicit_object_type(
name, info, doc, 'base', self._make_members(base, info)))
if tag_name:
variants = [self._make_variant(key, value)
for (key, value) in data.items()]
members = []

Note: if @partial, then variants no longer cover all tag values. As
we'll see below, this necessitates changes to some back ends.

Our general strategy is to make the front end normalize away convenience
features as much as possible. It looks quite possible here: add the
implicit variants. We need a type for them, obviously. Perhaps we can
use the internal 'q_empty'.

else:

Simple union; @partial is meaningless here.

variants = [self._make_simple_variant(key, value, info)
for (key, value) in data.items()]
typ = self._make_implicit_enum_type(name, info,
[v.name for v in variants])
tag_member = QAPISchemaObjectTypeMember('type', typ, False)
members = [tag_member]
self._def_entity(
QAPISchemaObjectType(name, info, doc, base, members,
QAPISchemaObjectTypeVariants(tag_name,
tag_member,
variants)))

Post by Anton Nefedov
QAPISchemaObjectType(name, info, doc, base, members,
QAPISchemaObjectTypeVariants(tag_name,
tag_member,
- variants)))
+ variants),
+ partial))
name = expr['alternate']

Okay, that's all for the front end.

As far as I can tell, 'data-partial' the front end accept and ignores
'data-partial' with simple unions. It should reject it instead.

Post by Anton Nefedov
diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 9b312b2..40dffc4 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
body=texi_entity(doc, 'Values',
member_func=texi_enum_value)))
doc = self.cur_doc
base = None
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 64d9c0f..e805509 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
self._genh.add(gen_array(name, element_type))
self._gen_type_cleanup(name)
# Nothing to do for the special empty builtin
return
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 5d72d89..3ee64bb 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -34,7 +34,7 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp);
c_name=c_name(name))
ret = mcgen('''
void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
@@ -93,9 +93,10 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
ret += mcgen('''
- abort();
+ %(action)s
}
-''')
+''',
+ action="break;" if partial else "abort();")

No change when partial=False: the switch cases cover all valid
discriminator values, and default aborts on invalid ones.

When partial=True, the cases cover only the values listed in the union,
and default does nothing. This isn't quite right. We should do nothing
only for the values not listed in the union, and still abort for invalid
ones.

If the front end normalizes away the convenience feature, we should get
this behavior without any changes here.

Post by Anton Nefedov
# 'goto out' produced for base, for each member, and if variants were
# present
self._genh.add(gen_visit_decl(name))
self._genc.add(gen_visit_list(name, element_type))
# Nothing to do for the special empty builtin
return
self._genh.add(gen_visit_members_decl(name))
- self._genc.add(gen_visit_object_members(name, base, members, variants))
+ self._genc.add(gen_visit_object_members(name, base, members, variants,
+ partial))
# TODO Worth changing the visitor signature, so we could
# directly use rather than repeat type.is_implicit()?
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index c1a144b..95cd575 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
print(' prefix %s' % prefix)
print('object %s' % name)
print(' base %s' % base.name)

Okay, now let's take a step back and review the problem this patch
solves, and the design space. The perfect patch would do that in the
commit message, but we're not expecting perfection, only honest effort
:)

Some unions have no variant members for certain discriminator values.
We currently have to use an empty type there, and we've accumulated
several just for the purpose, which is annoying.

QAPI language design alternatives:

1. Having unions cover all discriminator values explicitly is useful.
All we need is a more convenient way to denote empty types. Eric
proposed {}, as in 'foo: {}.

2. Having unions repeat all the discriminator values explicitly is not
useful. All we need is replacing the code enforcing that by code
defaulting missing ones to the empty type.

3. We can't decide, so we do both (this patch).

Preferences?

Eric Blake

2018-05-14 19:34:53 UTC

Permalink

Post by Markus Armbruster

Post by Anton Nefedov
The patch makes possible to avoid introducing dummy empty types
for the flat union branches that have no data.

Some unions have no variant members for certain discriminator values.
We currently have to use an empty type there, and we've accumulated
several just for the purpose, which is annoying.
1. Having unions cover all discriminator values explicitly is useful.
All we need is a more convenient way to denote empty types. Eric
proposed {}, as in 'foo: {}.

And even posted a patch for it once:
https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg00311.html

although it was late enough in that series with other churn in the
meantime that it would need serious revisiting to apply today.

Post by Markus Armbruster
2. Having unions repeat all the discriminator values explicitly is not
useful. All we need is replacing the code enforcing that by code
defaulting missing ones to the empty type.
3. We can't decide, so we do both (this patch).
Preferences?

Here's some tradeoffs to consider:

Allowing 'foo':{} makes your intent explicit - "I know this branch of
the union exists, but it specifically adds no further members". But
it's a lot of redundant typing - "I already had to type all the branch
names when declaring the enum type, why do it again here?"

Allowing an easy way to mark all non-listed members of an enum default
to the empty type is compact - "I told you the enum, so you are
permitted to fill in an empty type with every branch that does not
actually need further members; and I had to opt in to that behavior, so
that I purposefully get an error if I did not opt in but forgot an enum
member". But if the enum is likely to change, it can lead to forgotten
additions later on - "I'm adding member 'bar' to an enum that already
has member 'foo'; therefore, grepping for 'foo' should tell me all
places where I should add handling for 'bar', except that it doesn't
work when handling for 'foo' was implicit but handling for 'bar' should
not be".

Having said that, my personal preference is that opting in to an
explicit way to do less typing ("all branches of the enum listed here
are valid, and add no further members") seems like a nice syntactic
sugar trick; and the easiest way to actually implement it is to probably
already have support for an explicit 'foo':{} branch notation. That
way, you can always change your mind later and undo the opt-in
"data-partial" flag and rewrite the union with explicit empty branches
when needed, to match how the sugar was expanded on your behalf.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org

Markus Armbruster

2018-05-15 07:01:09 UTC

Permalink

Post by Eric Blake

Post by Markus Armbruster

Post by Anton Nefedov
The patch makes possible to avoid introducing dummy empty types
for the flat union branches that have no data.

https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg00311.html
although it was late enough in that series with other churn in the
meantime that it would need serious revisiting to apply today.

Allowing 'foo':{} makes your intent explicit - "I know this branch of
the union exists, but it specifically adds no further members".

Yes.

Post by Eric Blake
But
it's a lot of redundant typing - "I already had to type all the branch
names when declaring the enum type, why do it again here?"

Redundancy isn't bad per se, but it needs to serve a useful purpose.
What's the purpose of repeating the tag values?

You give one below: grep.

Post by Eric Blake
Allowing an easy way to mark all non-listed members of an enum default
to the empty type is compact - "I told you the enum, so you are
permitted to fill in an empty type with every branch that does not
actually need further members; and I had to opt in to that behavior,
so that I purposefully get an error if I did not opt in but forgot an
enum member".

Syntactic options aren't bad per se, but they need to serve a useful
purpose. What's the purpose of having both unions that require all tag
values, and unions that default omitted tag values to "no variant
members"?

Post by Eric Blake
But if the enum is likely to change, it can lead to
forgotten additions later on - "I'm adding member 'bar' to an enum
that already has member 'foo'; therefore, grepping for 'foo' should
tell me all places where I should add handling for 'bar', except that
it doesn't work when handling for 'foo' was implicit but handling for
'bar' should not be".

If your code still compiles when you forget to add members to a struct
or union, you obviously don't need the members you forgot :)

For what it's worth, grepping for the enum type's name finds the union
just fine, and is less likely to find unrelated stuff.

Post by Eric Blake
Having said that, my personal preference is that opting in to an
explicit way to do less typing ("all branches of the enum listed here
are valid, and add no further members") seems like a nice syntactic
sugar trick; and the easiest way to actually implement it is to
probably already have support for an explicit 'foo':{} branch
notation. That way, you can always change your mind later and undo
the opt-in "data-partial" flag and rewrite the union with explicit
empty branches when needed, to match how the sugar was expanded on
your behalf.

Is that 1. combined with 3.?

I dislike 3. because I feel it adds more complexity than the other
options. More apparent once you add the necessary test coverage
(another reason why requiring test coverage is such a good idea; having
to cover every bell & whistle tends to make people reconsider which ones
they actually need).

I'd prefer a more opinionated design here.

Either we opine making people repeat the tag values is an overall win.
Then make them repeat them always, don't give them the option to not
repeat them. Drop this patch. To reduce verbosity, we can add a
suitable way to denote a predefined empty type.

Or we opine it's not. Then let them not repeat them, don't give them
the option to make themselves repeat them. Evolve this patch into one
that makes flat union variants optional and default to empty. If we
later find we still want a way to denote a predefined empty type, we can
add it then.

My personal opinion is it's not.

Eric Blake

2018-05-15 15:20:01 UTC

Permalink

Post by Markus Armbruster

Post by Markus Armbruster
1. Having unions cover all discriminator values explicitly is useful.
2. Having unions repeat all the discriminator values explicitly is not
useful. All we need is replacing the code enforcing that by code
defaulting missing ones to the empty type.
3. We can't decide, so we do both (this patch).

I'd prefer a more opinionated design here.
Either we opine making people repeat the tag values is an overall win.
Then make them repeat them always, don't give them the option to not
repeat them. Drop this patch. To reduce verbosity, we can add a
suitable way to denote a predefined empty type.
Or we opine it's not. Then let them not repeat them, don't give them
the option to make themselves repeat them. Evolve this patch into one
that makes flat union variants optional and default to empty. If we
later find we still want a way to denote a predefined empty type, we can
add it then.
My personal opinion is it's not.

I followed the arguments up to the last sentence, but then I got lost on
whether you meant:

This patch is not an overall win, so let's drop it and keep status quo
and/or implement a way to write 'branch':{} (option 1 above)

or

Forcing repetition is not an overall win, so let's drop that requirement
by using something similar to this patch (option 2 above) but without
adding a 'partial-data' key.

But you've convinced me that option 3 (supporting a compact branch
representation AND supporting missing branches as defaulting to an empty
type) is more of a maintenance burden (any time there is more than one
way to write something, it requires more testing that both ways continue
to work) and thus not worth doing without strong evidence that we need
both ways (which we do not currently have).

--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org

Markus Armbruster

2018-05-15 17:40:04 UTC

Permalink

Post by Eric Blake

Post by Markus Armbruster

I followed the arguments up to the last sentence, but then I got lost
This patch is not an overall win, so let's drop it and keep status quo
and/or implement a way to write 'branch':{} (option 1 above)
or
Forcing repetition is not an overall win, so let's drop that
requirement by using something similar to this patch (option 2 above)
but without adding a 'partial-data' key.

Sorry about that. I meant the latter.

Post by Eric Blake
But you've convinced me that option 3 (supporting a compact branch
representation AND supporting missing branches as defaulting to an
empty type) is more of a maintenance burden (any time there is more
than one way to write something, it requires more testing that both
ways continue to work) and thus not worth doing without strong
evidence that we need both ways (which we do not currently have).

Anton Nefedov

2018-05-16 15:05:13 UTC

Permalink

Post by Markus Armbruster

Post by Eric Blake

Post by Markus Armbruster

Sorry about that. I meant the latter.

I agree that neither option 3 nor this patch are the best way to handle
this, so it's 1 or 2.

(2) sure brings some prettiness into jsons; I wonder when it might harm;
e.g. a person adds another block driver: it would be difficult to get
round BlockdevOptionsFoo, and what can be missed is something
optional like BlockdevStatsFoo, which is harmless if left empty and
probably would be made an empty branch anyway. The difference is that
an empty branch is something one might notice during a review and
object.

I think I'd vote for 2 (never enforce all-branches coverage) as well.

Anton Nefedov

2018-05-11 09:05:34 UTC

Permalink

the patch provides an example of a previously introduced data-partial
qapi union tag

Signed-off-by: Anton Nefedov <***@virtuozzo.com>
---
qapi/misc.json | 48 ++++--------------------------------------------
cpus.c | 2 --
2 files changed, 4 insertions(+), 46 deletions(-)

diff --git a/qapi/misc.json b/qapi/misc.json
index f5988cc..2059852 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -387,14 +387,14 @@
'qom_path': 'str', 'thread_id': 'int',
'*props': 'CpuInstanceProperties', 'arch': 'CpuInfoArch' },
'discriminator': 'arch',
+ 'data-partial': true,
'data': { 'x86': 'CpuInfoX86',
'sparc': 'CpuInfoSPARC',
'ppc': 'CpuInfoPPC',
'mips': 'CpuInfoMIPS',
'tricore': 'CpuInfoTricore',
's390': 'CpuInfoS390',
- 'riscv': 'CpuInfoRISCV',
- 'other': 'CpuInfoOther' } }
+ 'riscv': 'CpuInfoRISCV' } }

##
# @CpuInfoX86:
@@ -465,16 +465,6 @@
{ 'struct': 'CpuInfoRISCV', 'data': { 'pc': 'int' } }

##
-# @CpuInfoOther:
-#
-# No additional information is available about the virtual CPU
-#
-# Since: 2.6
-#
-##
-{ 'struct': 'CpuInfoOther', 'data': { } }
-
-##
# @CpuS390State:
#
# An enumeration of cpu states that can be assumed by a virtual
@@ -575,38 +565,8 @@
'arch' : 'CpuInfoArch',
'target' : 'SysEmuTarget' },
'discriminator' : 'target',
- 'data' : { 'aarch64' : 'CpuInfoOther',
- 'alpha' : 'CpuInfoOther',
- 'arm' : 'CpuInfoOther',
- 'cris' : 'CpuInfoOther',
- 'hppa' : 'CpuInfoOther',
- 'i386' : 'CpuInfoOther',
- 'lm32' : 'CpuInfoOther',
- 'm68k' : 'CpuInfoOther',
- 'microblaze' : 'CpuInfoOther',
- 'microblazeel' : 'CpuInfoOther',
- 'mips' : 'CpuInfoOther',
- 'mips64' : 'CpuInfoOther',
- 'mips64el' : 'CpuInfoOther',
- 'mipsel' : 'CpuInfoOther',
- 'moxie' : 'CpuInfoOther',
- 'nios2' : 'CpuInfoOther',
- 'or1k' : 'CpuInfoOther',
- 'ppc' : 'CpuInfoOther',
- 'ppc64' : 'CpuInfoOther',
- 'ppcemb' : 'CpuInfoOther',
- 'riscv32' : 'CpuInfoOther',
- 'riscv64' : 'CpuInfoOther',
- 's390x' : 'CpuInfoS390',
- 'sh4' : 'CpuInfoOther',
- 'sh4eb' : 'CpuInfoOther',
- 'sparc' : 'CpuInfoOther',
- 'sparc64' : 'CpuInfoOther',
- 'tricore' : 'CpuInfoOther',
- 'unicore32' : 'CpuInfoOther',
- 'x86_64' : 'CpuInfoOther',
- 'xtensa' : 'CpuInfoOther',
- 'xtensaeb' : 'CpuInfoOther' } }
+ 'data-partial' : true,
+ 'data' : { 's390x' : 'CpuInfoS390' } }

##
# @query-cpus-fast:
diff --git a/cpus.c b/cpus.c
index 5bcd3ec..bd0de44 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2273,8 +2273,6 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
info->value->target = target;
if (target == SYS_EMU_TARGET_S390X) {
cpustate_to_cpuinfo_s390(&info->value->u.s390x, cpu);
- } else {
- /* do nothing for @CpuInfoOther */
}

if (!cur_item) {

--
2.7.4