Re: [PATCH v2 1/4] qapi/block-core: avoid the re-use of MirrorSyncMode for backup

[Markus Armbruster]

Fiona Ebner <f.ebner@proxmox.com> writes:

> Backup supports all modes listed in MirrorSyncMode, while mirror does
> not. Introduce BackupSyncMode by copying the current MirrorSyncMode
> and drop the variants mirror does not support from MirrorSyncMode as
> well as the corresponding manual check in mirror_start().

Results in tighter introspection: query-qmp-schema no longer reports
drive-mirror and blockdev-mirror accepting @sync values they actually
reject.  Suggest to mention this in the commit message.

> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>
> I felt like keeping the "Since: X.Y" as before makes the most sense as
> to not lose history. Or is it necessary to change this for
> BackupSyncMode (and its members) since it got a new name?

Doc comments are for users of the QMP interface.  Type names do not
matter there.  I agree with your decision not to update the "since"
tags.

[...]

> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 1874f880a8..59d75b0793 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1304,10 +1304,10 @@
>    'data': ['report', 'ignore', 'enospc', 'stop', 'auto'] }
>  
>  ##
> -# @MirrorSyncMode:
> +# @BackupSyncMode:
>  #
> -# An enumeration of possible behaviors for the initial synchronization
> -# phase of storage mirroring.
> +# An enumeration of possible behaviors for image synchronization used
> +# by backup jobs.
>  #
>  # @top: copies data in the topmost image to the destination
>  #
> @@ -1323,7 +1323,7 @@
>  #
>  # Since: 1.3
>  ##
> -{ 'enum': 'MirrorSyncMode',
> +{ 'enum': 'BackupSyncMode',
>    'data': ['top', 'full', 'none', 'incremental', 'bitmap'] }
>  
>  ##
> @@ -1347,6 +1347,23 @@
>  { 'enum': 'BitmapSyncMode',
>    'data': ['on-success', 'never', 'always'] }
>  
> +##
> +# @MirrorSyncMode:
> +#
> +# An enumeration of possible behaviors for the initial synchronization
> +# phase of storage mirroring.
> +#
> +# @top: copies data in the topmost image to the destination
> +#
> +# @full: copies data from all images to the destination
> +#
> +# @none: only copy data written from now on
> +#
> +# Since: 1.3
> +##
> +{ 'enum': 'MirrorSyncMode',
> +  'data': ['top', 'full', 'none'] }
> +
>  ##
>  # @MirrorCopyMode:
>  #
> @@ -1624,7 +1641,7 @@
>  ##
>  { 'struct': 'BackupCommon',
>    'data': { '*job-id': 'str', 'device': 'str',
> -            'sync': 'MirrorSyncMode', '*speed': 'int',
> +            'sync': 'BackupSyncMode', '*speed': 'int',
>              '*bitmap': 'str', '*bitmap-mode': 'BitmapSyncMode',
>              '*compress': 'bool',
>              '*on-source-error': 'BlockdevOnError',

QAPI schema
Acked-by: Markus Armbruster <armbru@redhat.com>