[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V15 03/13] quorum: Add quorum_aio_writev and its
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH V15 03/13] quorum: Add quorum_aio_writev and its dependencies. |
Date: |
Tue, 4 Feb 2014 14:57:22 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 03.02.2014 um 22:51 hat Benoît Canet geschrieben:
> From: Benoît Canet <address@hidden>
>
> Signed-off-by: Benoit Canet <address@hidden>
> ---
> block/quorum.c | 104
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 104 insertions(+)
Starting with writes before the driver can even open an image is a weird
order to do things in. It also doesn't make the review any easier when
you don't know how things are initialised.
> diff --git a/block/quorum.c b/block/quorum.c
> index 157efdf..81bffdd 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -64,11 +64,115 @@ struct QuorumAIOCB {
> int vote_ret;
> };
>
> +static void quorum_aio_cancel(BlockDriverAIOCB *blockacb)
> +{
> + QuorumAIOCB *acb = container_of(blockacb, QuorumAIOCB, common);
> + BDRVQuorumState *s = acb->bqs;
> + int i;
> +
> + /* cancel all callback */
"callbacks"
> + for (i = 0; i < s->total; i++) {
> + bdrv_aio_cancel(acb->aios[i].aiocb);
> + }
> +}
Don't you want to free acb and similar cleanup?
> +
> +static AIOCBInfo quorum_aiocb_info = {
> + .aiocb_size = sizeof(QuorumAIOCB),
> + .cancel = quorum_aio_cancel,
> +};
> +
> +static void quorum_aio_finalize(QuorumAIOCB *acb)
> +{
> + BDRVQuorumState *s = acb->bqs;
block/quorum.c: In function 'quorum_aio_finalize':
block/quorum.c:86:22: error: unused variable 's' [-Werror=unused-variable]
> + int ret = 0;
> +
> + acb->common.cb(acb->common.opaque, ret);
> + if (acb->finished) {
> + *acb->finished = true;
> + }
> + g_free(acb->aios);
> + qemu_aio_release(acb);
> +}
> +
> +static QuorumAIOCB *quorum_aio_get(BDRVQuorumState *s,
> + BlockDriverState *bs,
> + QEMUIOVector *qiov,
> + uint64_t sector_num,
> + int nb_sectors,
> + BlockDriverCompletionFunc *cb,
> + void *opaque)
> +{
> + QuorumAIOCB *acb = qemu_aio_get(&quorum_aiocb_info, bs, cb, opaque);
> + int i;
> +
> + acb->bqs = s;
Noticed it only here, but it's really in patch 2 (and should be in
patch 1):
What is acb->bqs good for? Isn't it always the same as
acb->common.bs->opaque?
> + acb->sector_num = sector_num;
> + acb->nb_sectors = nb_sectors;
> + acb->qiov = qiov;
> + acb->aios = g_new0(QuorumSingleAIOCB, s->total);
> + acb->count = 0;
> + acb->success_count = 0;
> + acb->finished = NULL;
> + acb->is_read = false;
> + acb->vote_ret = 0;
> +
> + for (i = 0; i < s->total; i++) {
> + acb->aios[i].buf = NULL;
> + acb->aios[i].ret = 0;
> + acb->aios[i].parent = acb;
> + }
>
> +
> + return acb;
> +}
Kevin
- [Qemu-devel] [PATCH V15 00/13] Quorum block filter, Benoît Canet, 2014/02/03
- [Qemu-devel] [PATCH V15 02/13] quorum: Create BDRVQuorumState and BlkDriver and do init., Benoît Canet, 2014/02/03
- [Qemu-devel] [PATCH V15 04/13] blkverify: Extract qemu_iovec_clone() and qemu_iovec_compare() from blkverify., Benoît Canet, 2014/02/03
- [Qemu-devel] [PATCH V15 09/13] quorum: Add quorum_co_get_block_status., Benoît Canet, 2014/02/03
- [Qemu-devel] [PATCH V15 03/13] quorum: Add quorum_aio_writev and its dependencies., Benoît Canet, 2014/02/03
- Re: [Qemu-devel] [PATCH V15 03/13] quorum: Add quorum_aio_writev and its dependencies.,
Kevin Wolf <=
- [Qemu-devel] [PATCH V15 07/13] quorum: Add quorum_getlength()., Benoît Canet, 2014/02/03
- [Qemu-devel] [PATCH V15 05/13] quorum: Add quorum_aio_readv., Benoît Canet, 2014/02/03
- [Qemu-devel] [PATCH V15 08/13] quorum: Add quorum_invalidate_cache()., Benoît Canet, 2014/02/03
- [Qemu-devel] [PATCH V15 10/13] quorum: Add quorum_co_flush()., Benoît Canet, 2014/02/03
- [Qemu-devel] [PATCH V15 11/13] quorum: Implement recursive .bdrv_recurse_is_first_non_filter in quorum., Benoît Canet, 2014/02/03
- [Qemu-devel] [PATCH V15 01/13] quorum: Create quorum.c, add QuorumSingleAIOCB and QuorumAIOCB., Benoît Canet, 2014/02/03