Eric Blake <ebl...@redhat.com> writes:
> On 3/15/20 9:46 AM, Markus Armbruster wrote:
>> In v4.1.0, we added feature flags just to struct types (commit
>> 6a8c0b5102^..f3ed93d545), to satisfy an immediate need (commit
>> c9d4070991 "file-posix: Add dynamic-auto-read-only QAPI feature"). In
>> v4.2.0, we added them to commands (commit 23394b4c39 "qapi: Add
>> feature flags to commands") to satisfy another immediate need (commit
>> d76744e65e "qapi: Allow introspecting fix for savevm's cooperation
>> with blockdev").
>>
>> Add them to the remaining definitions: enumeration types, union types,
>> alternate types, and events.
>>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>
>
>> +++ b/qapi/introspect.json
>> @@ -89,12 +89,18 @@
>> #
>> # @meta-type: the entity's meta type, inherited from @base.
>> #
>> +# @features: names of features associated with the entity, in no
>> +# particular order.
>> +# (since 4.1 for object types, 4.2 for commands, 5.0 for
>> +# the rest)
>
> Odd versioning hint, but accurate, and I don't see any way to improve it.
>
>> +#
>> # Additional members depend on the value of @meta-type.
>> #
>> # Since: 2.5
>> ##
>> { 'union': 'SchemaInfo',
>> - 'base': { 'name': 'str', 'meta-type': 'SchemaMetaType' },
>> + 'base': { 'name': 'str', 'meta-type': 'SchemaMetaType',
>> + '*features': [ 'str' ] },
>> 'discriminator': 'meta-type',
>> 'data': {
>> 'builtin': 'SchemaInfoBuiltin',
>> @@ -174,9 +180,6 @@
>> # and may even differ from the order of the values of the
>> # enum type of the @tag.
>> #
>> -# @features: names of features associated with the type, in no particular
>> -# order. (since: 4.1)
>> -#
>> # Values of this type are JSON object on the wire.
>> #
>> # Since: 2.5
>> @@ -184,8 +187,7 @@
>> { 'struct': 'SchemaInfoObject',
>> 'data': { 'members': [ 'SchemaInfoObjectMember' ],
>> '*tag': 'str',
>> - '*variants': [ 'SchemaInfoObjectVariant' ],
>> - '*features': [ 'str' ] } }
>> + '*variants': [ 'SchemaInfoObjectVariant' ] } }
>
> The code motion from use in some of the union branches to now being
> present in the base class of all of the branches is
> backwards-compatible.
>
> The generator changes also look correct, and have enough testsuite
> coverage to make it easier to be confident about the patch.
>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
>
>
>> +++ b/tests/qapi-schema/doc-good.json
>> @@ -53,10 +53,14 @@
>> # @Enum:
>> # @one: The _one_ {and only}
>> #
>> +# Features:
>> +# @enum-feat: Also _one_ {and only}
+#
# @two is undocumented
##
{ 'enum': 'Enum', 'data':
[ { 'name': 'one', 'if': 'defined(IFONE)' }, 'two' ],
+ 'features': [ 'enum-feat' ],
'if': 'defined(IFCOND)' }
>
> All our existing public features are a single word (matching naming
> conventions elsewhere in QAPI). Are we sure we want to allow feature
> names that include whitespace? Of course, the fact that our testsuite
> covers it (even if we don't use it publically) means that we are sure
> that our generator can handle it, regardless of whether we decide that
> a separate patch should restrict feature names. But I don't see it
> holding up this patch.
We definitely do not want to exempt feature names from the QAPI naming
rules.
The code enforces this. If I change '-' to ' ' in 'features': [
'enum-feat' ], I get
doc-good.json:61: 'features' member 'enum feat' has an invalid name
Thanks!
