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

[Eric Blake <eblake@redhat.com>]

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.

> 
> 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.

> 
> 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.

> +#
> +# @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?

>  #
>  # 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

> +##
> +{ 'struct': 'SchemaInfoEnumMember',
> +  'data': { 'name': 'str' } }
>

Definitely more flexible.

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