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

[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.


--
Best regards,
Vladimir