[10/17] block: define get_block_status return value

Define the return value of get_block_status.  Bits 0, 1, 2 and 8-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>
---
 block.c               |  7 +++++--
 include/block/block.h | 23 +++++++++++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

Am 03.07.2013 um 16:34 schrieb Paolo Bonzini <pbonzini@redhat.com>:

> Define the return value of get_block_status.  Bits 0, 1, 2 and 8-62
> are valid; bit 63 (the sign bit) is reserved for errors.  Bits 3-7
> are left for future extensions.

Is Bit 8 not also reserved for future use? BDRV_SECTOR_BITS is 9.

Can you explain which information is exactly returned in Bits 9-62?

Peter

> 
> 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>
> ---
> block.c               |  7 +++++--
> include/block/block.h | 23 +++++++++++++++++++++++
> 2 files changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/block.c b/block.c
> index aa1a5f7..8931cac 100644
> --- a/block.c
> +++ b/block.c
> @@ -2976,7 +2976,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);
> @@ -3026,7 +3026,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);
> +    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 2b50b51..9e44bdd 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -81,6 +81,29 @@ 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: contents of sector available in bs->file at offset
> + *
> + * 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;
> -- 
> 1.8.2.1
> 
>

Il 03/07/2013 23:04, Peter Lieven ha scritto:
>> > Define the return value of get_block_status.  Bits 0, 1, 2 and 8-62
>> > are valid; bit 63 (the sign bit) is reserved for errors.  Bits 3-7
>> > are left for future extensions.
> Is Bit 8 not also reserved for future use? BDRV_SECTOR_BITS is 9.

Right.

> Can you explain which information is exactly returned in Bits 9-62?

Bits 9-62 are the offset at which the data is stored in bs->file, they
are valid if bit 2 (BDRV_BLOCK_OFFSET_VALID) is 1.

Paolo

Am 04.07.2013 um 10:13 schrieb Paolo Bonzini <pbonzini@redhat.com>:

> Il 03/07/2013 23:04, Peter Lieven ha scritto:
>>>> Define the return value of get_block_status.  Bits 0, 1, 2 and 8-62
>>>> are valid; bit 63 (the sign bit) is reserved for errors.  Bits 3-7
>>>> are left for future extensions.
>> Is Bit 8 not also reserved for future use? BDRV_SECTOR_BITS is 9.
> 
> Right.
> 
>> Can you explain which information is exactly returned in Bits 9-62?
> 
> Bits 9-62 are the offset at which the data is stored in bs->file, they
> are valid if bit 2 (BDRV_BLOCK_OFFSET_VALID) is 1.

Ok, so this is if bs->file is not linear?

If we return the offset into bs->file this would only make sense if the data
at that position is raw and not encoded otherwise and if *pnum is limited
to the size of the extend at that position, right?

I currently do not understand for what operation this info is needed.

Maybe you could add some info to the commit or the function including
your comment from above.

Thanks,
Peter

On Thu, 07/04 23:10, Peter Lieven wrote:
> 
> Am 04.07.2013 um 10:13 schrieb Paolo Bonzini <pbonzini@redhat.com>:
> 
> > Il 03/07/2013 23:04, Peter Lieven ha scritto:
> >>>> Define the return value of get_block_status.  Bits 0, 1, 2 and 8-62
> >>>> are valid; bit 63 (the sign bit) is reserved for errors.  Bits 3-7
> >>>> are left for future extensions.
> >> Is Bit 8 not also reserved for future use? BDRV_SECTOR_BITS is 9.
> > 
> > Right.
> > 
> >> Can you explain which information is exactly returned in Bits 9-62?
> > 
> > Bits 9-62 are the offset at which the data is stored in bs->file, they
> > are valid if bit 2 (BDRV_BLOCK_OFFSET_VALID) is 1.
> 
> Ok, so this is if bs->file is not linear?
> 
> If we return the offset into bs->file this would only make sense if the data
> at that position is raw and not encoded otherwise and if *pnum is limited
> to the size of the extend at that position, right?
Exactly.
> 
> I currently do not understand for what operation this info is needed.
Quoted from the cover letter:
  > One example usage is
  > (for non-compressed, non-encrypted images) to transform the metadata
  > into a Linux device-mapper setup, and make a qcow2 image available (for
  > read only) as a block device.  Another possible usage is to determine
  > the used areas of a file, and convert it in place to another format.

Thanks.

Fam