Le Friday 08 Feb 2013 à 11:38:38 (+0100), Kevin Wolf a écrit : > Am 28.01.2013 18:07, schrieb Benoît Canet: > > Signed-off-by: Benoit Canet <[email protected]> > > --- > > block/quorum.c | 111 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 111 insertions(+) > > > > diff --git a/block/quorum.c b/block/quorum.c > > index d8fffbe..5d8470b 100644 > > --- a/block/quorum.c > > +++ b/block/quorum.c > > @@ -52,11 +52,122 @@ struct QuorumAIOCB { > > int vote_ret; > > }; > > > > +static void quorum_aio_cancel(BlockDriverAIOCB *blockacb) > > +{ > > + QuorumAIOCB *acb = container_of(blockacb, QuorumAIOCB, common); > > + bool finished = false; > > + > > + /* Wait for the request to finish */ > > + acb->finished = &finished; > > + while (!finished) { > > + qemu_aio_wait(); > > + } > > +} > > + > > +static AIOCBInfo quorum_aiocb_info = { > > + .aiocb_size = sizeof(QuorumAIOCB), > > + .cancel = quorum_aio_cancel, > > +}; > > + > > +static void quorum_aio_bh(void *opaque) > > +{ > > + QuorumAIOCB *acb = opaque; > > + BDRVQuorumState *s = acb->bqs; > > + int ret; > > + > > + ret = s->threshold <= acb->success_count ? 0 : -EIO; > > It would be very much preferable if you stored the actual error code > instead of turning everything into -EIO. > > > + > > + qemu_bh_delete(acb->bh); > > + acb->common.cb(acb->common.opaque, ret); > > + if (acb->finished) { > > + *acb->finished = true; > > + } > > + g_free(acb->aios); > > + qemu_aio_release(acb); > > +} > > Move this down so that it's next to the function using the bottom half. > > > + > > +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->aios = g_new0(QuorumSingleAIOCB, s->total); > > + > > + acb->bqs = s; > > + acb->qiov = qiov; > > + acb->bh = NULL; > > + acb->count = 0; > > + acb->success_count = 0; > > + acb->sector_num = sector_num; > > + acb->nb_sectors = nb_sectors; > > + acb->vote = NULL; > > + acb->vote_ret = 0; > > + acb->finished = NULL; > > + > > + for (i = 0; i < s->total; i++) { > > + acb->aios[i].buf = NULL; > > + acb->aios[i].ret = 0; > > + acb->aios[i].parent = acb; > > + } > > Would you mind to reorder the initialisation of the fields according to > the order that is used in the struct definition? > > > + > > + return acb; > > +} > > + > > +static void quorum_aio_cb(void *opaque, int ret) > > +{ > > + QuorumSingleAIOCB *sacb = opaque; > > + QuorumAIOCB *acb = sacb->parent; > > + BDRVQuorumState *s = acb->bqs; > > + > > + sacb->ret = ret; > > + acb->count++; > > + if (ret == 0) { > > + acb->success_count++; > > + } > > + assert(acb->count <= s->total); > > + assert(acb->success_count <= s->total); > > + if (acb->count < s->total) { > > + return; > > + } > > + > > + acb->bh = qemu_bh_new(quorum_aio_bh, acb); > > + qemu_bh_schedule(acb->bh); > > What's the reason for using a bottom half here? Worth a comment? > > multiwrite_cb() in block.c doesn't use one to achieve something similar. > Is it buggy when you need one here? It think I get the bottom half by largely taking inspiration reading Marcello blkmirror code.
Best regards Benoît > > Kevin >
