From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39007) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WAI9O-0004GE-5z for qemu-devel@nongnu.org; Mon, 03 Feb 2014 06:57:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WAI9J-0006xJ-4Y for qemu-devel@nongnu.org; Mon, 03 Feb 2014 06:57:18 -0500 Received: from paradis.irqsave.net ([62.212.105.220]:48914) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WAI9I-0006xF-Pf for qemu-devel@nongnu.org; Mon, 03 Feb 2014 06:57:13 -0500 Date: Mon, 3 Feb 2014 12:57:11 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140203115711.GD3078@irqsave.net> References: <1390927974-31325-1-git-send-email-benoit.canet@irqsave.net> <1390927974-31325-11-git-send-email-benoit.canet@irqsave.net> <52EEC091.1000906@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <52EEC091.1000906@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH V10 10/13] quorum: Add quorum_co_flush(). List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: =?iso-8859-1?Q?Beno=EEt?= Canet , kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com Le Sunday 02 Feb 2014 =E0 23:02:57 (+0100), Max Reitz a =E9crit : > On 28.01.2014 17:52, Beno=EEt Canet wrote: > >From: Beno=EEt Canet > > > >Makes a vote to select error if any. > > > >Signed-off-by: Benoit Canet > >--- > > block/quorum.c | 34 ++++++++++++++++++++++++++++++++++ > > 1 file changed, 34 insertions(+) > > > >diff --git a/block/quorum.c b/block/quorum.c > >index 9b0718b..1b84b07 100644 > >--- a/block/quorum.c > >+++ b/block/quorum.c > >@@ -653,12 +653,46 @@ free_exit: > > return result; > > } > >+static coroutine_fn int quorum_co_flush(BlockDriverState *bs) > >+{ > >+ BDRVQuorumState *s =3D bs->opaque; > >+ QuorumVoteVersion *winner =3D NULL; > >+ QuorumVotes error_votes; > >+ QuorumVoteValue result_value; > >+ int i; > >+ int result =3D 0; > >+ bool error =3D false; > >+ > >+ QLIST_INIT(&error_votes.vote_list); > >+ error_votes.compare =3D quorum_64bits_compare; > >+ > >+ for (i =3D 0; i < s->total; i++) { > >+ result =3D bdrv_co_flush(s->bs[i]); > >+ if (result) { > >+ error =3D true; > >+ result_value.l =3D result; > >+ quorum_count_vote(&error_votes, &result_value, i); > >+ } > >+ } > >+ > >+ if (error) { > >+ winner =3D quorum_get_vote_winner(&error_votes); > >+ result =3D winner->value.l; > >+ } > >+ > >+ quorum_free_vote_list(&error_votes); > >+ > >+ return result; > >+} > >+ > > static BlockDriver bdrv_quorum =3D { > > .format_name =3D "quorum", > > .protocol_name =3D "quorum", > > .instance_size =3D sizeof(BDRVQuorumState), > >+ .bdrv_co_flush_to_disk =3D quorum_co_flush, > >+ > > .bdrv_getlength =3D quorum_getlength, > > .bdrv_aio_readv =3D quorum_aio_readv, >=20 > So, my general opinion on this patch (for reads/writes we don't vote > on the error code either; so why here?) hasn't changed, but well, I > definitely don't oppose it. For the reads/writes Kevin asked me to return the first error. For the flush function someone else (probably Eric) asked me to do voting= . >=20 > Another problem, however: If any error occurs, this function will > return an error as well. Is that intended? If an error on a > read/write operation occurs but there are still enough successful > reads/writes to reach quorum, no error is returned. Is there a > reason why this should be different for flush? If an error occurs multiple times and is able to establish quorum this wi= nning error will be returned. Else 0 will be returned due to the vote result if the error is in minorit= y. So the behavior is similar as long quorum is reached. >=20 > Max