Re: [PATCH 10/22] qapi: Convert simple union TransactionAction to flat one

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On Mon, Sep 13, 2021 at 02:39:20PM +0200, Markus Armbruster wrote:
>> Simple unions predate flat unions.  Having both complicates the QAPI
>> schema language and the QAPI generator.  We haven't been using simple
>> unions in new code for a long time, because they are less flexible and
>> somewhat awkward on the wire.
>> 
>> To prepare for their removal, convert simple union TransactionAction
>> to an equivalent flat one.  Adds some boilerplate to the schema, which
>> is a bit ugly, but a lot easier to maintain than the simple union
>> feature.
>> 
>> Cc: Kevin Wolf <kwolf@redhat.com>
>> Cc: Hanna Reitz <hreitz@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  qapi/transaction.json | 111 +++++++++++++++++++++++++++++++++++++-----
>>  1 file changed, 99 insertions(+), 12 deletions(-)
>
> Same comments for each of 5-10 as for 4; the conversion is sane, and
> the only question is on documentation, whether you want...
>
>> 
>> diff --git a/qapi/transaction.json b/qapi/transaction.json
>> index 894258d9e2..d7fc73d7df 100644
>> --- a/qapi/transaction.json
>> +++ b/qapi/transaction.json
>> @@ -38,6 +38,91 @@
>>  { 'enum': 'ActionCompletionMode',
>>    'data': [ 'individual', 'grouped' ] }
>>  
>> +##
>> +# @TransactionActionKind:
>> +#
>> +# Since: 6.1
>
> ... 6.2 here, or to preserve the implicit...
>
>>  ##
>>  # @TransactionAction:
>>  #
>> @@ -60,19 +145,21 @@
>>  # Since: 1.1
>
> ...1.1 matching when the simple union was first formed (actually, this
> simple union has grown over time, which makes it trickier to decide
> which historical Since: to use on each Wrapper, so I'd lean towards
> 6.2 on all of them as being less work).

The enum becomes explicit in the schema, but is the same as before.  I
think copying "since" information from the no-longer-simple union and
its branches to the enum and its members makes sense.

The wrapper types become explicit, but with a new name.  We can copy
"since" from their branch anyway.  Doesn't really matter, unlike for the
enum.

> For patches 5-10:
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!