From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58901) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1blnHZ-0000s9-In for qemu-devel@nongnu.org; Sun, 18 Sep 2016 21:22:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1blnHX-0007yM-Cl for qemu-devel@nongnu.org; Sun, 18 Sep 2016 21:22:04 -0400 Date: Mon, 19 Sep 2016 09:21:51 +0800 From: Fam Zheng Message-ID: <20160919012151.GA4883@lemon> References: <1473957290-13382-1-git-send-email-den@openvz.org> <1473957290-13382-2-git-send-email-den@openvz.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1473957290-13382-2-git-send-email-den@openvz.org> 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: "Denis V. Lunev" Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, Kevin Wolf , Jeff Cody , Max Reitz , Stefan Hajnoczi 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: -------------- > 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 > > 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 > >