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

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.

3. Versioned query-qmp-schema.

   query-qmp-schema provides either @values or @members.  The QMP
   client can select which version it wants.

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.
+#
+# @values: the enumeration type's member names, in no particular order.
+#          Redundant with @members.  Just for backward compatibility.
 #
 # 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
+##
+{ 'struct': 'SchemaInfoEnumMember',
+  'data': { 'name': 'str' } }
 
 ##
 # @SchemaInfoArray:
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 4c079ee627..6334546363 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -68,6 +68,7 @@
 # TypedDict constructs, so they are broadly typed here as simple
 # Python Dicts.
 SchemaInfo = Dict[str, object]
+SchemaInfoEnumMember = Dict[str, object]
 SchemaInfoObject = Dict[str, object]
 SchemaInfoObjectVariant = Dict[str, object]
 SchemaInfoObjectMember = Dict[str, object]
@@ -274,8 +275,16 @@ def _gen_tree(self, name: str, mtype: str, obj: Dict[str, object],
             obj['features'] = self._gen_features(features)
         self._trees.append(Annotated(obj, ifcond, comment))
 
-    def _gen_member(self, member: QAPISchemaObjectTypeMember
-                    ) -> Annotated[SchemaInfoObjectMember]:
+    @staticmethod
+    def _gen_enum_member(member: QAPISchemaEnumMember
+                         ) -> Annotated[SchemaInfoEnumMember]:
+        obj: SchemaInfoEnumMember = {
+            'name': member.name,
+        }
+        return Annotated(obj, member.ifcond)
+
+    def _gen_object_member(self, member: QAPISchemaObjectTypeMember
+                           ) -> Annotated[SchemaInfoObjectMember]:
         obj: SchemaInfoObjectMember = {
             'name': member.name,
             'type': self._use_type(member.type)
@@ -305,7 +314,8 @@ def visit_enum_type(self, name: str, info: Optional[QAPISourceInfo],
                         prefix: Optional[str]) -> None:
         self._gen_tree(
             name, 'enum',
-            {'values': [Annotated(m.name, m.ifcond) for m in members]},
+            {'members': [self._gen_enum_member(m) for m in members],
+             'values': [Annotated(m.name, m.ifcond) for m in members]},
             ifcond, features
         )
 
@@ -322,7 +332,7 @@ def visit_object_type_flat(self, name: str, info: Optional[QAPISourceInfo],
                                members: List[QAPISchemaObjectTypeMember],
                                variants: Optional[QAPISchemaVariants]) -> None:
         obj: SchemaInfoObject = {
-            'members': [self._gen_member(m) for m in members]
+            'members': [self._gen_object_member(m) for m in members]
         }
         if variants:
             obj['tag'] = variants.tag_member.name
-- 
2.31.1

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

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.