From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46210) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WhxG8-0000mW-3j for qemu-devel@nongnu.org; Wed, 07 May 2014 04:31:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WhxG1-0005sw-Be for qemu-devel@nongnu.org; Wed, 07 May 2014 04:31:24 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59188) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WhxG1-0005sY-2N for qemu-devel@nongnu.org; Wed, 07 May 2014 04:31:17 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s478VGTB020029 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 7 May 2014 04:31:16 -0400 Date: Wed, 7 May 2014 10:31:12 +0200 From: Kevin Wolf Message-ID: <20140507083112.GC4045@noname.str.redhat.com> References: <1399383018-17797-1-git-send-email-kwolf@redhat.com> <53693DD0.40107@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <53693DD0.40107@redhat.com> Subject: Re: [Qemu-devel] [PATCH] block: Fix bdrv_is_allocated() for short backing files List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com Am 06.05.2014 um 21:53 hat Max Reitz geschrieben: > On 06.05.2014 15:30, Kevin Wolf wrote: > >bdrv_is_allocated() shouldn't return true for sectors that are > >unallocated, but after the end of a short backing file, even though > >such sectors are (correctly) marked as containing zeros. > > > >Signed-off-by: Kevin Wolf > >--- > > block.c | 8 +++++--- > > include/block/block.h | 11 +++++++---- > > 2 files changed, 12 insertions(+), 7 deletions(-) > > > >diff --git a/block.c b/block.c > >index c90c71a..d3a9906 100644 > >--- a/block.c > >+++ b/block.c > >@@ -3883,6 +3883,10 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, > > *pnum, pnum); > > } > >+ if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) { > >+ ret |= BDRV_BLOCK_ALLOCATED; > >+ } > >+ > > Shouldn't BDRV_BLOCK_ALLOCATED be set in the > !bs->drv->bdrv_co_get_block_status case as well? It should. Thanks, I'll send a v2. > > if (!(ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO)) { > > if (bdrv_unallocated_blocks_are_zero(bs)) { > > ret |= BDRV_BLOCK_ZERO; > >@@ -3959,9 +3963,7 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, > > if (ret < 0) { > > return ret; > > } > >- return > >- (ret & BDRV_BLOCK_DATA) || > >- ((ret & BDRV_BLOCK_ZERO) && !bdrv_has_zero_init(bs)); > >+ return (ret & BDRV_BLOCK_ALLOCATED); > > } > > /* > >diff --git a/include/block/block.h b/include/block/block.h > >index 2fda81c..ad4c7e8 100644 > >--- a/include/block/block.h > >+++ b/include/block/block.h > >@@ -116,6 +116,8 @@ typedef enum { > > /* BDRV_BLOCK_DATA: data is read from bs->file or another file > > * BDRV_BLOCK_ZERO: sectors read as zero > > * BDRV_BLOCK_OFFSET_VALID: sector stored in bs->file as raw data > >+ * BDRV_BLOCK_ALLOCATED: the content of the block is determined by this > >+ * layer (as opposed to the backing file) > > I guess this is above BDRV_BLOCK_RAW (albeit having a greater value) > because it is not only used internally? (to pick up on the topic of > OCD :-P) Because it felt right. This may or may not be equivalent. Kevin