Markus Armbruster <armbru@xxxxxxxxxx> writes:
> Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxx> writes:
>
>> 08.06.2021 14:12, Markus Armbruster wrote:
>>> Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxx> writes:
>>>
>>> [...]
>>>
>>>> TODO: We also need to deprecate drive-backup transaction action..
>>>> But union members in QAPI doesn't support 'deprecated' feature. I tried
>>>> to dig a bit, but failed :/ Markus, could you please help with it? At
>>>> least by advice?
>>>
>>> There are two closely related things in play here: the union branch and
>>> the corresponding enum value.
>>>
>>> So far, the QAPI schema language doesn't support tacking feature flags
>>> to either.
>>>
>>> If an enum value is deprecated, any union branches corresponding to it
>>> must also be deprecated (because their use requires using the deprecated
>>> enum value).
>>>
>>> The converse is not true, but I can't see a use for deprecating a union
>>> branch without also deprecating the enum member.
>>>
>>> I think we can implement feature flags just for enum members, then
>>> document that 'deprecated' enum value implies corresponding union
>>> branches are also deprecated.
>>>
>>> I have unfinished patches implementing feature flags for enum members.
>>>
>>> Since TransactionAction is a simple union, the corresponding enum is
>>> implicit. We can make it explicit by converting to a flat union.
>>> Simple unions need to die anyway.
>>
>>
>> Does BlockStatsSpecific from qapi/block-core.json a correct example of flat union you mean? I can make patch to convert TransactionAction to be similar if that helps (discriminator field should be called "type", yes?).
>
> From docs/devel/qapi-code-gen.txt:
>
> A simple union can always be re-written as a flat union where the base
> class has a single member named 'type', and where each branch of the
> union has a struct with a single member named 'data'. That is,
>
> { 'union': 'Simple', 'data': { 'one': 'str', 'two': 'int' } }
>
> is identical on the wire to:
>
> { 'enum': 'Enum', 'data': ['one', 'two'] }
> { 'struct': 'Branch1', 'data': { 'data': 'str' } }
> { 'struct': 'Branch2', 'data': { 'data': 'int' } }
> { 'union': 'Flat', 'base': { 'type': 'Enum' }, 'discriminator': 'type',
> 'data': { 'one': 'Branch1', 'two': 'Branch2' } }
>
> The generated C isn't identical, but adjusting the code using it should
> be straightforward.
>
>>> Does this make sense?
>>>
>>
>> Yes if it helps)
>>
>> Did you also look at John's https://gitlab.com/jsnow/qemu/-/commits/hack-deprecate-union-branches/ ?
>
> Not yet.
>
>> I hope you and John will send patches that you have, I'll help with reviewing (keep me in CC), and finally we'll get the feature.
>
> Sounds like a plan. I need to get my post-vacation e-mail pileup under
> control first.
Just sent:
Subject: [PATCH RFC 0/5] Subject: [PATCH RFC 0/5] qapi: Add feature flags to enum members
Message-Id: <20210915192425.4104210-1-armbru@xxxxxxxxxx>
Yes, I mangled the subject %-/
