On October 2, 2020 9:06 am, Markus Armbruster wrote:
> Fabian Grünbichler <f.gruenbich...@proxmox.com> writes:
>
>> From: John Snow <js...@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 <mahaoc...@didichuxing.com>
>> Signed-off-by: Ma Haocong <mahaoc...@didichuxing.com>
>> Signed-off-by: John Snow <js...@redhat.com>
>> Signed-off-by: Fabian Grünbichler <f.gruenbich...@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"?
bitmap mode is sync=bitmap , as in the first sentence. if you set
sync=bitmap, you must specify a bitmap and a bitmap-mode. if you use
another sync mode, you must not specify a bitmap or a bitmap-mode.
there is also a 'sugar' sync mode 'incremental' that desugars to
sync=bitmap with bitmap-mode=on-success. I guess that should also be
mentioned somewhere in QAPI, it's mainly there since MirrorSyncMode has
it as possible value, it's semantics are straight-forward to map onto
this combination, and it's how the sync modes are known from backup
jobs.
maybe the following is easier to understand and more aligned with
bitmap-mode:
The name of the bitmap to use for sync=bitmap/sync=incremental mode.
Must be present if sync is "bitmap" or "incremental". Must NOT be
present otherwise.
>> +# 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)
Specifies the type of data the bitmap should contain after the operation
concludes. Must be present if sync is "bitmap". Must be "on-success" or
absent if sync is "incremental". Must NOT be present otherwise.
>> +#
>> # @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?
yes.
>
> If yes, then it doesn't actually default to anything then, does it?
we must use the same granularity as the sync bitmap passed in via
'bitmap', so the caller can't set a different one.
> Looks like we have
>
> sync 'bitmap' anything else
> -------------------------------------------------
> bitmap required forbidden
> bitmap-mode required forbidden
> granularity forbidden optional
>
> Correct?
yes. with the addition of sync=incremental as subset of sync=bitmap, as
described above.
>
>> # @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...]
>
>
>
>
