Re: [PATCH qemu 1/4] drive-mirror: add support for sync=bitmap mode=never

[Markus Armbruster <armbru@redhat.com>]

Fabian Grünbichler <f.gruenbichler@proxmox.com> writes:

> From: John Snow <jsnow@redhat.com>
>
> This patch adds support for the "BITMAP" sync mode to drive-mirror and
> blockdev-mirror. It adds support only for the BitmapSyncMode "never,"
> because it's the simplest mode.
>
> This mode simply uses a user-provided bitmap as an initial copy
> manifest, and then does not clear any bits in the bitmap at the
> conclusion of the operation.
>
> Any new writes dirtied during the operation are copied out, in contrast
> to backup. Note that whether these writes are reflected in the bitmap
> at the conclusion of the operation depends on whether that bitmap is
> actually recording!
>
> This patch was originally based on one by Ma Haocong, but it has since
> been modified pretty heavily.
>
> Suggested-by: Ma Haocong <mahaocong@didichuxing.com>
> Signed-off-by: Ma Haocong <mahaocong@didichuxing.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
[...]
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 2d94873ca0..dac5497084 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1961,10 +1961,19 @@
>  #        (all the disk, only the sectors allocated in the topmost image, or
>  #        only new I/O).
>  #
> +# @bitmap: The name of a bitmap to use for sync=bitmap mode. This argument must
> +#          be present for bitmap mode and absent otherwise. The bitmap's

What is "bitmap mode"?  Do you mean "present when @bitmap-mode is, else
absent"?

> +#          granularity is used instead of @granularity (since 5.2).
> +#
> +# @bitmap-mode: Specifies the type of data the bitmap should contain after
> +#               the operation concludes. Must be present if sync is "bitmap".
> +#               Must NOT be present otherwise. (Since 5.2)
> +#
>  # @granularity: granularity of the dirty bitmap, default is 64K
>  #               if the image format doesn't have clusters, 4K if the clusters
>  #               are smaller than that, else the cluster size.  Must be a
> -#               power of 2 between 512 and 64M (since 1.4).
> +#               power of 2 between 512 and 64M. Must not be specified if
> +#               @bitmap is present (since 1.4).
>  #

Is @granularity forbidden with @bitmap because it makes no sense?

If yes, then it doesn't actually default to anything then, does it?

Looks like we have

    sync            'bitmap'            anything else
    -------------------------------------------------
    bitmap          required            forbidden
    bitmap-mode     required            forbidden
    granularity     forbidden           optional

Correct?

>  # @buf-size: maximum amount of data in flight from source to
>  #            target (since 1.4).
> @@ -2002,7 +2011,9 @@
>  { 'struct': 'DriveMirror',
>    'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
>              '*format': 'str', '*node-name': 'str', '*replaces': 'str',
> -            'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
> +            'sync': 'MirrorSyncMode', '*bitmap': 'str',
> +            '*bitmap-mode': 'BitmapSyncMode',
> +            '*mode': 'NewImageMode',
>              '*speed': 'int', '*granularity': 'uint32',
>              '*buf-size': 'int', '*on-source-error': 'BlockdevOnError',
>              '*on-target-error': 'BlockdevOnError',
[Same for blockdev-mirror...]