On 22/11/2016 13:24, 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 <kw...@redhat.com>
> Reviewed-by: Alberto Garcia <be...@igalia.com>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
> ---
> block/quorum.c | 42 +++++++++++++-----------------------------
> 1 file changed, 13 insertions(+), 29 deletions(-)
>
> diff --git a/block/quorum.c b/block/quorum.c
> index c0994dc..52fa806 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -252,30 +252,6 @@ static void quorum_report_bad_acb(QuorumChildRequest
> *sacb, int ret)
> quorum_report_bad(type, acb->offset, acb->bytes, sacb->bs->node_name,
> ret);
> }
>
> -static int quorum_fifo_aio_cb(void *opaque, int ret)
> -{
> - QuorumChildRequest *sacb = opaque;
> - QuorumAIOCB *acb = sacb->parent;
> - BDRVQuorumState *s = acb->bs->opaque;
> -
> - assert(acb->is_read && s->read_pattern == QUORUM_READ_PATTERN_FIFO);
> -
> - if (ret < 0) {
> - quorum_report_bad_acb(sacb, ret);
> -
> - /* We try to read next child in FIFO order if we fail to read */
> - if (acb->children_read < s->num_children) {
> - return read_fifo_child(acb);
> - }
> - }
> -
> - acb->vote_ret = ret;
> -
> - /* FIXME: rewrite failed children if acb->children_read > 1? */
> -
> - return ret;
> -}
> -
> static void quorum_report_bad_versions(BDRVQuorumState *s,
> QuorumAIOCB *acb,
> QuorumVoteValue *value)
> @@ -685,12 +661,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->children_read is not used outside read_fifo_child, so you can get
rid of it. I'm not sure if this also means that you can rewrite this
loop as a "for (n = 0; n < s->num_children; n++)", such as by...
> + acb->qcrs[n].bs = s->children[n]->bs;
> + ret = bdrv_co_preadv(s->children[n], acb->offset, acb->bytes,
> + acb->qiov, 0);
... break-ing out here if ret >= 0.
Paolo
> + if (ret < 0) {
> + quorum_report_bad_acb(&acb->qcrs[n], ret);
> + }
> + } while (ret < 0 && acb->children_read < s->num_children);
>
> - acb->qcrs[n].bs = s->children[n]->bs;
> - ret = bdrv_co_preadv(s->children[n], acb->offset, acb->bytes, acb->qiov,
> 0);
> - ret = quorum_fifo_aio_cb(&acb->qcrs[n], ret);
> + /* FIXME: rewrite failed children if acb->children_read > 1? */
>
> return ret;
> }
>
