Re: [PATCH v2 3/3] qapi: deprecate drive-backup
Vladimir Sementsov-Ogievskiy <vsementsov(a)virtuozzo.com> writes:
08.06.2021 14:12, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov(a)virtuozzo.com> 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.