| Message ID | 1455529968-21772-2-git-send-email-xiecl.fnst@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
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? > > 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. > { > 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).
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 > >
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) { 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 + && candidate->value.l == 0) { + winner = candidate; } } @@ -544,7 +550,7 @@ static int quorum_vote_error(QuorumAIOCB *acb) } if (error) { - winner = quorum_get_vote_winner(&error_votes); + winner = quorum_get_vote_winner(&error_votes, false); ret = winner->value.l; } @@ -609,7 +615,7 @@ static bool quorum_vote(QuorumAIOCB *acb) } /* vote to select the most represented version */ - winner = quorum_get_vote_winner(&acb->votes); + winner = quorum_get_vote_winner(&acb->votes, false); /* if the winner count is smaller than threshold the read fails */ if (winner->vote_count < s->threshold) { @@ -769,7 +775,7 @@ static coroutine_fn int quorum_co_flush(BlockDriverState *bs) quorum_count_vote(&error_votes, &result_value, i); } - winner = quorum_get_vote_winner(&error_votes); + winner = quorum_get_vote_winner(&error_votes, true); result = winner->value.l; quorum_free_vote_list(&error_votes);