From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:59399)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <xiecl.fnst@cn.fujitsu.com>) id 1aVUZ0-0002Aw-Hc
	for qemu-devel@nongnu.org; Mon, 15 Feb 2016 20:36:27 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <xiecl.fnst@cn.fujitsu.com>) id 1aVUYv-0001Ei-TL
	for qemu-devel@nongnu.org; Mon, 15 Feb 2016 20:36:26 -0500
Received: from [59.151.112.132] (port=15278 helo=heian.cn.fujitsu.com)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <xiecl.fnst@cn.fujitsu.com>) id 1aVUYu-00017y-7A
	for qemu-devel@nongnu.org; Mon, 15 Feb 2016 20:36:21 -0500
Message-ID: <56C27D52.9050701@cn.fujitsu.com>
Date: Tue, 16 Feb 2016 09:37:22 +0800
From: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
MIME-Version: 1.0
References: <1455529968-21772-1-git-send-email-xiecl.fnst@cn.fujitsu.com>
	<1455529968-21772-2-git-send-email-xiecl.fnst@cn.fujitsu.com>
	<56C2069A.7090607@redhat.com>
In-Reply-To: <56C2069A.7090607@redhat.com>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 1/1] quorum: change vote rules for 64 bits
	hash
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Eric Blake <eblake@redhat.com>, qemu devel <qemu-devel@nongnu.org>, Alberto Garcia <berto@igalia.com>, Kevin Wolf <kwolf@redhat.com>
Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

On 02/16/2016 01:10 AM, Eric Blake wrote:
> On 02/15/2016 02:52 AM, Changlong Xie wrote:
>> Before:
>> 1) vote_count > max: winner = candidate, update max
>> 2) vote_count <= max: do nothing
>> Current:
>> 1) vote_count > max: winner = candidate, update max
>> 2) vote_count = max: compare the value of winner with
>> candidate, if candidate->value.l == 0, winner = candidate,
>> else do nothing
>> 3) vote_count < max: do nothing
>
> This says what you did, but not why.  Can you demonstrate a scenario
> that is broken without the patch, as part of your commit message?

Surely, will do in next version.

>
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> Signed-off-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
>> ---
>>   block/quorum.c | 14 ++++++++++----
>>   1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/block/quorum.c b/block/quorum.c
>> index a5ae4b8..e431ff4 100644
>> --- a/block/quorum.c
>> +++ b/block/quorum.c
>> @@ -446,7 +446,7 @@ static int quorum_compute_hash(QuorumAIOCB *acb, int i, QuorumVoteValue *hash)
>>       return 0;
>>   }
>>
>> -static QuorumVoteVersion *quorum_get_vote_winner(QuorumVotes *votes)
>> +static QuorumVoteVersion *quorum_get_vote_winner(QuorumVotes *votes, bool vote_error)
>
> Long line. Please wrap things to stay within 80 columns.
>

Ok

>>   {
>>       int max = 0;
>>       QuorumVoteVersion *candidate, *winner = NULL;
>> @@ -455,6 +455,12 @@ static QuorumVoteVersion *quorum_get_vote_winner(QuorumVotes *votes)
>>           if (candidate->vote_count > max) {
>>               max = candidate->vote_count;
>>               winner = candidate;
>> +            continue;
>> +        }
>> +        /* For 64 bit hash */
>> +        if (vote_error == true && candidate->vote_count == max
>
> s/ == true// (no need to do a redundant comparison of a bool against a
> bool).

Ok

Thanks
	-Xie

>
>