3:57 a.m.
Eric Blake <eblake(a)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(a)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.