Re: [Qemu-devel] [PATCH v2 10/17] block: define get_block_status return value

[Peter Lieven <pl@kamp.de>]

On 16.07.2013 18:29, Paolo Bonzini wrote:
> Define the return value of get_block_status.  Bits 0, 1, 2 and 9-62
> are valid; bit 63 (the sign bit) is reserved for errors.  Bits 3-7
> are left for future extensions.
>
> The return code is compatible with the old is_allocated API: returning
> just 0 or 1 (aka BDRV_BLOCK_DATA) will not cause any behavioral change
> in clients of is_allocated.  We will return more precise information
> in the next patches.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>          v1->v2: improved comment
>
>   block.c               |  7 +++++--
>   include/block/block.h | 26 ++++++++++++++++++++++++++
>   2 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/block.c b/block.c
> index 6e7a8a3..7ff0716 100644
> --- a/block.c
> +++ b/block.c
> @@ -2990,7 +2990,7 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
>   
>       if (!bs->drv->bdrv_co_get_block_status) {
>           *pnum = nb_sectors;
> -        return 1;
> +        return BDRV_BLOCK_DATA;
>       }
>   
>       return bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum);
> @@ -3040,7 +3040,10 @@ int64_t bdrv_get_block_status(BlockDriverState *bs, int64_t sector_num,
>   int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num,
>                                      int nb_sectors, int *pnum)
>   {
> -    return bdrv_get_block_status(bs, sector_num, nb_sectors, pnum);
> +    int64_t ret = bdrv_get_block_status(bs, sector_num, nb_sectors, pnum);
just stumbled about a question i have:

isn't here a

if (ret < 0) {
  *pnum = 0;
  return 0;
}

missing?

I was told that this is the expected return behaviour in the
error case of the old API.

Peter

> +    return
> +        (ret & BDRV_BLOCK_DATA) ||
> +        ((ret & BDRV_BLOCK_ZERO) && !bdrv_has_zero_init(bs));
>   }
>   
>   /*
> diff --git a/include/block/block.h b/include/block/block.h
> index fbc0822..83c7112 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -81,6 +81,32 @@ typedef struct BlockDevOps {
>   #define BDRV_SECTOR_SIZE   (1ULL << BDRV_SECTOR_BITS)
>   #define BDRV_SECTOR_MASK   ~(BDRV_SECTOR_SIZE - 1)
>   
> +/* BDRV_BLOCK_DATA: data is read from bs->file or another file
> + * BDRV_BLOCK_ZERO: sectors read as zero
> + * BDRV_BLOCK_OFFSET_VALID: sector stored in bs->file as raw data
> + *
> + * If BDRV_BLOCK_OFFSET_VALID is set, bits 9-62 represent the offset in
> + * bs->file where sector data can be read from as raw data.
> + *
> + * DATA == 0 && ZERO == 0 means that data is read from backing_hd if present.
> + *
> + * DATA ZERO OFFSET_VALID
> + *  t    t        t       sectors read as zero, bs->file is zero at offset
> + *  t    f        t       sectors read as valid from bs->file at offset
> + *  f    t        t       sectors preallocated, read as zero, bs->file not
> + *                        necessarily zero at offset
> + *  f    f        t       sectors preallocated but read from backing_hd,
> + *                        bs->file contains garbage at offset
> + *  t    t        f       sectors preallocated, read as zero, unknown offset
> + *  t    f        f       sectors read from unknown file or offset
> + *  f    t        f       not allocated or unknown offset, read as zero
> + *  f    f        f       not allocated or unknown offset, read from backing_hd
> + */
> +#define BDRV_BLOCK_DATA         1
> +#define BDRV_BLOCK_ZERO         2
> +#define BDRV_BLOCK_OFFSET_VALID 4
> +#define BDRV_BLOCK_OFFSET_MASK  BDRV_SECTOR_MASK
> +
>   typedef enum {
>       BDRV_ACTION_REPORT, BDRV_ACTION_IGNORE, BDRV_ACTION_STOP
>   } BlockErrorAction;


-- 

Mit freundlichen Grüßen

Peter Lieven

...........................................................

   KAMP Netzwerkdienste GmbH
   Vestische Str. 89-91 | 46117 Oberhausen
   Tel: +49 (0) 208.89 402-50 | Fax: +49 (0) 208.89 402-40
   pl@kamp.de | http://www.kamp.de

   Geschäftsführer: Heiner Lante | Michael Lante
   Amtsgericht Duisburg | HRB Nr. 12154
   USt-Id-Nr.: DE 120607556

...........................................................