Re: [Qemu-devel] [PATCH v2 09/17] block: introduce bdrv_get_block_status API

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4744 bytes --]

On 07/16/2013 10:29 AM, Paolo Bonzini wrote:
> For now, bdrv_get_block_status is just another name for bdrv_is_allocated.
> The next patches will add more flags.
> 
> This also touches all block drivers with a mostly mechanical rename.  The
> sole exception is cow; because it calls cow_co_is_allocated from the read
> code, we keep that function and make cow_co_get_block_status a wrapper.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>         v1->v2: rebase after vmdk changes

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> @@ -370,7 +376,7 @@ static BlockDriver bdrv_cow = {
>  
>      .bdrv_read              = cow_co_read,
>      .bdrv_write             = cow_co_write,
> -    .bdrv_co_is_allocated   = cow_co_is_allocated,
> +    .bdrv_co_get_block_status   = cow_co_get_block_status,

Is it worth realigning indentation now that you have a longer name?

> +++ b/block/qcow.c
> @@ -395,7 +395,7 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
>      return cluster_offset;
>  }
>  
> -static int coroutine_fn qcow_co_is_allocated(BlockDriverState *bs,
> +static int64_t coroutine_fn qcow_co_get_block_status(BlockDriverState *bs,
>          int64_t sector_num, int nb_sectors, int *pnum)

Is it worth fixing alignment while you touch this?

>  {
>      BDRVQcowState *s = bs->opaque;
> @@ -896,7 +896,7 @@ static BlockDriver bdrv_qcow = {
>  
>      .bdrv_co_readv          = qcow_co_readv,
>      .bdrv_co_writev         = qcow_co_writev,
> -    .bdrv_co_is_allocated   = qcow_co_is_allocated,
> +    .bdrv_co_get_block_status   = qcow_co_get_block_status,

Another spot for realignment?

> +++ b/block/qcow2.c
> @@ -640,7 +640,7 @@ static int qcow2_reopen_prepare(BDRVReopenState *state,
>      return 0;
>  }
>  
> -static int coroutine_fn qcow2_co_is_allocated(BlockDriverState *bs,
> +static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs,
>          int64_t sector_num, int nb_sectors, int *pnum)

alignment?

>  {
>      BDRVQcowState *s = bs->opaque;
> @@ -1784,7 +1784,7 @@ static BlockDriver bdrv_qcow2 = {
>      .bdrv_reopen_prepare  = qcow2_reopen_prepare,
>      .bdrv_create        = qcow2_create,
>      .bdrv_has_zero_init = bdrv_has_zero_init_1,
> -    .bdrv_co_is_allocated = qcow2_co_is_allocated,
> +    .bdrv_co_get_block_status = qcow2_co_get_block_status,
>      .bdrv_set_key       = qcow2_set_key,
>      .bdrv_make_empty    = qcow2_make_empty,
>  

wow, this is already an alignment mess before your change

> +++ b/block/qed.c
> @@ -667,7 +667,7 @@ static void qed_is_allocated_cb(void *opaque, int ret, uint64_t offset, size_t l
>      }
>  }
>  
> -static int coroutine_fn bdrv_qed_co_is_allocated(BlockDriverState *bs,
> +static int64_t coroutine_fn bdrv_qed_co_get_block_status(BlockDriverState *bs,
>                                                   int64_t sector_num,
>                                                   int nb_sectors, int *pnum)

alignment

> +++ b/block/raw-posix.c
> @@ -1084,7 +1084,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options)
>   * '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 int coroutine_fn raw_co_is_allocated(BlockDriverState *bs,
> +static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
>                                              int64_t sector_num,
>                                              int nb_sectors, int *pnum)

and again

>  {
> @@ -1200,7 +1200,7 @@ static BlockDriver bdrv_file = {
>      .bdrv_close = raw_close,
>      .bdrv_create = raw_create,
>      .bdrv_has_zero_init = bdrv_has_zero_init_1,
> -    .bdrv_co_is_allocated = raw_co_is_allocated,
> +    .bdrv_co_get_block_status = raw_co_get_block_status,
>  
>      .bdrv_aio_readv = raw_aio_readv,
>      .bdrv_aio_writev = raw_aio_writev,

here, nothing was aligned, so you actually met status quo :)

But that raises a question of consistency between drivers...

> +++ b/block/raw.c
> @@ -35,11 +35,11 @@ static void raw_close(BlockDriverState *bs)
>  {
>  }
>  
> -static int coroutine_fn raw_co_is_allocated(BlockDriverState *bs,
> +static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
>                                              int64_t sector_num,
>                                              int nb_sectors, int *pnum)

alignment.  You get the picture; I'll quit pointing it out, since it
doesn't affect semantics.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]