@samitouri / QOSamiQemu / commits / 0f51f9c342

mirror: Fix missed dirty bitmap writes during startup

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 <kwolf@redhat.com> Message-ID: <20260219202446.312493-1-kwolf@redhat.com> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com> Tested-by: Jean-Louis Dupond <jean-louis@dupond.be> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Kevin Wolf committed Feb 19, 2026 at 21:24 UTC 0f51f9c3420b31bb383e456dd7bf24d3056eeb73
1 file changed +32 -20
block/mirror.c
+32 -20
@@ -99,6 +99,7 @@ typedef struct MirrorBlockJob {
99
100 typedef struct MirrorBDSOpaque {
101 MirrorBlockJob *job;
102 + BdrvDirtyBitmap *dirty_bitmap;
103 bool stop;
104 bool is_commit;
105 } MirrorBDSOpaque;
@@ -1675,9 +1676,11 @@ bdrv_mirror_top_do_write(BlockDriverState *bs, MirrorMethod method,
1676 abort();
1677 }
1678
1678 - if (!copy_to_target && s->job && s->job->dirty_bitmap) {
1679 - qatomic_set(&s->job->actively_synced, false);
1680 - bdrv_set_dirty_bitmap(s->job->dirty_bitmap, offset, bytes);
1679 + if (!copy_to_target) {
1680 + if (s->job) {
1681 + qatomic_set(&s->job->actively_synced, false);
1682 + }
1683 + bdrv_set_dirty_bitmap(s->dirty_bitmap, offset, bytes);
1684 }
1685
1686 if (ret < 0) {
@@ -1904,13 +1907,35 @@ static BlockJob *mirror_start_job(
1907
1908 bdrv_drained_begin(bs);
1909 ret = bdrv_append(mirror_top_bs, bs, errp);
1907 - bdrv_drained_end(bs);
1908 -
1910 if (ret < 0) {
1911 + bdrv_drained_end(bs);
1912 + bdrv_unref(mirror_top_bs);
1913 + return NULL;
1914 + }
1915 +
1916 + bs_opaque->dirty_bitmap = bdrv_create_dirty_bitmap(mirror_top_bs,
1917 + granularity,
1918 + NULL, errp);
1919 + if (!bs_opaque->dirty_bitmap) {
1920 + bdrv_drained_end(bs);
1921 bdrv_unref(mirror_top_bs);
1922 return NULL;
1923 }
1924
1925 + /*
1926 + * The mirror job doesn't use the block layer's dirty tracking because it
1927 + * needs to be able to switch seemlessly between background copy mode (which
1928 + * does need dirty tracking) and write blocking mode (which doesn't) and
1929 + * doing that would require draining the node. Instead, mirror_top_bs takes
1930 + * care of updating the dirty bitmap as appropriate.
1931 + *
1932 + * Note that write blocking mode only becomes effective after mirror_run()
1933 + * sets mirror_top_opaque->job (see should_copy_to_target()). Until then,
1934 + * we're still in background copy mode irrespective of @copy_mode.
1935 + */
1936 + bdrv_disable_dirty_bitmap(bs_opaque->dirty_bitmap);
1937 + bdrv_drained_end(bs);
1938 +
1939 /* Make sure that the source is not resized while the job is running */
1940 s = block_job_create(job_id, driver, NULL, mirror_top_bs,
1941 BLK_PERM_CONSISTENT_READ,
@@ -2005,24 +2030,13 @@ static BlockJob *mirror_start_job(
2030 s->base_overlay = bdrv_find_overlay(bs, base);
2031 s->granularity = granularity;
2032 s->buf_size = ROUND_UP(buf_size, granularity);
2033 + s->dirty_bitmap = bs_opaque->dirty_bitmap;
2034 s->unmap = unmap;
2035 if (auto_complete) {
2036 s->should_complete = true;
2037 }
2038 bdrv_graph_rdunlock_main_loop();
2039
2014 - s->dirty_bitmap = bdrv_create_dirty_bitmap(s->mirror_top_bs, granularity,
2015 - NULL, errp);
2016 - if (!s->dirty_bitmap) {
2017 - goto fail;
2018 - }
2019 -
2020 - /*
2021 - * The dirty bitmap is set by bdrv_mirror_top_do_write() when not in active
2022 - * mode.
2023 - */
2024 - bdrv_disable_dirty_bitmap(s->dirty_bitmap);
2025 -
2040 bdrv_graph_wrlock_drained();
2041 ret = block_job_add_bdrv(&s->common, "source", bs, 0,
2042 BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE |
@@ -2102,9 +2116,6 @@ fail:
2116 g_free(s->replaces);
2117 blk_unref(s->target);
2118 bs_opaque->job = NULL;
2105 - if (s->dirty_bitmap) {
2106 - bdrv_release_dirty_bitmap(s->dirty_bitmap);
2107 - }
2119 job_early_fail(&s->common.job);
2120 }
2121
@@ -2118,6 +2129,7 @@ fail:
2129 bdrv_graph_wrunlock();
2130 bdrv_drained_end(bs);
2131
2132 + bdrv_release_dirty_bitmap(bs_opaque->dirty_bitmap);
2133 bdrv_unref(mirror_top_bs);
2134
2135 return NULL;