From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44827) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c8usx-0006Yl-4g for qemu-devel@nongnu.org; Mon, 21 Nov 2016 15:08:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c8usw-0001rQ-2K for qemu-devel@nongnu.org; Mon, 21 Nov 2016 15:08:15 -0500 References: <1479749488-31808-1-git-send-email-kwolf@redhat.com> <1479749488-31808-9-git-send-email-kwolf@redhat.com> From: Eric Blake Message-ID: Date: Mon, 21 Nov 2016 14:08:06 -0600 MIME-Version: 1.0 In-Reply-To: <1479749488-31808-9-git-send-email-kwolf@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="oG23PItfu2UW5Pc96jPFUSeG2ro4geBIr" Subject: Re: [Qemu-devel] [PATCH 8/8] quorum: Inline quorum_fifo_aio_cb() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf , qemu-block@nongnu.org Cc: berto@igalia.com, mreitz@redhat.com, qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --oG23PItfu2UW5Pc96jPFUSeG2ro4geBIr From: Eric Blake To: Kevin Wolf , qemu-block@nongnu.org Cc: berto@igalia.com, mreitz@redhat.com, qemu-devel@nongnu.org Message-ID: Subject: Re: [PATCH 8/8] quorum: Inline quorum_fifo_aio_cb() References: <1479749488-31808-1-git-send-email-kwolf@redhat.com> <1479749488-31808-9-git-send-email-kwolf@redhat.com> In-Reply-To: <1479749488-31808-9-git-send-email-kwolf@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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. >=20 > Signed-off-by: Kevin Wolf > Reviewed-by: Alberto Garcia > --- > block/quorum.c | 42 +++++++++++++----------------------------- > 1 file changed, 13 insertions(+), 29 deletions(-) >=20 > QuorumVoteValue *value) > @@ -683,12 +659,20 @@ static int read_quorum_children(QuorumAIOCB *acb)= > static int read_fifo_child(QuorumAIOCB *acb) > { > BDRVQuorumState *s =3D acb->bs->opaque; > - int n =3D acb->children_read++; > - int ret; > + int n, ret; > + > + /* We try to read the next child in FIFO order if we failed to rea= d */ > + do { > + n =3D acb->children_read++; > + acb->qcrs[n].bs =3D s->children[n]->bs; > + ret =3D 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 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --oG23PItfu2UW5Pc96jPFUSeG2ro4geBIr Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJYM1QmAAoJEKeha0olJ0Nq/JYIAKImAU1cQCY9DGlzdt6zAt3s cgXp0V6K5c9yFhrONBq0NOJESSQ3lCndWhIgZ3BQa8MtoFQI1+h+8J+YXmgsQLx0 YvzfalAEZSzzhT+SmIE7T0Xo+RCj6FkRXNmLahOun4lrj7o21mqGAGXMsov55F+P k6rHRtojBENOBnMX30lNHFvgfsX+WRRODEVrbIGNlpZ/GbtvS9qmdP4s7/WdnCtF GI4oOCRK7hqM7TKVD7mWgbgE0RkmJ5PiYcGaZD3bkE+Xpe0p8Eb/TTX1Ne0hGF+8 Ln0AFv90VTPGqkQDQqVulreh3Vt0gIZK9PcrMvUwQK6n/I1QwIGJjWqMCqAqkZE= =+T5J -----END PGP SIGNATURE----- --oG23PItfu2UW5Pc96jPFUSeG2ro4geBIr--