From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60089) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VX9yu-0008Mf-Ef for qemu-devel@nongnu.org; Fri, 18 Oct 2013 09:20:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VX9yo-0006yI-3F for qemu-devel@nongnu.org; Fri, 18 Oct 2013 09:20:44 -0400 Received: from mx.ipv6.kamp.de ([2a02:248:0:51::16]:57202 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VX9yn-0006rW-OO for qemu-devel@nongnu.org; Fri, 18 Oct 2013 09:20:38 -0400 Message-ID: <526135A1.5030405@kamp.de> Date: Fri, 18 Oct 2013 15:20:33 +0200 From: Peter Lieven MIME-Version: 1.0 References: <1381233491-17019-1-git-send-email-pl@kamp.de> <1381233491-17019-15-git-send-email-pl@kamp.de> <20131018123812.GB19041@stefanha-thinkpad.redhat.com> In-Reply-To: <20131018123812.GB19041@stefanha-thinkpad.redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv4 14/17] block/get_block_status: fix BDRV_BLOCK_ZERO for unallocated blocks List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: kwolf@redhat.com, stefanha@redhat.com, qemu-devel@nongnu.org, anthony@codemonkey.ws, pbonzini@redhat.com, ronniesahlberg@gmail.com On 18.10.2013 14:38, Stefan Hajnoczi wrote: > On Tue, Oct 08, 2013 at 01:58:08PM +0200, Peter Lieven wrote: >> this patch does 2 things: >> a) only do additional call outs if BDRV_BLOCK_ZERO is not already set. >> b) use the newly introduced bdrv_has_discard_zeroes() to return the >> zero state of an unallocated block. the used callout to >> bdrv_has_zero_init() is only valid right after bdrv_create. >> >> Reviewed-by: Eric Blake >> Signed-off-by: Peter Lieven >> --- >> block.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/block.c b/block.c >> index fc931e3..1be4418 100644 >> --- a/block.c >> +++ b/block.c >> @@ -3247,8 +3247,8 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, >> return ret; >> } >> >> - if (!(ret & BDRV_BLOCK_DATA)) { >> - if (bdrv_has_zero_init(bs)) { >> + if (!(ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO)) { >> + if (bdrv_has_discard_zeroes(bs)) { > I'm a little unclear about the semantics of bdrv_has_discard_zeroes(). > Originally I thought it just meant any blocks discarded will read back > as zeroes. But here it implies that any unallocated block reads > back as zeroes too? > > In other words, this patch assumes unallocated blocks behave the same as > discarded blocks wrt to zeroes. > > Stefan bdrv_has_discard_zeroes() indicates if unallocated blocks read back as zeroes. as a discard may silently fail this is no guarantee that a discard will result in a properly zeroed area. bdrv_has_discard_write_zeroes() indicates that the driver is able to honour the BDRV_REQ_MAY_UNMAP flag on bdrv_write_zeroes(). see documentation in block_int.h /* * Returns true if discarded blocks read back as zeroes. */ bool (*bdrv_has_discard_zeroes)(BlockDriverState *bs); /* * Returns true if the driver can optimize writing zeroes by discarding * sectors. It is additionally required that the block device is * opened with BDRV_O_UNMAP and the that write zeroes request carries * the BDRV_REQ_MAY_UNMAP flag for this to work. */ bool (*bdrv_has_discard_write_zeroes)(BlockDriverState *bs); Peter