From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34763) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1blqKk-00013l-Cu for qemu-devel@nongnu.org; Mon, 19 Sep 2016 00:37:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1blqKf-0004ZR-EP for qemu-devel@nongnu.org; Mon, 19 Sep 2016 00:37:34 -0400 References: <1473957290-13382-1-git-send-email-den@openvz.org> <1473957290-13382-2-git-send-email-den@openvz.org> <20160919012151.GA4883@lemon> From: "Denis V. Lunev" Message-ID: <960740fa-37d0-48d5-820f-ebfe8979ffa8@openvz.org> Date: Mon, 19 Sep 2016 07:37:20 +0300 MIME-Version: 1.0 In-Reply-To: <20160919012151.GA4883@lemon> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 1/2] block: sync bdrv_co_get_block_status_above() with bdrv_is_allocated_above() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, Kevin Wolf , Jeff Cody , Max Reitz , Stefan Hajnoczi On 09/19/2016 04:21 AM, Fam Zheng wrote: > On Thu, 09/15 19:34, Denis V. Lunev wrote: >> They should work very similar, covering same areas if backing store is >> shorter than the image. This change is necessary for the followup patch >> switching to bdrv_get_block_status_above() in mirror to avoid assert >> in check_block. >> >> This change should be made very carefully. Let us assume that we have >> top image and 2 backing stores L0->L1->L2. > Stupid question: which one is top and which are backing? L0 is top, L2 is at bottom. >> L0: -------------- >> L1: ------- >> L2: -------======= >> The data marked as '=' in L2 should not appear as BDRV_BLOCK_ALLOCATED >> and we should return it as filled in L0 image with properly calculated >> *pnum value. > What '-', '=' and ' ' represent aren't immediately clear to me, could you put a > legend in the message too? Something like: > > '-': allocated > '=': unallocated > ' ': beyong EOF ok. here '-' in unallocated '=' is allocated. virtual size of L1 image is shorter that L2 and L0, thus ' ' is beyond EOF. Thank you, will rewrite today. Den >> Signed-off-by: Denis V. Lunev >> CC: Stefan Hajnoczi >> CC: Fam Zheng >> CC: Kevin Wolf >> CC: Max Reitz >> CC: Jeff Cody >> --- >> block/io.c | 25 ++++++++++++++++++++----- >> 1 file changed, 20 insertions(+), 5 deletions(-) >> >> diff --git a/block/io.c b/block/io.c >> index 420944d..067d465 100644 >> --- a/block/io.c >> +++ b/block/io.c >> @@ -1741,18 +1741,33 @@ static int64_t coroutine_fn bdrv_co_get_block_status_above(BlockDriverState *bs, >> BlockDriverState **file) >> { >> BlockDriverState *p; >> - int64_t ret = 0; >> + int64_t ret = 0, res = nb_sectors; >> >> assert(bs != base); >> for (p = bs; p != base; p = backing_bs(p)) { >> - ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, pnum, file); >> - if (ret < 0 || ret & BDRV_BLOCK_ALLOCATED) { >> - break; >> + int sc; >> + ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, &sc, file); >> + if (ret < 0) { >> + return ret; >> + } else if (ret & BDRV_BLOCK_ALLOCATED) { >> + *pnum = sc; >> + return ret; >> + } >> + >> + if (res > sc && (p == bs || sector_num + sc < p->total_sectors)) { >> + res = sc; >> } >> + >> /* [sector_num, pnum] unallocated on this layer, which could be only >> * the first part of [sector_num, nb_sectors]. */ >> - nb_sectors = MIN(nb_sectors, *pnum); >> + nb_sectors = MIN(nb_sectors, sc); >> + >> + if (nb_sectors == 0) { >> + break; >> + } >> } >> + >> + *pnum = res; >> return ret; >> } >> >> -- >> 2.7.4 >> >>