Re: [Qemu-devel] [PATCH v2 1/1] quorum: Change vote rules for 64 bits hash

[Changlong Xie <xiecl.fnst@cn.fujitsu.com>]

On 02/20/2016 10:28 PM, Max Reitz wrote:
> On 19.02.2016 12:24, Alberto Garcia wrote:
>> On Fri 19 Feb 2016 09:26:53 AM CET, Wen Congyang <wency@cn.fujitsu.com> wrote:
>>
>>>>> If quorum has two children(A, B). A do flush sucessfully, but B
>>>>> flush failed.  We MUST choice A as winner rather than just pick
>>>>> anyone of them. Otherwise the filesystem of guest will become
>>>>> read-only with following errors:
>>>>>
>>>>> end_request: I/O error, dev vda, sector 11159960
>>>>> Aborting journal on device vda3-8
>>>>> EXT4-fs error (device vda3): ext4_journal_start_sb:327: Detected abort journal
>>>>> EXT4-fs (vda3): Remounting filesystem read-only
>>>>
>>>> Hi Xie,
>>>>
>>>> Let's see if I'm getting this right:
>>>>
>>>> - When Quorum flushes to disk, there's a vote among the return values of
>>>>    the flush operations of its members, and the one that wins is the one
>>>>    that Quorum returns.
>>>>
>>>> - If there's a tie then Quorum choses the first result from the list of
>>>>    winners.
>>>>
>>>> - With your patch you want to give priority to the vote with result == 0
>>>>    if there's any, so Quorum would return 0 (and succeed).
>>>>
>>>> This seems to me like an ad-hoc fix for a particular use case. What
>>>> if you have 3 members and two of them fail with the same error code?
>>>> Would you still return 0 or the error code from the other two?
>>>
>>> For example:
>>> children.0 returns 0
>>> children.1 returns -EIO
>>> children.2 returns -EPIPE
>>>
>>> In this case, quorum returns -EPIPE now(without this patch).
>>>
>>> For example:
>>> children.0 returns -EPIPE
>>> children.1 returns -EIO
>>> children.2 returns 0
>>> In this case, quorum returns 0 now.
>>
>> My question is: what's the rationale for returning 0 in case a) but not
>> in case b)?
>>
>>    a)
>>      children.0 returns -EPIPE
>>      children.1 returns -EIO
>>      children.2 returns 0
>>
>>    b)
>>      children.0 returns -EIO
>>      children.1 returns -EIO
>>      children.2 returns 0
>>
>> In both cases you have one successful flush and two errors. You want to
>> return always 0 in case a) and always -EIO in case b). But the only
>> difference is that in case b) the errors happen to be the same, so why
>> does that matter?
>>
>> That said, I'm not very convinced of the current logics of the Quorum
>> flush code either, so it's not even a problem with your patch... it
>> seems to me that the code should follow the same logics as in the
>> read/write case: if the number of correct flushes >= threshold then
>> return 0, else select the most common error code.
>
> I'm not convinced of the logic either, which is why I waited for you to
> respond to this patch. :-)
>
> Intuitively, I'd expect Quorum to return an error if flushing failed for
> any of the children, because, well, flushing failed. I somehow feel like
> flushing is different from a read or write operation and therefore
> ignoring the threshold would be fine here. However, maybe my intuition
> is just off.
>
> Anyway, regardless of that, if we do take the threshold into account, we
> should not use the exact error value for voting but just whether an
> error occurred or not. If all but one children fail to flush (all for
> different reasons), I find it totally wrong to return success. We should
> then just return -EIO or something.
>
Hi Berto & Max

Thanks for your comments, i'd like to have a summary here. For flush cases:

1) if flush successfully(result >= 0), result = 0; else if result < 0, 
result = -EIO. then invoke quorum_count_vote
2) if correct flushes >= threshold, mark correct flushes as winner directly.

Will fix in next version.

Thanks
	-Xie
> Max
>