Re: [PATCH v2 3/3] qapi: deprecate drive-backup

Vladimir Sementsov-Ogievskiy <vsementsov(a)virtuozzo.com&gt; writes: > 08.06.2021 14:12, Markus Armbruster wrote: >> Vladimir Sementsov-Ogievskiy <vsementsov(a)virtuozzo.com&gt; 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.