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 -- Best regards, Vladimir