On 22.12.21 18:40, Vladimir Sementsov-Ogievskiy wrote:
> This will be used in the following commit to bring "incremental" mode
> to copy-before-write filter.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> include/block/block-copy.h | 2 +-
> block/block-copy.c | 14 ++++++++++++--
> block/copy-before-write.c | 2 +-
> 3 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/include/block/block-copy.h b/include/block/block-copy.h
> index 99370fa38b..8da4cec1b6 100644
> --- a/include/block/block-copy.h
> +++ b/include/block/block-copy.h
> @@ -25,7 +25,7 @@ typedef struct BlockCopyState BlockCopyState;
> typedef struct BlockCopyCallState BlockCopyCallState;
>
> BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
> - Error **errp);
> + BdrvDirtyBitmap *bitmap, Error **errp);
>
> /* Function should be called prior any actual copy request */
> void block_copy_set_copy_opts(BlockCopyState *s, bool use_copy_range,
> diff --git a/block/block-copy.c b/block/block-copy.c
> index abda7a80bd..f6345e3a4c 100644
> --- a/block/block-copy.c
> +++ b/block/block-copy.c
> @@ -384,8 +384,9 @@ static int64_t block_copy_calculate_cluster_size(BlockDriverState *target,
> }
>
> BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
> - Error **errp)
> + BdrvDirtyBitmap *bitmap, Error **errp)
Could be `const` to signal that we won’t be using this bitmap for the
BCS, but given our inconsistent usage of `const`, it isn’t anything
that’d be important.
> {
> + ERRP_GUARD();
> BlockCopyState *s;
> int64_t cluster_size;
> BdrvDirtyBitmap *copy_bitmap;
> @@ -402,7 +403,16 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
> return NULL;
> }
> bdrv_disable_dirty_bitmap(copy_bitmap);
> - bdrv_set_dirty_bitmap(copy_bitmap, 0, bdrv_dirty_bitmap_size(copy_bitmap));
> + if (bitmap) {
> + if (!bdrv_merge_dirty_bitmap(copy_bitmap, bitmap, NULL, errp)) {
> + error_prepend(errp, "Failed to merge bitmap '%s' to internal "
> + "copy-bitmap: ", bdrv_dirty_bitmap_name(bitmap));
> + return NULL;
What might be Should we free `copy_bitmap` here?
(Apart from this, looks good to me!)
> + }
> + } else {
> + bdrv_set_dirty_bitmap(copy_bitmap, 0,
> + bdrv_dirty_bitmap_size(copy_bitmap));
> + }
>
> /*
> * If source is in backing chain of target assume that target is going to be
> diff --git a/block/copy-before-write.c b/block/copy-before-write.c
> index 5bdaf0a9d9..799223e3fb 100644
> --- a/block/copy-before-write.c
> +++ b/block/copy-before-write.c
> @@ -170,7 +170,7 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
> ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
> bs->file->bs->supported_zero_flags);
>
> - s->bcs = block_copy_state_new(bs->file, s->target, errp);
> + s->bcs = block_copy_state_new(bs->file, s->target, NULL, errp);
> if (!s->bcs) {
> error_prepend(errp, "Cannot create block-copy-state: ");
> return -EINVAL;