Re: [PATCH 3/9] mirror: implement mirror_change method

[Fiona Ebner]

Am 01.03.23 um 16:18 schrieb Vladimir Sementsov-Ogievskiy:
> On 24.02.23 17:48, Fiona Ebner wrote:
>> which allows switching the @copy-mode from 'background' to
>> 'write-blocking'.
>>
>> Once the job is in active mode, no new writes need to be registered in
>> the dirty bitmap, because they are synchronously written to the
>> target. But since the method is called from the monitor and IO might
>> be happening in an iothread at the same time, a drained section is
>> needed.
>>
>> This is useful for management applications, so they can start out in
>> background mode to avoid limiting guest write speed and switch to
>> active mode when certain criteria are fullfilled. Currently, the
>> amount of information that can be used for those criteria is a bit
>> limited, so the plan is to extend quering of block jobs to return more
>> information relevant for mirror.
>>
>> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>> ---
>>
>> Sorry, I still haven't fully grasped the drained logic. Is my
>> rationale for the drained section correct? There also are some yield
>> points in block/io.c, for example when the driver implements
>> bdrv_aio_pwritev (file-win32 and null), and the bitmap is only updated
>> after that. Is that another reason it's required?
>>
> 
> I think your logic is correct. But draining is quite expensive.
> 
> Could we avoid it? For example, just count separately
> in_flight_non_active writes (the writes that are started with copy_mode
> = BACKGROUND), and disable dirty bitmap when this counter becomes 0.
> 
> Or better idea: move the dirty bitmap to the filter, and make it always
> disabled. Then, we can set it by hand in bdrv_mirror_top_do_write() only
> when copy_to_target = false.
> 
Sounds good to me! I'll try to go for the second approach in the next
version.

Best Regards,
Fiona