Re: [PATCH 03/22] qapi: Convert simple union KeyValue to flat one

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On Mon, Sep 13, 2021 at 02:39:13PM +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 KeyValue 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: Gerd Hoffmann <kraxel@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  qapi/ui.json | 30 ++++++++++++++++++++++++++++--
>>  1 file changed, 28 insertions(+), 2 deletions(-)
>> 
>> diff --git a/qapi/ui.json b/qapi/ui.json
>> index b2cf7a6759..a6b0dce876 100644
>> --- a/qapi/ui.json
>> +++ b/qapi/ui.json
>> @@ -824,6 +824,30 @@
>>              'ac_home', 'ac_back', 'ac_forward', 'ac_refresh', 
>> 'ac_bookmarks',
>>              'lang1', 'lang2' ] }
>>  
>> +##
>> +# @KeyValueKind:
>> +#
>> +# Since: 6.1
>
> 6.2 now?  Or should this be...

Yes.  Can't count :)

>> +
>>  ##
>>  # @KeyValue:
>>  #
>> @@ -832,9 +856,11 @@
>>  # Since: 1.3
>
> ...1.3, since the type has been around by that name already (albeit
> implicitly) since that older release?

I'll change it to 1.3.

My first version had KeyValueType here.  Then I found the renaming of
its uses in C code tedious, and realized I could avoid it by unreserving
*Kind type names early.  I forgot to adjust the Since tag.

>>  ##
>>  { 'union': 'KeyValue',
>> +  'base': { 'type': 'KeyValueKind' },
>> +  'discriminator': 'type',
>>    'data': {
>> -    'number': 'int',
>> -    'qcode': 'QKeyCode' } }
>> +    'number': 'IntWrapper',
>> +    'qcode': 'QKeyCodeWrapper' } }
>>
>
> I'll trust your decision on the documentation issue; the conversion
> itself is sane, so I'm fine with:
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!