Eric Blake <eblake@redhat.com> writes:
On Wed, Sep 15, 2021 at 09:24:21PM +0200, Markus Armbruster wrote:
The next commit will add feature flags to enum members. There's a problem, though: query-qmp-schema shows an enum type's members as an array of member names (SchemaInfoEnum member @values). If it showed an array of objects with a name member, we could simply add more members to these objects. Since it's just strings, we can't.
I can see three ways to correct this design mistake:
1. Do it the way we should have done it, plus compatibility goo.
We want a ['SchemaInfoEnumMember'] member in SchemaInfoEnum. Since changing @values would be a compatibility break, add a new member @members instead.
@values is now redundant. We should be able to get rid of it eventually.
In my testing, output of qemu-system-x86_64's query-qmp-schema grows by 11% (18.5KiB).
This makes sense if we plan to deprecate @values - if so, that deprecation would make sense as part of this series, although we may drag our feet for how long before we actually remove it.
Yes. Changing query-qmp-schema requires extra care, as it is the very means for coping with change.
2. Like 1, but omit "boring" elements of @member, and empty @member.
@values does not become redundant. Output of query-qmp-schema grows only as we make enum members non-boring.
Does not change whether libvirt would have to learn to query the new members, but adds a mandatory fallback step for learning about boring members (although the fallback step will have to be there anyway for older qemu). Peter probably has a better idea of which version is nicer.
3. Versioned query-qmp-schema.
query-qmp-schema provides either @values or @members. The QMP client can select which version it wants.
Sounds more complicated to implement. I'm not opposed to it, but am leaning towards 1 or 2 myself.
More on this in my reply to Peter.
This commit implements 1. simply because it's the solution I thought of first. I'm prepared to implement one of the others if we decide that's what we want.
Signed-off-by: Markus Armbruster <armbru@redhat.com> --- qapi/introspect.json | 20 ++++++++++++++++++-- scripts/qapi/introspect.py | 18 ++++++++++++++---- 2 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/qapi/introspect.json b/qapi/introspect.json index 39bd303778..250748cd95 100644 --- a/qapi/introspect.json +++ b/qapi/introspect.json @@ -142,14 +142,30 @@ # # Additional SchemaInfo members for meta-type 'enum'. # -# @values: the enumeration type's values, in no particular order. +# @members: the enum type's members, in no particular order.
Missing a '(since 6.2)' tag.
Yes.
+# +# @values: the enumeration type's member names, in no particular order. +# Redundant with @members. Just for backward compatibility.
Worth marking as deprecated in this patch, or in a followup?
If we intend to deprecate, we can just as well do it right away.
# # Values of this type are JSON string on the wire. # # Since: 2.5 ## { 'struct': 'SchemaInfoEnum', - 'data': { 'values': ['str'] } } + 'data': { 'members': [ 'SchemaInfoEnumMember' ], + 'values': ['str'] } } + +## +# @SchemaInfoEnumMember: +# +# An object member. +# +# @name: the member's name, as defined in the QAPI schema. +# +# Since: 6.1
6.2
Whoops!
+## +{ 'struct': 'SchemaInfoEnumMember', + 'data': { 'name': 'str' } }
Definitely more flexible.