Am 08.03.26 um 9:24 AM schrieb Michael Tokarev:
> On 19.02.2026 23:24, Kevin Wolf wrote:
>> Currently, mirror disables the block layer's dirty bitmap before its own
>> replacement is working. This means that during startup, there is a
>> window in which the allocation status of blocks in the source has
>> already been checked, but new writes coming in aren't tracked yet,
>> resulting in a corrupted copy:
>>
>> 1. Dirty bitmap is disabled in mirror_start_job()
>> 2. Some request are started in mirror_top_bs while s->job == NULL
>> 3. mirror_dirty_init() -> bdrv_co_is_allocated_above() runs and because
>> the request hasn't completed yet, the block isn't allocated
>> 4. The request completes, still sees s->job == NULL and skips the
>> bitmap, and nothing else will mark it dirty either
>>
>> One ingredient is that mirror_top_opaque->job is only set after the
>> job is fully initialized. For the rationale, see commit 32125b1460
>> ("mirror: Fix access of uninitialised fields during start").
>>
>> Fix this by giving mirror_top_bs access to dirty_bitmap and enabling it
>> to track writes from the beginning. Disabling the block layer's tracking
>> and enabling the mirror_top_bs one happens in a drained section, so
>> there is no danger of races with in-flight requests any more. All of
>> this happens well before the block allocation status is checked, so we
>> can be sure that no writes will be missed.
>>
>> Cc: [email protected]
>> Closes: https://gitlab.com/qemu-project/qemu/-/issues/3273
>> Fixes: 32125b14606a ('mirror: Fix access of uninitialised fields
>> during start')
>> Signed-off-by: Kevin Wolf <[email protected]>
>
> Hi!
>
> While trying to apply this one to 10.0.x series of qemu, I noticed there
> are a few other commits missing in this area. Namely, these commits are
> missing in there:
>
> 91ba0e1c382bd4 block: move drain outside of bdrv_change_aio_context()
> and mark GRAPH_RDLOCK
> ffdcd081f52544 block: move drain outside of bdrv_root_attach_child()
> 6b89e851fabf78 block: add bdrv_graph_wrlock_drained() convenience wrapper
>
> This is just a context, but I wonder if the resulting thing look sane.
> https://gitlab.com/mjt0k/qemu/-/
> commit/04c53bdbef634f93ac4149a19bf2de2e8d156b2b --
> can you take a look please?
FWIW, it looks fine to me :) The fix here should be orthogonal to the
draining changes from those commits.
Best Regards,
Fiona
