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(-)

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
+##
+{ 'enum': 'TransactionActionKind',
+  'data': [ 'abort', 'block-dirty-bitmap-add', 'block-dirty-bitmap-remove',
+            'block-dirty-bitmap-clear', 'block-dirty-bitmap-enable',
+            'block-dirty-bitmap-disable', 'block-dirty-bitmap-merge',
+            'blockdev-backup', 'blockdev-snapshot',
+            'blockdev-snapshot-internal-sync', 'blockdev-snapshot-sync',
+            'drive-backup' ] }
+
+##
+# @AbortWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'AbortWrapper',
+  'data': { 'data': 'Abort' } }
+
+##
+# @BlockDirtyBitmapAddWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'BlockDirtyBitmapAddWrapper',
+  'data': { 'data': 'BlockDirtyBitmapAdd' } }
+
+##
+# @BlockDirtyBitmapWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'BlockDirtyBitmapWrapper',
+  'data': { 'data': 'BlockDirtyBitmap' } }
+
+##
+# @BlockDirtyBitmapMergeWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'BlockDirtyBitmapMergeWrapper',
+  'data': { 'data': 'BlockDirtyBitmapMerge' } }
+
+##
+# @BlockdevBackupWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'BlockdevBackupWrapper',
+  'data': { 'data': 'BlockdevBackup' } }
+
+##
+# @BlockdevSnapshotWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'BlockdevSnapshotWrapper',
+  'data': { 'data': 'BlockdevSnapshot' } }
+
+##
+# @BlockdevSnapshotInternalWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'BlockdevSnapshotInternalWrapper',
+  'data': { 'data': 'BlockdevSnapshotInternal' } }
+
+##
+# @BlockdevSnapshotSyncWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'BlockdevSnapshotSyncWrapper',
+  'data': { 'data': 'BlockdevSnapshotSync' } }
+
+##
+# @DriveBackupWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'DriveBackupWrapper',
+  'data': { 'data': 'DriveBackup' } }
+
 ##
 # @TransactionAction:
 #
@@ -60,19 +145,21 @@
 # Since: 1.1
 ##
 { 'union': 'TransactionAction',
+  'base': { 'type': 'TransactionActionKind' },
+  'discriminator': 'type',
   'data': {
-       'abort': 'Abort',
-       'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd',
-       'block-dirty-bitmap-remove': 'BlockDirtyBitmap',
-       'block-dirty-bitmap-clear': 'BlockDirtyBitmap',
-       'block-dirty-bitmap-enable': 'BlockDirtyBitmap',
-       'block-dirty-bitmap-disable': 'BlockDirtyBitmap',
-       'block-dirty-bitmap-merge': 'BlockDirtyBitmapMerge',
-       'blockdev-backup': 'BlockdevBackup',
-       'blockdev-snapshot': 'BlockdevSnapshot',
-       'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
-       'blockdev-snapshot-sync': 'BlockdevSnapshotSync',
-       'drive-backup': 'DriveBackup'
+       'abort': 'AbortWrapper',
+       'block-dirty-bitmap-add': 'BlockDirtyBitmapAddWrapper',
+       'block-dirty-bitmap-remove': 'BlockDirtyBitmapWrapper',
+       'block-dirty-bitmap-clear': 'BlockDirtyBitmapWrapper',
+       'block-dirty-bitmap-enable': 'BlockDirtyBitmapWrapper',
+       'block-dirty-bitmap-disable': 'BlockDirtyBitmapWrapper',
+       'block-dirty-bitmap-merge': 'BlockDirtyBitmapMergeWrapper',
+       'blockdev-backup': 'BlockdevBackupWrapper',
+       'blockdev-snapshot': 'BlockdevSnapshotWrapper',
+       'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternalWrapper',
+       'blockdev-snapshot-sync': 'BlockdevSnapshotSyncWrapper',
+       'drive-backup': 'DriveBackupWrapper'
    } }
 
 ##
-- 
2.31.1

On 13.09.21 14:39, 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(-)

Acked-by: Hanna Reitz <hreitz@redhat.com>

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).

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

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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!