Re: [Qemu-devel] [PATCH V17 05/12] quorum: Add quorum_aio_readv.

[Max Reitz <mreitz@redhat.com>]

On 12.02.2014 23:06, Benoît Canet wrote:
> From: Benoît Canet <benoit@irqsave.net>
>
> Add code to do num_children reads in parallel and cleanup the structures
> afterward.

"afterwards"

> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>   block/quorum.c | 39 ++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/block/quorum.c b/block/quorum.c
> index 197cdca..c7a5d79 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -91,10 +91,18 @@ static AIOCBInfo quorum_aiocb_info = {
>   
>   static void quorum_aio_finalize(QuorumAIOCB *acb)
>   {
> -    int ret = 0;
> +    BDRVQuorumState *s = acb->common.bs->opaque;
> +    int i, ret = 0;
>   
>       acb->common.cb(acb->common.opaque, ret);
>   
> +    if (acb->is_read) {
> +        for (i = 0; i < s->num_children; i++) {
> +            qemu_vfree(acb->qcrs[i].buf);
> +            qemu_iovec_destroy(&acb->qcrs[i].qiov);
> +        }
> +    }
> +
>       g_free(acb->qcrs);
>       qemu_aio_release(acb);
>   }
> @@ -149,6 +157,34 @@ static void quorum_aio_cb(void *opaque, int ret)
>       quorum_aio_finalize(acb);
>   }
>   
> +static BlockDriverAIOCB *quorum_aio_readv(BlockDriverState *bs,
> +                                         int64_t sector_num,
> +                                         QEMUIOVector *qiov,
> +                                         int nb_sectors,
> +                                         BlockDriverCompletionFunc *cb,
> +                                         void *opaque)
> +{
> +    BDRVQuorumState *s = bs->opaque;
> +    QuorumAIOCB *acb = quorum_aio_get(s, bs, qiov, sector_num,
> +                                      nb_sectors, cb, opaque);
> +    int i;
> +
> +    acb->is_read = true;
> +
> +    for (i = 0; i < s->num_children; i++) {
> +        acb->qcrs[i].buf = qemu_blockalign(s->bs[i], qiov->size);
> +        qemu_iovec_init(&acb->qcrs[i].qiov, qiov->niov);
> +        qemu_iovec_clone(&acb->qcrs[i].qiov, qiov, acb->qcrs[i].buf);
> +    }
> +
> +    for (i = 0; i < s->num_children; i++) {
> +        bdrv_aio_readv(s->bs[i], sector_num, &acb->qcrs[i].qiov, nb_sectors,
> +                       quorum_aio_cb, &acb->qcrs[i]);

I know Kevin told you to, but using the child's own QIOV here means 
quorum_aio_readv() won't work after this patch but only after patch 6. 
Just pointing it out, I don't like it, but Kevin told you to, so:

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

> +    }
> +
> +    return &acb->common;
> +}
> +
>   static BlockDriverAIOCB *quorum_aio_writev(BlockDriverState *bs,
>                                             int64_t sector_num,
>                                             QEMUIOVector *qiov,
> @@ -176,6 +212,7 @@ static BlockDriver bdrv_quorum = {
>   
>       .instance_size      = sizeof(BDRVQuorumState),
>   
> +    .bdrv_aio_readv     = quorum_aio_readv,
>       .bdrv_aio_writev    = quorum_aio_writev,
>   };
>