[PATCH RFC 1/5] qapi: Enable enum member introspection to show more than name

[Markus Armbruster]

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