From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:52987)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <lieven-lists@dlhnet.de>) id 1VKNt9-0006Ru-7z
	for qemu-devel@nongnu.org; Fri, 13 Sep 2013 03:34:06 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <lieven-lists@dlhnet.de>) id 1VKNt1-00075f-Sb
	for qemu-devel@nongnu.org; Fri, 13 Sep 2013 03:33:59 -0400
Received: from ssl.dlhnet.de ([91.198.192.8]:46822 helo=ssl.dlh.net)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <lieven-lists@dlhnet.de>) id 1VKNt1-00075Y-ME
	for qemu-devel@nongnu.org; Fri, 13 Sep 2013 03:33:51 -0400
Message-ID: <5232BFDF.6000402@dlhnet.de>
Date: Fri, 13 Sep 2013 09:33:51 +0200
From: Peter Lieven <lieven-lists@dlhnet.de>
MIME-Version: 1.0
References: <1378481953-23099-1-git-send-email-stefanha@redhat.com>
	<1378481953-23099-34-git-send-email-stefanha@redhat.com>
In-Reply-To: <1378481953-23099-34-git-send-email-stefanha@redhat.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PULL 33/42] block: return BDRV_BLOCK_ZERO past
 end of backing file
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org, Anthony Liguori <anthony@codemonkey.ws>

On 06.09.2013 17:39, Stefan Hajnoczi wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
>
> If the sectors are unallocated and we are past the end of the
> backing file, they will read as zero.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   block.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/block.c b/block.c
> index aa9ec83..82bbd6c 100644
> --- a/block.c
> +++ b/block.c
> @@ -3102,8 +3102,16 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
>           return ret;
>       }
>   
> -    if (!(ret & BDRV_BLOCK_DATA) && bdrv_has_zero_init(bs)) {
> -        ret |= BDRV_BLOCK_ZERO;
> +    if (!(ret & BDRV_BLOCK_DATA)) {
> +        if (bdrv_has_zero_init(bs)) {
this should be bdi->discard_zeroes. bdrv_has_zero_init() does only give a valid result
right after bdrv_create(). i currently working on extending bdi. I can send a patch for this
if you agree.
> +            ret |= BDRV_BLOCK_ZERO;
> +        } else {
> +            BlockDriverState *bs2 = bs->backing_hd;

this segfaults if there is no backing_hd. found while testing get_block_status with iscsi.

paolo, is this the correct fix?

@@ -3110,7 +3157,7 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
      if (!(ret & BDRV_BLOCK_DATA)) {
          if (bdrv_has_zero_init(bs)) {
              ret |= BDRV_BLOCK_ZERO;
-        } else {
+        } else if (bs->backing_hd)  {
              BlockDriverState *bs2 = bs->backing_hd;
              int64_t length2 = bdrv_getlength(bs2);
              if (length2 >= 0 && sector_num >= (length2 >> BDRV_SECTOR_BITS)) {


> +            int64_t length2 = bdrv_getlength(bs2);
> +            if (length2 >= 0 && sector_num >= (length2 >> BDRV_SECTOR_BITS)) {
> +                ret |= BDRV_BLOCK_ZERO;
> +            }
> +        }
>       }
>       return ret;
>   }