From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60561) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bnODi-0004cm-Dh for qemu-devel@nongnu.org; Fri, 23 Sep 2016 07:00:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bnODf-0006wJ-O6 for qemu-devel@nongnu.org; Fri, 23 Sep 2016 07:00:42 -0400 Received: from mailhub.sw.ru ([195.214.232.25]:13817 helo=relay.sw.ru) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bnODf-0006w2-Bk for qemu-devel@nongnu.org; Fri, 23 Sep 2016 07:00:39 -0400 References: <1473957290-13382-1-git-send-email-den@openvz.org> <1473957290-13382-3-git-send-email-den@openvz.org> From: Vladimir Sementsov-Ogievskiy Message-ID: <57E50B44.3080809@virtuozzo.com> Date: Fri, 23 Sep 2016 14:00:20 +0300 MIME-Version: 1.0 In-Reply-To: <1473957290-13382-3-git-send-email-den@openvz.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 2/2] mirror: fix improperly filled copy_bitmap for mirror block job List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Denis V. Lunev" , qemu-block@nongnu.org, qemu-devel@nongnu.org Cc: Kevin Wolf , Fam Zheng , Jeff Cody , Max Reitz , Stefan Hajnoczi On 15.09.2016 19:34, 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 > CC: Stefan Hajnoczi > CC: Fam Zheng > CC: Kevin Wolf > CC: Max Reitz > CC: Jeff Cody > --- > block/mirror.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/block/mirror.c b/block/mirror.c > index e0b3f41..2710f62 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -548,11 +548,11 @@ 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; > > @@ -585,11 +585,15 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) > mirror_drain(s); > } > > + alloc_mask = base == NULL ? BDRV_BLOCK_ALLOCATED : BDRV_BLOCK_DATA; should not s/==/!= ? > + > /* First part, loop on the sectors and initialize the dirty bitmap. */ > for (sector_num = 0; sector_num < end; ) { > /* 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 +601,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; -- Best regards, Vladimir