Re: [Qemu-devel] [PATCH v3 12/19] block: return get_block_status data and flags for formats

[Kevin Wolf]

Am 25.07.2013 um 16:23 hat Paolo Bonzini geschrieben:
> Reviewed-by: Eric Blake <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  block/cow.c      |  8 +++++++-
>  block/qcow.c     |  9 ++++++++-
>  block/qcow2.c    | 16 ++++++++++++++--
>  block/qed.c      | 35 ++++++++++++++++++++++++++++-------
>  block/sheepdog.c |  2 +-
>  block/vdi.c      | 13 ++++++++++++-
>  block/vmdk.c     | 19 ++++++++++++++++++-
>  block/vvfat.c    | 11 ++++++-----
>  8 files changed, 94 insertions(+), 19 deletions(-)
> 
> diff --git a/block/cow.c b/block/cow.c
> index e738b96..1e90413 100644
> --- a/block/cow.c
> +++ b/block/cow.c
> @@ -194,7 +194,13 @@ static int coroutine_fn 
> cow_co_is_allocated(BlockDriverState *bs,
>  static int64_t coroutine_fn cow_co_get_block_status(BlockDriverState *bs,
>          int64_t sector_num, int nb_sectors, int *num_same)
>  {
> -    return cow_co_is_allocated(bs, sector_num, nb_sectors, num_same);
> +    BDRVCowState *s = bs->opaque;
> +    int ret = cow_co_is_allocated(bs, sector_num, nb_sectors, num_same);
> +    int64_t offset = s->cow_sectors_offset + (sector_num << 
> BDRV_SECTOR_BITS);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +    return (ret ? BDRV_BLOCK_DATA : 0) | offset | BDRV_BLOCK_OFFSET_VALID;
>  }
>  
>  static int cow_update_bitmap(BlockDriverState *bs, int64_t sector_num,
> diff --git a/block/qcow.c b/block/qcow.c
> index acd1aeb..1a65822 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -410,7 +410,14 @@ static int64_t coroutine_fn 
> qcow_co_get_block_status(BlockDriverState *bs,
>      if (n > nb_sectors)
>          n = nb_sectors;
>      *pnum = n;
> -    return (cluster_offset != 0);
> +    if (!cluster_offset) {
> +        return 0;

If you take your comment in patch 11 serious, you should return
bs->backing_hd ? 0 : BDRV_BLOCK_ZERO instead. (I think it would be
useful behaviour, too, because knowing that a sector is zero enables
optimisations in several places.)

Of course, this is something that could be done in the block.c
implementation of bdrv_co_get_block_status() instead of each single
driver.

Kevin