Re: [Qemu-devel] [PATCH v2 17/17] block: look for zero blocks in bs->file

[Stefan Hajnoczi]

On Tue, Jul 16, 2013 at 06:29:28PM +0200, Paolo Bonzini wrote:
> diff --git a/block.c b/block.c
> index 557ce29..2d7d71f 100644
> --- a/block.c
> +++ b/block.c
> @@ -2977,7 +2977,7 @@ static int64_t coroutine_fn 
> bdrv_co_get_block_status(BlockDriverState *bs,
>                                                       int nb_sectors, int 
> *pnum)
>  {
>      int64_t n;
> -    int64_t ret;
> +    int64_t ret, ret2;
>  
>      if (sector_num >= bs->total_sectors) {
>          *pnum = 0;
> @@ -3003,6 +3003,14 @@ static int64_t coroutine_fn 
> bdrv_co_get_block_status(BlockDriverState *bs,
>          ret |= BDRV_BLOCK_ZERO;
>      }
>  
> +    if (bs->file &&
> +        (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
> +        (ret & BDRV_BLOCK_OFFSET_VALID)) {
> +        ret2 = bdrv_co_get_block_status(bs->file, ret >> BDRV_SECTOR_BITS,
> +                                        *pnum, pnum);
> +        ret |= (ret2 & BDRV_BLOCK_ZERO);
> +    }

This patch breaks qemu-iotests 030 (image streaming).

The problem is that bdrv_co_get_block_status() uses bs->total_sectors
directly instead of calling bdv_get_geometry()/bdrv_getlength().

With qcow2 the bs->file can grow on disk.  We don't update
bs->total_sectors.

Then this patch calls bdrv_co_get_block_status(bs->file) where we fail
with *pnum = 0, ret = 0 because bs->total_sectors suggests it is beyond
the end of the file.

The result is that 030 goes into an infinite loop.

As a quick test I switched the direct bs->total_sectors accesses to
bdrv_get_geometry() and it stopped hanging.  Perhaps the
bs->total_sectors caching needs to be improved though.

Stefan