Re: [Qemu-devel] [PATCH 1/3] block: Ignore allocation size in underlying file

[Benoît Canet]

The Saturday 16 Aug 2014 à 20:54:16 (+0200), Max Reitz wrote :
> When falling through to the underlying file in
> bdrv_co_get_block_status(), do not let the number of sectors for which
> information could be obtained be overwritten.
> 
> Signed-off-by: Max Reitz <address@hidden>
> ---
>  block.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 3e252a2..c922664 100644
> --- a/block.c
> +++ b/block.c
> @@ -3991,9 +3991,11 @@ static int64_t coroutine_fn 
> bdrv_co_get_block_status(BlockDriverState *bs,
>      if (bs->file &&
>          (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
>          (ret & BDRV_BLOCK_OFFSET_VALID)) {
> +        int backing_pnum;
> +
>          ret2 = bdrv_co_get_block_status(bs->file, ret >> BDRV_SECTOR_BITS,
> -                                        *pnum, pnum);
> -        if (ret2 >= 0) {
> +                                        *pnum, &backing_pnum);
> +        if (ret2 >= 0 && backing_pnum >= *pnum) {

About backing_pnum >= *pnum.

The documentation of bdrv_co_get_block_status says:

 * 'nb_sectors' is the max value 'pnum' should be set to.  If nb_sectors goes   
 * beyond the end of the disk image it will be clamped.                         
 */                                                                             
static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,      
                                                     int64_t sector_num,        
                                                     int nb_sectors, int *pnum) 

So clearly after the bdrv_co_get_block_status *pnum >= backing_pnum.

This means that  backing_pnum > *pnum will never happen.

I think either this test is wrong or the doc is wrong.

Best regards

Benoît


>              /* Ignore errors.  This is just providing extra information, it
>               * is useful but not necessary.
>               */
> -- 
> 2.0.4
> 
>