Re: [PATCH 3/4] block/mirror: support unaligned write in active mirror

[Max Reitz]

On 03.10.19 11:34, Vladimir Sementsov-Ogievskiy wrote:
> 02.10.2019 18:52, Max Reitz wrote:
>> On 02.10.19 17:06, Vladimir Sementsov-Ogievskiy wrote:
>>> 02.10.2019 18:03, Vladimir Sementsov-Ogievskiy wrote:
>>>> 02.10.2019 17:57, Max Reitz wrote:
>>>>> On 12.09.19 17:13, Vladimir Sementsov-Ogievskiy wrote:
>>>>>> Prior 9adc1cb49af8d do_sync_target_write had a bug: it reset aligned-up
>>>>>> region in the dirty bitmap, which means that we may not copy some bytes
>>>>>> and assume them copied, which actually leads to producing corrupted
>>>>>> target.
>>>>>>
>>>>>> So 9adc1cb49af8d forced dirty bitmap granularity to be
>>>>>> request_alignment for mirror-top filter, so we are not working with
>>>>>> unaligned requests. However forcing large alignment obviously decreases
>>>>>> performance of unaligned requests.
>>>>>>
>>>>>> This commit provides another solution for the problem: if unaligned
>>>>>> padding is already dirty, we can safely ignore it, as
>>>>>> 1. It's dirty, it will be copied by mirror_iteration anyway
>>>>>> 2. It's dirty, so skipping it now we don't increase dirtiness of the
>>>>>>      bitmap and therefore don't damage "synchronicity" of the
>>>>>>      write-blocking mirror.
>>>>>
>>>>> But that’s not what active mirror is for.  The point of active mirror is
>>>>> that it must converge because every guest write will contribute towards
>>>>> that goal.
>>>>>
>>>>> If you skip active mirroring for unaligned guest writes, they will not
>>>>> contribute towards converging, but in fact lead to the opposite.
>>>>>
>>>>
>>>> The will not contribute only if region is already dirty. Actually, after
>>>> first iteration of mirroring (copying the whole disk), all following writes
>>>> will contribute, so the whole process must converge. It is a bit similar 
>>>> with
>>>> running one mirror loop in normal mode, and then enable write-blocking.
>>>>
>>>
>>>
>>> In other words, we don't need "all guest writes contribute" to converge,
>>> "all guest writes don't create new dirty bits" is enough, as we have 
>>> parallel
>>> mirror iteration which contiguously handles dirty bits.
>>
>> Hm, in a sense.  But it does mean that guest writes will not contribute
>> to convergence.
>>
>> And that’s against the current definition of write-blocking, which does
>> state that “when data is written to the source, write it (synchronously)
>> to the target as well”.
>>
> 
> Hmm, understand. But IMHO our proposed behavior is better in general.
> Do you think it's a problem to change spec now?
> If yes, I'll resend with an option

Well, the thing is that I’d find it weird if write-blocking wasn’t
blocking in all cases.  And in my opinion, it makes more sense for
active mirror if all writes actively contributed to convergence.

Max

signature.asc
Description: OpenPGP digital signature