Re: [Qemu-devel] [PATCH 8/8] quorum: Inline quorum_fifo_aio_cb()

[Eric Blake]

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