Eric Blake <eblake@xxxxxxxxxx> 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@xxxxxxxxxx>
>> ---
>> 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.
