Message ID | 1391464280-25627-6-git-send-email-benoit.canet@irqsave.net |
---|---|
State | New |
Headers | show |
Am 03.02.2014 um 22:51 hat Benoît Canet geschrieben: > From: Benoît Canet <benoit@irqsave.net> > > Signed-off-by: Benoit Canet <benoit@irqsave.net> > Reviewed-by: Max Reitz <mreitz@redhat.com> > --- > block/quorum.c | 38 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/block/quorum.c b/block/quorum.c > index 81bffdd..699b512 100644 > --- a/block/quorum.c > +++ b/block/quorum.c > @@ -86,10 +86,19 @@ static void quorum_aio_finalize(QuorumAIOCB *acb) > BDRVQuorumState *s = acb->bqs; > int ret = 0; > > + for (i = 0; i < s->total; i++) { > + qemu_vfree(acb->aios[i].buf); > + acb->aios[i].buf = NULL; > + acb->aios[i].ret = 0; You free acb->aios anyway, no point in clearing the fields. > + } > + > acb->common.cb(acb->common.opaque, ret); > if (acb->finished) { > *acb->finished = true; > } > + for (i = 0; acb->is_read && i < s->total; i++) { As Max noted, pulling acb->is_read into the for loop condition is clever. Code being unnecessarily clever is usually bad. > + qemu_iovec_destroy(&acb->aios[i].qiov); > + } Any reason to have two separate for loops? I can't see one why the qiov should be any longer valid than the data in it. I also can't see the reason for the different conditions, other than qemu_vfree() being a no-op for writes. Keep it simple: if (acb->is_read) { for (i = 0; i < s->total; i++) { qemu_vfree(acb->aios[i].buf); qemu_iovec_destroy(&acb->aios[i].qiov); } } It's shorter, less clever and more readable than your version. > g_free(acb->aios); > qemu_aio_release(acb); > } > @@ -145,6 +154,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->total; i++) { > + acb->aios[i].buf = qemu_blockalign(bs->file, qiov->size); Shouldn't this be s->bs[i] instead of bs->file? > + qemu_iovec_init(&acb->aios[i].qiov, qiov->niov); > + qemu_iovec_clone(&acb->aios[i].qiov, qiov, acb->aios[i].buf); > + } > + > + for (i = 0; i < s->total; i++) { > + bdrv_aio_readv(s->bs[i], sector_num, qiov, nb_sectors, > + quorum_aio_cb, &acb->aios[i]); > + } > + > + return &acb->common; > +} > + > static BlockDriverAIOCB *quorum_aio_writev(BlockDriverState *bs, > int64_t sector_num, > QEMUIOVector *qiov, > @@ -172,6 +209,7 @@ static BlockDriver bdrv_quorum = { > > .instance_size = sizeof(BDRVQuorumState), > > + .bdrv_aio_readv = quorum_aio_readv, > .bdrv_aio_writev = quorum_aio_writev, > }; Kevin
diff --git a/block/quorum.c b/block/quorum.c index 81bffdd..699b512 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -86,10 +86,19 @@ static void quorum_aio_finalize(QuorumAIOCB *acb) BDRVQuorumState *s = acb->bqs; int ret = 0; + for (i = 0; i < s->total; i++) { + qemu_vfree(acb->aios[i].buf); + acb->aios[i].buf = NULL; + acb->aios[i].ret = 0; + } + acb->common.cb(acb->common.opaque, ret); if (acb->finished) { *acb->finished = true; } + for (i = 0; acb->is_read && i < s->total; i++) { + qemu_iovec_destroy(&acb->aios[i].qiov); + } g_free(acb->aios); qemu_aio_release(acb); } @@ -145,6 +154,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->total; i++) { + acb->aios[i].buf = qemu_blockalign(bs->file, qiov->size); + qemu_iovec_init(&acb->aios[i].qiov, qiov->niov); + qemu_iovec_clone(&acb->aios[i].qiov, qiov, acb->aios[i].buf); + } + + for (i = 0; i < s->total; i++) { + bdrv_aio_readv(s->bs[i], sector_num, qiov, nb_sectors, + quorum_aio_cb, &acb->aios[i]); + } + + return &acb->common; +} + static BlockDriverAIOCB *quorum_aio_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov, @@ -172,6 +209,7 @@ static BlockDriver bdrv_quorum = { .instance_size = sizeof(BDRVQuorumState), + .bdrv_aio_readv = quorum_aio_readv, .bdrv_aio_writev = quorum_aio_writev, };