[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 2/2] mirror: fix improperly filled
From: |
Jeff Cody |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 2/2] mirror: fix improperly filled copy_bitmap for mirror block job |
Date: |
Tue, 13 Sep 2016 01:03:23 -0400 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Fri, Sep 09, 2016 at 03:31:48PM +0300, Denis V. Lunev wrote:
> bdrv_is_allocated_above() returns true in the case even for completel
> zeroed areas as BDRV_BLOCK_ALLOCATED flag is set in both cases.
>
> The patch stops using bdrv_is_allocated_above() wrapper and switches to
> bdrv_get_block_status_above() to distinguish zeroed areas and areas with
> data to avoid extra IO operations if possible.
>
> Signed-off-by: Denis V. Lunev <address@hidden>
> CC: Stefan Hajnoczi <address@hidden>
> CC: Fam Zheng <address@hidden>
> CC: Kevin Wolf <address@hidden>
> CC: Max Reitz <address@hidden>
> CC: Jeff Cody <address@hidden>
> ---
> block/mirror.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index e0b3f41..da55375 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -548,14 +548,15 @@ static void mirror_throttle(MirrorBlockJob *s)
>
> static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s)
> {
> - int64_t sector_num, end;
> + int64_t sector_num, end, alloc_mask;
> BlockDriverState *base = s->base;
> BlockDriverState *bs = blk_bs(s->common.blk);
> BlockDriverState *target_bs = blk_bs(s->target);
> - int ret, n;
> + int n;
>
> end = s->bdev_length / BDRV_SECTOR_SIZE;
>
> + alloc_mask = BDRV_BLOCK_ALLOCATED;
> if (base == NULL && !bdrv_has_zero_init(target_bs)) {
> if (!bdrv_can_write_zeroes_with_unmap(target_bs)) {
> bdrv_set_dirty_bitmap(s->dirty_bitmap, 0, end);
> @@ -583,6 +584,8 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob
> *s)
> }
>
> mirror_drain(s);
> +
> + alloc_mask = BDRV_BLOCK_DATA;
What about when base == NULL, and bdrv_has_zero_init(target_bs) == true? In
that case we also know the target image is zeroed, but this does not take
advantage of that.
> }
>
> /* First part, loop on the sectors and initialize the dirty bitmap. */
> @@ -590,6 +593,8 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob
> *s)
> /* Just to make sure we are not exceeding int limit. */
> int nb_sectors = MIN(INT_MAX >> BDRV_SECTOR_BITS,
> end - sector_num);
> + int64_t status;
> + BlockDriverState *file;
>
> mirror_throttle(s);
>
> @@ -597,13 +602,14 @@ static int coroutine_fn
> mirror_dirty_init(MirrorBlockJob *s)
> return 0;
> }
>
> - ret = bdrv_is_allocated_above(bs, base, sector_num, nb_sectors, &n);
> - if (ret < 0) {
> - return ret;
> + status = bdrv_get_block_status_above(bs, base, sector_num,
> + nb_sectors, &n, &file);
> + if (status < 0) {
> + return status;
> }
>
> assert(n > 0);
> - if (ret == 1) {
> + if (status & alloc_mask) {
> bdrv_set_dirty_bitmap(s->dirty_bitmap, sector_num, n);
> }
> sector_num += n;
-Jeff