Re: [Qemu-devel] [PATCH v4 11/23] block: Rename BlockDriverAIOCB* to BlockAIOCB*

[Max Reitz <mreitz@redhat.com>]

On 30.09.2014 21:25, Markus Armbruster wrote:
> I'll use BlockDriverAIOCB with block backends shortly, and the name is
> going to fit badly there.  It's a block layer thing anyway, not just a
> block driver thing.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   block-migration.c           |   2 +-
>   block.c                     | 124 ++++++++++++++++++++++----------------------
>   block/archipelago.c         |  10 ++--
>   block/blkdebug.c            |  10 ++--
>   block/blkverify.c           |   8 +--
>   block/curl.c                |   4 +-
>   block/iscsi.c               |   6 +--
>   block/linux-aio.c           |   6 +--
>   block/null.c                |  10 ++--
>   block/qed.c                 |   8 +--
>   block/qed.h                 |   2 +-
>   block/quorum.c              |  36 ++++++-------
>   block/raw-aio.h             |   4 +-
>   block/raw-posix.c           |  16 +++---
>   block/raw-win32.c           |   8 +--
>   block/raw_bsd.c             |   8 +--
>   block/rbd.c                 |  12 ++---
>   block/sheepdog.c            |   4 +-
>   block/win32-aio.c           |   4 +-
>   dma-helpers.c               |  12 ++---
>   hw/block/nvme.h             |   2 +-
>   hw/ide/ahci.h               |   2 +-
>   hw/ide/core.c               |   8 +--
>   hw/ide/internal.h           |   6 +--
>   hw/ppc/mac.h                |   2 +-
>   hw/scsi/scsi-generic.c      |   2 +-
>   include/block/aio.h         |   8 +--
>   include/block/block.h       |  20 +++----
>   include/block/block_int.h   |  10 ++--
>   include/block/thread-pool.h |   2 +-
>   include/hw/scsi/scsi.h      |   2 +-
>   include/sysemu/dma.h        |  26 +++++-----
>   tests/test-thread-pool.c    |   2 +-
>   thread-pool.c               |   8 +--
>   34 files changed, 197 insertions(+), 197 deletions(-)
>

[snip]

> diff --git a/block/archipelago.c b/block/archipelago.c
> index 73d91a4..edbfbb5 100644
> --- a/block/archipelago.c
> +++ b/block/archipelago.c
> @@ -86,7 +86,7 @@ typedef enum {
>   } ARCHIPCmd;
>   
>   typedef struct ArchipelagoAIOCB {
> -    BlockDriverAIOCB common;
> +    BlockAIOCB common;
>       QEMUBH *bh;
>       struct BDRVArchipelagoState *s;
>       QEMUIOVector *qiov;
> @@ -856,7 +856,7 @@ err_exit:
>       return ret;
>   }
>   
> -static BlockDriverAIOCB *qemu_archipelago_aio_rw(BlockDriverState *bs,
> +static BlockAIOCB *qemu_archipelago_aio_rw(BlockDriverState *bs,
>                                                    int64_t sector_num,
>                                                    QEMUIOVector *qiov,
>                                                    int nb_sectors,

This breaks the alignment.

[snip]

> diff --git a/block/blkverify.c b/block/blkverify.c
> index 7d64a23..a29ed05 100644
> --- a/block/blkverify.c
> +++ b/block/blkverify.c

[snip]

> -static BlockDriverAIOCB *blkverify_aio_flush(BlockDriverState *bs,
> +static BlockAIOCB *blkverify_aio_flush(BlockDriverState *bs,
>                                                BlockDriverCompletionFunc *cb,
>                                                void *opaque)
>   {

Breaks the alignment.

[snip]

> diff --git a/block/null.c b/block/null.c
> index 8284419..b353a73 100644
> --- a/block/null.c
> +++ b/block/null.c

[snip]

> @@ -94,7 +94,7 @@ static void null_bh_cb(void *opaque)
>       qemu_aio_unref(acb);
>   }
>   
> -static inline BlockDriverAIOCB *null_aio_common(BlockDriverState *bs,
> +static inline BlockAIOCB *null_aio_common(BlockDriverState *bs,
>                                                   BlockDriverCompletionFunc *cb,
>                                                   void *opaque)
>   {

Alignment again (applies to all hunks in block/null.c touching function 
headers).

[snip]

> diff --git a/block/qed.c b/block/qed.c
> index 7f03c26..0382228 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -1365,7 +1365,7 @@ static void qed_aio_next_io(void *opaque, int ret)
>                         io_fn, acb);
>   }
>   
> -static BlockDriverAIOCB *qed_aio_setup(BlockDriverState *bs,
> +static BlockAIOCB *qed_aio_setup(BlockDriverState *bs,
>                                          int64_t sector_num,
>                                          QEMUIOVector *qiov, int nb_sectors,
>                                          BlockDriverCompletionFunc *cb,

Alignment, for all function header hunks in block/qed.c.

[snip]

> diff --git a/block/rbd.c b/block/rbd.c
> index 96947e3..f44a093 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c

[snip]

> @@ -589,7 +589,7 @@ static int rbd_aio_flush_wrapper(rbd_image_t image,
>   #endif
>   }
>   
> -static BlockDriverAIOCB *rbd_start_aio(BlockDriverState *bs,
> +static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
>                                          int64_t sector_num,
>                                          QEMUIOVector *qiov,
>                                          int nb_sectors,

Alignment, for all function header hunks in block/rbd.c.

[snip]

> diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
> index 20587b4..8baee16 100644
> --- a/hw/scsi/scsi-generic.c
> +++ b/hw/scsi/scsi-generic.c
> @@ -157,7 +157,7 @@ static void scsi_cancel_io(SCSIRequest *req)
>   
>   static int execute_command(BlockDriverState *bdrv,
>                              SCSIGenericReq *r, int direction,
> -			   BlockDriverCompletionFunc *complete)
> +                           BlockDriverCompletionFunc *complete)
>   {
>       r->io_header.interface_id = 'S';
>       r->io_header.dxfer_direction = direction;

Albeit no functional change, I think this belongs in the next patch.

[snip]

> diff --git a/include/block/block.h b/include/block/block.h
> index 3dc7c56..4349c95 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h

[snip]

> @@ -326,19 +326,19 @@ BlockDriverState *check_to_replace_node(const char *node_name, Error **errp);
>   /* async block I/O */
>   typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector,
>                                        int sector_num);
> -BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num,
> +BlockAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num,
>                                    QEMUIOVector *iov, int nb_sectors,
>                                    BlockDriverCompletionFunc *cb, void *opaque);
> -BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
> +BlockAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
>                                     QEMUIOVector *iov, int nb_sectors,
>                                     BlockDriverCompletionFunc *cb, void *opaque);
> -BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
> +BlockAIOCB *bdrv_aio_flush(BlockDriverState *bs,
>                                    BlockDriverCompletionFunc *cb, void *opaque);
> -BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs,
> +BlockAIOCB *bdrv_aio_discard(BlockDriverState *bs,
>                                      int64_t sector_num, int nb_sectors,
>                                      BlockDriverCompletionFunc *cb, void *opaque);

Alignment again.

I'm not sure why these alignment breaks appeared in v4; I can't imagine 
they're on purpose, because some hunks do fix up the alignment.

Furthermore, docs/blkdebug.txt mentions BlockDriverAIOCB. You may want 
to replace that occurrence as well. With the alignment issues fixed, the 
scsi-generic.c hunk moved to patch 12 and the two trivial replacements 
in docs/blkdebug.txt:

Reviewed-by: Max Reitz <mreitz@redhat.com>