Re: [Qemu-devel] [PATCH V10 10/13] quorum: Add quorum_co_flush().

[Max Reitz <mreitz@redhat.com>]

On 28.01.2014 17:52, Benoît Canet wrote:
> From: Benoît Canet <benoit@irqsave.net>
>
> Makes a vote to select error if any.
>
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
>   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 = bs->opaque;
> +    QuorumVoteVersion *winner = NULL;
> +    QuorumVotes error_votes;
> +    QuorumVoteValue result_value;
> +    int i;
> +    int result = 0;
> +    bool error = false;
> +
> +    QLIST_INIT(&error_votes.vote_list);
> +    error_votes.compare = quorum_64bits_compare;
> +
> +    for (i = 0; i < s->total; i++) {
> +        result = bdrv_co_flush(s->bs[i]);
> +        if (result) {
> +            error = true;
> +            result_value.l = result;
> +            quorum_count_vote(&error_votes, &result_value, i);
> +        }
> +    }
> +
> +    if (error) {
> +        winner = quorum_get_vote_winner(&error_votes);
> +        result = winner->value.l;
> +    }
> +
> +    quorum_free_vote_list(&error_votes);
> +
> +    return result;
> +}
> +
>   static BlockDriver bdrv_quorum = {
>       .format_name        = "quorum",
>       .protocol_name      = "quorum",
>   
>       .instance_size      = sizeof(BDRVQuorumState),
>   
> +    .bdrv_co_flush_to_disk = quorum_co_flush,
> +
>       .bdrv_getlength     = quorum_getlength,
>   
>       .bdrv_aio_readv     = 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.

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?

Max