Le Sunday 02 Feb 2014 à 22:44:07 (+0100), Max Reitz a écrit : > On 28.01.2014 17:52, Benoît Canet wrote: > >From: Benoît Canet <ben...@irqsave.net> > > > >Signed-off-by: Benoit Canet <ben...@irqsave.net> > >--- > > block/quorum.c | 67 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 67 insertions(+) > > > >diff --git a/block/quorum.c b/block/quorum.c > >index a47cd33..9b0718b 100644 > >--- a/block/quorum.c > >+++ b/block/quorum.c > >@@ -171,6 +171,22 @@ static int quorum_sha256_compare(QuorumVoteValue *a, > >QuorumVoteValue *b) > > return memcmp(a->h, b->h, HASH_LENGTH); > > } > >+static int quorum_64bits_compare(QuorumVoteValue *a, QuorumVoteValue *b) > >+{ > >+ int64_t i = a->l; > >+ int64_t j = b->l; > >+ > >+ if (i < j) { > >+ return -1; > >+ } > >+ > >+ if (i > j) { > >+ return 1; > >+ } > >+ > >+ return 0; > >+} > >+ > > static QuorumAIOCB *quorum_aio_get(BDRVQuorumState *s, > > BlockDriverState *bs, > > QEMUIOVector *qiov, > >@@ -587,6 +603,56 @@ static void quorum_invalidate_cache(BlockDriverState > >*bs) > > } > > } > >+static int64_t coroutine_fn quorum_co_get_block_status(BlockDriverState *bs, > >+ int64_t sector_num, > >+ int nb_sectors, > >+ int *pnum) > >+{ > >+ BDRVQuorumState *s = bs->opaque; > >+ QuorumVoteVersion *winner = NULL; > >+ QuorumVotes result_votes, num_votes; > >+ QuorumVoteValue result_value, num_value; > >+ int i, num; > >+ int64_t result = 0; > >+ > >+ QLIST_INIT(&result_votes.vote_list); > >+ QLIST_INIT(&num_votes.vote_list); > >+ result_votes.compare = quorum_64bits_compare; > >+ num_votes.compare = quorum_64bits_compare; > >+ > >+ for (i = 0; i < s->total; i++) { > >+ result = bdrv_get_block_status(s->bs[i], sector_num, nb_sectors, > >&num); > >+ /* skip failed requests */ > >+ if (result < 0) { > >+ continue; > >+ } > >+ result_value.l = result & BDRV_BLOCK_DATA; > >+ num_value.l = num; > >+ quorum_count_vote(&result_votes, &result_value, i); > >+ quorum_count_vote(&num_votes, &num_value, i); > >+ } > >+ > >+ winner = quorum_get_vote_winner(&result_votes); > >+ result = winner->value.l; > > Below, you're reading the winning value after checking whether it's > corresponding votes exceeded the threshold. It doesn't matter in the > end, but for the sake of uniformity, I'd do it the same way here > (i.e., move this statement below the if block). > > >+ if (winner->vote_count < s->threshold) { > >+ result = -ERANGE; > > Is there any specific reason why you're returning -ERANGE here and > -EIO everywhere else (even in quorum_getlength())?
I probably though "It's a comparison so it return -ERANGE". A bad reason so :( > > Max > > >+ goto free_exit; > >+ } > >+ > >+ winner = quorum_get_vote_winner(&num_votes); > >+ if (winner->vote_count < s->threshold) { > >+ result = -ERANGE; > >+ goto free_exit; > >+ } > >+ *pnum = winner->value.l; > >+ > >+free_exit: > >+ quorum_free_vote_list(&result_votes); > >+ quorum_free_vote_list(&num_votes); > >+ > >+ return result; > >+} > >+ > > static BlockDriver bdrv_quorum = { > > .format_name = "quorum", > > .protocol_name = "quorum", > >@@ -598,6 +664,7 @@ static BlockDriver bdrv_quorum = { > > .bdrv_aio_readv = quorum_aio_readv, > > .bdrv_aio_writev = quorum_aio_writev, > > .bdrv_invalidate_cache = quorum_invalidate_cache, > >+ .bdrv_co_get_block_status = quorum_co_get_block_status, > > }; > > static void bdrv_quorum_init(void) > >