Re: [Qemu-devel] [PULL 33/42] block: return BDRV_BLOCK_ZERO past end of backing file

[Peter Lieven]

On 06.09.2013 17:39, Stefan Hajnoczi wrote:
> From: Paolo Bonzini <address@hidden>
>
> 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 <address@hidden>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
>   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;
>   }