| Message ID | 20210913123932.3306639-11-armbru@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qapi: Remove simple unions from the schema language | expand |
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 <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!
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>
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' } } ##
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(-)