From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48932) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WhJyH-0001Cp-BC for qemu-devel@nongnu.org; Mon, 05 May 2014 10:34:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WhJyB-0002y5-5H for qemu-devel@nongnu.org; Mon, 05 May 2014 10:34:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55806) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WhJyA-0002xy-U8 for qemu-devel@nongnu.org; Mon, 05 May 2014 10:34:15 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s45EYEPQ009810 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Mon, 5 May 2014 10:34:14 -0400 Date: Mon, 5 May 2014 16:34:12 +0200 From: Kevin Wolf Message-ID: <20140505143412.GE3317@noname.str.redhat.com> References: <1378314038-15525-1-git-send-email-pbonzini@redhat.com> <1378314038-15525-13-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1378314038-15525-13-git-send-email-pbonzini@redhat.com> Subject: Re: [Qemu-devel] [PATCH v5 12/21] block: define get_block_status return value List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, stefanha@redhat.com Am 04.09.2013 um 19:00 hat Paolo Bonzini geschrieben: > Define the return value of get_block_status. Bits 0, 1, 2 and 9-62 > are valid; bit 63 (the sign bit) is reserved for errors. Bits 3-8 > are left for future extensions. > > The return code is compatible with the old is_allocated API: if a driver > only returns 0 or 1 (aka BDRV_BLOCK_DATA) like is_allocated used to, > clients of is_allocated will not have any change in behavior. Still, > we will return more precise information in the next patches and the > new definition of bdrv_is_allocated is already prepared for this. > > Reviewed-by: Eric Blake > Signed-off-by: Paolo Bonzini > --- > block.c | 10 ++++++++-- > include/block/block.h | 26 ++++++++++++++++++++++++++ > 2 files changed, 34 insertions(+), 2 deletions(-) > > diff --git a/block.c b/block.c > index 47a5d63..0eb5e43 100644 > --- a/block.c > +++ b/block.c > @@ -3073,7 +3073,7 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, > > if (!bs->drv->bdrv_co_get_block_status) { > *pnum = nb_sectors; > - return 1; > + return BDRV_BLOCK_DATA; > } > > return bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum); > @@ -3123,7 +3123,13 @@ int64_t bdrv_get_block_status(BlockDriverState *bs, int64_t sector_num, > int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, > int nb_sectors, int *pnum) > { > - return bdrv_get_block_status(bs, sector_num, nb_sectors, pnum); > + int64_t ret = bdrv_get_block_status(bs, sector_num, nb_sectors, pnum); > + if (ret < 0) { > + return ret; > + } > + return > + (ret & BDRV_BLOCK_DATA) || > + ((ret & BDRV_BLOCK_ZERO) && !bdrv_has_zero_init(bs)); > } I'm afraid that the assumptions eventually became too clever: $ ./qemu-img create -f qcow2 /tmp/backing.qcow2 1G Formatting '/tmp/backing.qcow2', fmt=qcow2 size=1073741824 encryption=off cluster_size=65536 lazy_refcounts=off $ ./qemu-img create -f qcow2 -b /tmp/backing.qcow2 /tmp/overlay.qcow2 2G Formatting '/tmp/overlay.qcow2', fmt=qcow2 size=2147483648 backing_file='/tmp/backing.qcow2' encryption=off cluster_size=65536 lazy_refcounts=off $ ./qemu-io -c 'alloc 1G 64k' /tmp/overlay.qcow2 65536/65536 sectors allocated at offset 1 GiB BDRV_BLOCK_ZERO is set for everything past the end of the backing file (which kind of makes sense), and bdrv_has_zero_init() returns false for anything that has a backing file (which makes sense, too), so now we're reporting all sectors past the end of the backing file as allocated (which is bad). I think we're missing the information whether the BDRV_BLOCK_ZERO flag actually comes from bs itself or from the backing chain. Do we need another flag or does someone have a better idea? Kevin