Re: [Qemu-devel] [PATCH 04/17] block: Rename bdrv_co_do_preadv/writev to bdrv_co_preadv/writev

[Eric Blake <eblake@redhat.com>]

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

On 04/27/2016 03:52 AM, Kevin Wolf wrote:
> It used to be an internal helper function just for implementing
> bdrv_co_do_readv/writev(), but now that it's a public interface, it
> deserves a name without "do" in it.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/block-backend.c     |  4 ++--
>  block/io.c                | 20 ++++++++++----------
>  block/raw_bsd.c           |  4 ++--
>  hw/ide/macio.c            |  4 ++--
>  include/block/block_int.h |  4 ++--
>  5 files changed, 18 insertions(+), 18 deletions(-)
> 

> @@ -1127,7 +1127,7 @@ static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs,
>          return -EINVAL;
>      }
>  
> -    return bdrv_co_do_preadv(bs, sector_num << BDRV_SECTOR_BITS,
> +    return bdrv_co_preadv(bs, sector_num << BDRV_SECTOR_BITS,
>                               nb_sectors << BDRV_SECTOR_BITS, qiov, flags);
>  }

Missed alignment.


> @@ -1523,7 +1523,7 @@ static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs,
>          return -EINVAL;
>      }
>  
> -    return bdrv_co_do_pwritev(bs, sector_num << BDRV_SECTOR_BITS,
> +    return bdrv_co_pwritev(bs, sector_num << BDRV_SECTOR_BITS,
>                                nb_sectors << BDRV_SECTOR_BITS, qiov, flags);
>  }

and again

> +++ b/hw/ide/macio.c
> @@ -55,8 +55,8 @@ static const int debug_macio = 0;
>  /*
>   * Unaligned DMA read/write access functions required for OS X/Darwin which
>   * don't perform DMA transactions on sector boundaries. These functions are
> - * modelled on bdrv_co_do_preadv()/bdrv_co_do_pwritev() and so should be
> - * easy to remove if the unaligned block APIs are ever exposed.
> + * modelled on bdrv_co_preadv()/bdrv_co_pwritev() and so should be easy to
> + * remove if the unaligned block APIs are ever exposed.
>   */

Is this comment now stale as a result of your series?

> +++ b/include/block/block_int.h
> @@ -517,10 +517,10 @@ extern BlockDriver bdrv_qcow2;
>   */
>  void bdrv_setup_io_funcs(BlockDriver *bdrv);
>  
> -int coroutine_fn bdrv_co_do_preadv(BlockDriverState *bs,
> +int coroutine_fn bdrv_co_preadv(BlockDriverState *bs,
>      int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
>      BdrvRequestFlags flags);
> -int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs,
> +int coroutine_fn bdrv_co_pwritev(BlockDriverState *bs,
>      int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
>      BdrvRequestFlags flags);

Should alignment be attempted here, while touching it?

My comments are minor, so whether or not you make those changes:
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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: 604 bytes --]