From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47155) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WAOnA-0005mc-7x for qemu-devel@nongnu.org; Mon, 03 Feb 2014 14:02:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WAOn5-0007yh-6a for qemu-devel@nongnu.org; Mon, 03 Feb 2014 14:02:48 -0500 Received: from mx1.redhat.com ([209.132.183.28]:64797) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WAOn4-0007yL-UG for qemu-devel@nongnu.org; Mon, 03 Feb 2014 14:02:43 -0500 Message-ID: <52EFE84F.9020601@redhat.com> Date: Mon, 03 Feb 2014 20:04:47 +0100 From: Max Reitz MIME-Version: 1.0 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> <20140203115711.GD3078@irqsave.net> In-Reply-To: <20140203115711.GD3078@irqsave.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed 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: =?ISO-8859-1?Q?Beno=EEt_Canet?= Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com On 03.02.2014 12:57, Beno=EEt Canet wrote: > 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, >> 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 voti= ng. > >> 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 = winning > error will be returned. > Else 0 will be returned due to the vote result if the error is in minor= ity. > So the behavior is similar as long quorum is reached. You're only counting results which are not 0. Therefore, 0 will never=20 reach majority. Max >> Max