[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 8/8] quorum: Inline quorum_fifo_aio_cb()
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH 8/8] quorum: Inline quorum_fifo_aio_cb() |
Date: |
Mon, 21 Nov 2016 14:08:06 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 |
On 11/21/2016 11:31 AM, Kevin Wolf wrote:
> Inlining the function removes some boilerplace code and replaces
> recursion by a simple loop, so the code becomes somewhat easier to
> understand.
>
> Signed-off-by: Kevin Wolf <address@hidden>
> Reviewed-by: Alberto Garcia <address@hidden>
> ---
> block/quorum.c | 42 +++++++++++++-----------------------------
> 1 file changed, 13 insertions(+), 29 deletions(-)
>
> QuorumVoteValue *value)
> @@ -683,12 +659,20 @@ static int read_quorum_children(QuorumAIOCB *acb)
> static int read_fifo_child(QuorumAIOCB *acb)
> {
> BDRVQuorumState *s = acb->bs->opaque;
> - int n = acb->children_read++;
> - int ret;
> + int n, ret;
> +
> + /* We try to read the next child in FIFO order if we failed to read */
> + do {
> + n = acb->children_read++;
> + acb->qcrs[n].bs = s->children[n]->bs;
> + ret = bdrv_co_preadv(s->children[n], acb->offset, acb->bytes,
> + acb->qiov, 0);
> + if (ret < 0) {
> + quorum_report_bad_acb(&acb->qcrs[n], ret);
> + }
> + } while (ret < 0 && acb->children_read < s->num_children);
Back to my comments earlier in the series - We may want to think about a
parallel FIFO mode, where we kick off all reads, but then are prepared
to cancel reads on later children once we have a positive answer on an
earlier child. This rewrite may interfere with such a mode, where we'd
have to go back to the callback approach. But I'm not convinced we need
the complexity of a parallel fifo mode, and your rewrite is indeed
simpler to read, so I won't let it hold up my review.
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature