qemu/qemu.git/block/mirror.c, branch master

block: Add flags parameter to blk_*_pdiscard()

2026-05-19T15:25:48+00:00

All existing callers pass 0, but we need a way to pass BDRV_REQ_NO_QUEUE for discard requests. Signed-off-by: Kevin Wolf Message-ID: <20260421161132.99878-4-kwolf@redhat.com> Signed-off-by: Kevin Wolf

block/mirror: fix assertion failure upon duplicate complete for job using 'replaces'

2026-03-17T11:31:54+00:00

If s->replace_blocker was already set by an earlier invocation of mirror_complete(), then there will be an assertion failure when error_setg() is called for it a second time. The bdrv_op_block_all() and bdrv_ref() operations should only be done a single time too. Signed-off-by: Fiona Ebner Message-Id: <20260311145717.668492-2-f.ebner@proxmox.com> Reviewed-by: Hanna Czenczek Signed-off-by: Hanna Czenczek

mirror: Fix missed dirty bitmap writes during startup

2026-03-04T10:47:44+00:00

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: qemu-stable@nongnu.org Closes: https://gitlab.com/qemu-project/qemu/-/issues/3273 Fixes: 32125b14606a ('mirror: Fix access of uninitialised fields during start') Signed-off-by: Kevin Wolf Message-ID: <20260219202446.312493-1-kwolf@redhat.com> Reviewed-by: Fiona Ebner Tested-by: Jean-Louis Dupond Signed-off-by: Kevin Wolf

block/mirror: check range when setting zero bitmap for sync write

2026-02-12T08:38:17+00:00

Some Proxmox users reported an occasional assertion failure [0][1] in busy VMs when using drive mirror with active mode. In particular, the failure may occur for zero writes shorter than the job granularity: > #0 0x00007b421154b507 in abort () > #1 0x00007b421154b420 in ?? () > #2 0x0000641c582e061f in bitmap_set (map=0x7b4204014e00, start=14, nr=-1) > #3 0x0000641c58062824 in do_sync_target_write (job=0x641c7e73d1e0, > method=MIRROR_METHOD_ZERO, offset=852480, bytes=4096, qiov=0x0, flags=0) > #4 0x0000641c58062250 in bdrv_mirror_top_do_write (bs=0x641c7e62e1f0, method=MIRROR_METHOD_ZERO, copy_to_target=true, offset=852480, bytes=4096, qiov=0x0, flags=0) > #5 0x0000641c58061f31 in bdrv_mirror_top_pwrite_zeroes (bs=0x641c7e62e1f0, offset=852480, bytes=4096, flags=0) The range for the dirty bitmap described by dirty_bitmap_offset and dirty_bitmap_end is narrower than the original range and in fact, dirty_bitmap_end might be smaller than dirty_bitmap_offset. There already is a check for 'dirty_bitmap_offset < dirty_bitmap_end' before resetting the dirty bitmap. Add such a check for setting the zero bitmap too, which uses the same narrower range. [0]: https://forum.proxmox.com/threads/177981/ [1]: https://bugzilla.proxmox.com/show_bug.cgi?id=7222 Cc: qemu-stable@nongnu.org Fixes: 7e277545b9 ("mirror: Skip writing zeroes when target is already zero") Signed-off-by: Fiona Ebner Message-ID: <20260112152544.261923-1-f.ebner@proxmox.com> Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Vladimir Sementsov-Ogievskiy

block: drop wrapper for bdrv_set_backing_hd_drained()

2025-07-14T13:41:58+00:00

Nearly all callers (outside of the tests) are already using the _drained() variant of the function. It doesn't seem worth keeping. Simply adapt the remaining callers of bdrv_set_backing_hd() and rename bdrv_set_backing_hd_drained() to bdrv_set_backing_hd(). Signed-off-by: Fiona Ebner Message-ID: <20250530151125.955508-31-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf

block/mirror: switch to bdrv_set_backing_hd_drained() variant

2025-07-14T13:41:34+00:00

This is in preparation to mark bdrv_set_backing_hd() as GRAPH_UNLOCKED. Switch to using the bdrv_set_backing_hd_drained() variant, so that the drained and locked section can also cover the calls to bdrv_skip_filters() and bdrv_cow_bs(). Signed-off-by: Fiona Ebner Message-ID: <20250530151125.955508-26-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf

block: add bdrv_graph_wrlock_drained() convenience wrapper

2025-07-14T13:40:58+00:00

Many write-locked sections are also drained sections. A new bdrv_graph_wrunlock_drained() wrapper around bdrv_graph_wrunlock() is introduced, which will begin a drained section first. A global variable is used so bdrv_graph_wrunlock() knows if it also needs to end such a drained section. Both the aio_poll call in bdrv_graph_wrlock() and the aio_bh_poll() in bdrv_graph_wrunlock() can re-enter a write-locked section. While for the latter, ending the drain could be moved to before the call, the former requires that the variable is a counter and not just a boolean. Since the wrapper calls bdrv_drain_all_begin(), which must be called with the graph unlocked, mark the wrapper as GRAPH_UNLOCKED too. The switch to the new helpers was generated with the following commands and then manually checked: find . -name '*.c' -exec sed -i -z 's/bdrv_drain_all_begin();\n\s*bdrv_graph_wrlock();/bdrv_graph_wrlock_drained();/g' {} ';' find . -name '*.c' -exec sed -i -z 's/bdrv_graph_wrunlock();\n\s*bdrv_drain_all_end();/bdrv_graph_wrunlock();/g' {} ';' Suggested-by: Kevin Wolf Signed-off-by: Fiona Ebner Message-ID: <20250530151125.955508-25-f.ebner@proxmox.com> [kwolf: Removed redundant GRAPH_UNLOCKED] Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf

block: move drain outside of bdrv_root_attach_child()

2025-06-04T16:16:34+00:00

This is part of resolving the deadlock mentioned in commit "block: move draining out of bdrv_change_aio_context() and mark GRAPH_RDLOCK". The function bdrv_root_attach_child() runs under the graph lock, so it is not allowed to drain. It is called by: 1. blk_insert_bs(), where a drained section is introduced. 2. block_job_add_bdrv(), which holds the graph lock itself. block_job_add_bdrv() is called by: 1. mirror_start_job() 2. stream_start() 3. commit_start() 4. backup_job_create() 5. block_job_create() 6. In the test_blockjob_common_drain_node() unit test In all callers, a drained section is introduced. Signed-off-by: Fiona Ebner Reviewed-by: Kevin Wolf Message-ID: <20250530151125.955508-13-f.ebner@proxmox.com> Signed-off-by: Kevin Wolf

mirror: Reduce I/O when destination is detect-zeroes:unmap

2025-05-15T02:27:04+00:00

If we are going to punch holes in the mirror destination even for the portions where the source image is unallocated, it is nicer to treat the entire image as dirty and punch as we go, rather than pre-zeroing the entire image just to re-do I/O to the allocated portions of the image. Signed-off-by: Eric Blake Message-ID: <20250513220142.535200-2-eblake@redhat.com> Reviewed-by: Stefan Hajnoczi

mirror: Skip writing zeroes when target is already zero

2025-05-15T01:27:49+00:00

When mirroring, the goal is to ensure that the destination reads the same as the source; this goal is met whether the destination is sparse or fully-allocated (except when explicitly punching holes, then merely reading zero is not enough to know if it is sparse, so we still want to punch the hole). Avoiding a redundant write to zero (whether in the background because the zero cluster was marked in the dirty bitmap, or in the foreground because the guest is writing zeroes) when the destination already reads as zero makes mirroring faster, and avoids allocating the destination merely because the source reports as allocated. The effect is especially pronounced when the source is a raw file. That's because when the source is a qcow2 file, the dirty bitmap only visits the portions of the source that are allocated, which tend to be non-zero. But when the source is a raw file, bdrv_co_is_allocated_above() reports the entire file as allocated so mirror_dirty_init sets the entire dirty bitmap, and it is only later during mirror_iteration that we change to consulting the more precise bdrv_co_block_status_above() to learn where the source reads as zero. Remember that since a mirror operation can write a cluster more than once (every time the guest changes the source, the destination is also changed to keep up), and the guest can change whether a given cluster reads as zero, is discarded, or has non-zero data over the course of the mirror operation, we can't take the shortcut of relying on s->target_is_zero (which is static for the life of the job) in mirror_co_zero() to see if the destination is already zero, because that information may be stale. Any solution we use must be dynamic in the face of the guest writing or discarding a cluster while the mirror has been ongoing. We could just teach mirror_co_zero() to do a block_status() probe of the destination, and skip the zeroes if the destination already reads as zero, but we know from past experience that extra block_status() calls are not always cheap (tmpfs, anyone?), especially when they are random access rather than linear. Use of block_status() of the source by the background task in a linear fashion is not our bottleneck (it's a background task, after all); but since mirroring can be done while the source is actively being changed, we don't want a slow block_status() of the destination to occur on the hot path of the guest trying to do random-access writes to the source. So this patch takes a slightly different approach: any time we have to track dirty clusters, we can also track which clusters are known to read as zero. For sync=TOP or when we are punching holes from "detect-zeroes":"unmap", the zero bitmap starts out empty, but prevents a second write zero to a cluster that was already zero by an earlier pass; for sync=FULL when we are not punching holes, the zero bitmap starts out full if the destination reads as zero during initialization. Either way, I/O to the destination can now avoid redundant write zero to a cluster that already reads as zero, all without having to do a block_status() per write on the destination. With this patch, if I create a raw sparse destination file, connect it with QMP 'blockdev-add' while leaving it at the default "discard": "ignore", then run QMP 'blockdev-mirror' with "sync": "full", the destination remains sparse rather than fully allocated. Meanwhile, a destination image that is already fully allocated remains so unless it was opened with "detect-zeroes": "unmap". And any time writing zeroes is skipped, the job counters are not incremented. Signed-off-by: Eric Blake Message-ID: <20250509204341.3553601-26-eblake@redhat.com> Reviewed-by: Stefan Hajnoczi