[Devel] [PATCH hci-8.1 v3 0/5] dm-qcow2: fix merges not stopping the users of the images they change

[Devel] [PATCH hci-8.1 v3 0/5] dm-qcow2: fix merges not stopping the users of the images they change

[Denis V. Lunev]

Checking dm-qcow2 for the same problem showed that a merge does not
stop everything using the images it changes (VSTOR-153049,
VSTOR-153050):

- a backward merge of an inner image starts without pausing anything,
  while reads enter through the top image. A read reaching the inner
  image as the merge starts oopses on a NULL qio->ext (patch 2);

- SEEK_HOLE/SEEK_DATA walks the images with nothing keeping them
  alive, so merge_backward complete frees an image under the walk
  (patch 3). Patch 4 (VSTOR-153422) fixes a seek that sleeps forever
  after a failed metadata read, which with patch 3 would also hold off
  the next merge_backward complete and every target message.

Patches 1 and 5 fix the refcount table relocation. Patch 1
(VSTOR-153051) is the counterpart of dm-ploop patch 4, an error path
leaving state behind: after a failed relocation, allocating writes past
the old refcount coverage fail with EIO until the device is suspended
and resumed. Patch 5 (VSTOR-153421) fixes a relocation failing on a
valid image once the new reftable fills a refcount group by itself,
with 4K clusters and refcount_bits=64 at about 511G.

The series does not depend on the dm-ploop one.

Tested on dev-cl04, stock 6.12.0-211.52.1.22.11.vz10, dm-qcow2 built
out of tree:
- with a test-only kprobe module widening the windows, on the stock
  module: with 4K clusters and refcount_bits=64 so the reftable
  relocates at 1G, a failed md page insert left every later write past
  1G failing with EIO; a seek went on with an image qcow2_destroy() had
  freed; an intermediate merge oopsed in submit_read_sliced_clu(). None
  of them reproduce with the series;
- SEEK_DATA over an image whose backing device starts failing reads
  returns EIO with patch 4;
- an image with a 511-cluster reftable covering 511G and every cluster
  in use: without patch 5 the first allocating write fails with EIO
  after a WARN, and qemu-img check then reports 513 errors; with it the
  writes go through and the check finds no errors;
- the ploop pytest suite, qcow2 format, passes.

Changes in v3, after Konstantin's review:
- 1/5: once the header write has succeeded, the error paths only return
  -EIO and leave the new md pages alone, as they may already be dirty
  and queued for writeback. The commit message says so in place of the
  note;
- 2/5: "no guest writes" in the comment, and the message says that
  starting or stopping a merge of an inner image now pauses guest I/O
  to the top image for the inflight ref switch;
- 3/5: the message says that merge_backward complete now waits for a
  running seek while it holds ctl_mutex;
- 4/5, 5/5: new, the two older bugs found in the review;
- rebased on rh10-6.12.0-211.52.1.22.17.vz10, Reviewed-by tags added
  to 2/5 and 3/5; Andrey's on 1/5 is dropped, as its code changed.

v2: http://lore.virtuozzo.com/kernel/20261103090813.166986-1-den@openvz.org/
v1: http://lore.virtuozzo.com/kernel/20261103083708.131379-1-den@openvz.org/

Denis V. Lunev (5):
  dm-qcow2: fix a failed refcount table relocation leaking md pages
  dm-qcow2: fix backward merge of an inner image racing its reads
  dm-qcow2: fix SEEK_HOLE/SEEK_DATA using an image freed by a merge
  dm-qcow2: fix SEEK_HOLE/SEEK_DATA hanging after a failed metadata read
  dm-qcow2: fix a refcount table relocation failing on a valid image

 drivers/md/dm-qcow2-cmd.c    | 28 ++++++++-----------
 drivers/md/dm-qcow2-map.c    | 53 ++++++++++++++++++++++++++----------
 drivers/md/dm-qcow2-target.c |  1 +
 drivers/md/dm-qcow2.h        |  2 ++
 4 files changed, 54 insertions(+), 30 deletions(-)


base-commit: 95ab11e2c57e8468412ca586d4be8cd75b6bfbf0
-- 
2.53.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v3 1/5] dm-qcow2: fix a failed refcount table relocation leaking md pages

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

relocate_refcount_table() inserts md pages for the last cluster of the
new reftable and for the new refblocks, one page at a time from
pos + old_clus * clu_size up to r2_end. On error it frees pages from
end only, one per cluster, so the pages of the new reftable cluster
and all but the first page of each refblock cluster bigger than a page
stay in md_pages.

The header is rolled back, so the next allocation past the old refcount
coverage relocates the table again. Its first insert finds a leaked page
and fails with -EEXIST, the relocation returns -EIO, and every
allocating write beyond the old coverage fails with an I/O error until
the device is suspended and resumed or the table is reloaded. A failed
page allocation or a failed header write is enough to get there.

Free exactly the pages the allocation loop inserted when the relocation
fails before the header write. Once the header write has succeeded, the
error paths skip the freeing and only return -EIO: the header on disk
already points at the new table, and its pages may be dirty and queued
for writeback by then.

Fixes: 222b6f1291c1 ("dm-qcow2: Introduce driver to create block devices over QCOW2 files")
https://virtuozzo.atlassian.net/browse/VSTOR-153051
Feature: dm-qcow2: block device over QCOW2 files driver
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/md/dm-qcow2-map.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 1c269a78ad35..ac4dbe0623ea 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -2277,6 +2277,7 @@ static void set_reftable_in_raw_hdr(struct page *page0, loff_t pos, loff_t clus)
 static int relocate_refcount_table(struct qcow2 *qcow2, struct qio **qio)
 {
 	loff_t i, old_pos, old_end, pos, end, r2_end, delta;
+	loff_t new_md_start, new_md_end;
 	u32 old_clus, clus, clu_size = qcow2->clu_size;
 	u32 r2_clus, bits = qcow2->refblock_bits;
 	unsigned long nr_pages, index;
@@ -2340,8 +2341,9 @@ static int relocate_refcount_table(struct qcow2 *qcow2, struct qio **qio)
 		return ret;
 
 	/* Alloc R1/R2 pages covering clusters of new R1 and new R2 */
-	for (i = pos + (u64)old_clus * clu_size; i < r2_end; i += PAGE_SIZE) {
-		ret = qcow2_alloc_and_insert_md_page(qcow2, i >> PAGE_SHIFT, &md);
+	new_md_start = pos + (u64)old_clus * clu_size;
+	for (new_md_end = new_md_start; new_md_end < r2_end; new_md_end += PAGE_SIZE) {
+		ret = qcow2_alloc_and_insert_md_page(qcow2, new_md_end >> PAGE_SHIFT, &md);
 		if (ret < 0)
 			goto err_free_r2_pages;
 		spin_lock_irq(&qcow2->md_pages_lock);
@@ -2377,21 +2379,21 @@ static int relocate_refcount_table(struct qcow2 *qcow2, struct qio **qio)
 		if (!WARN_ON_ONCE(!md))
 			qcow2_md_make_dirty(qcow2, md, true);
 		if (!md)
-			break; /* goto err_free_r2_pages */
+			break; /* goto err_eio */
 	}
 	spin_unlock_irq(&qcow2->md_pages_lock);
 	if (i != nr_pages)
-		goto err_free_r2_pages;
+		goto err_eio;
 
 	/* Connect new R2 to new R1 */
 	for (i = end; i < r2_end; i += clu_size) {
 		if (calc_refcounters_map(qcow2, i, &r1, &r2) < 0) {
 			WARN_ON_ONCE(1);
-			goto err_free_r2_pages;
+			goto err_eio;
 		}
 		ret = handle_md_page(qcow2, r1.page_id, NULL, &md);
 		if (WARN_ON_ONCE(ret <= 0))
-			goto err_free_r2_pages;
+			goto err_eio;
 		spin_lock_irq(&qcow2->md_pages_lock);
 		set_u64_to_be_page(md->page, r1.index_in_page, i);
 		qcow2_md_make_dirty(qcow2, md, true);
@@ -2402,7 +2404,7 @@ static int relocate_refcount_table(struct qcow2 *qcow2, struct qio **qio)
 	for (i = pos; i < r2_end; i += clu_size) {
 		ret = qcow2_handle_r1r2_maps(qcow2, i, NULL, &r1, &r2, false);
 		if (WARN_ON_ONCE(ret <= 0))
-			goto err_free_r2_pages;
+			goto err_eio;
 		mark_cluster_used(qcow2, r2.md, r2.index_in_page);
 	}
 
@@ -2410,14 +2412,14 @@ static int relocate_refcount_table(struct qcow2 *qcow2, struct qio **qio)
 	for (i = old_pos; i < old_end; i += clu_size) {
 		ret = qcow2_handle_r1r2_maps(qcow2, i, NULL, &r1, &r2, false);
 		if (WARN_ON_ONCE(ret <= 0))
-			goto err_free_r2_pages;
+			goto err_eio;
 		mark_cluster_unused(qcow2, r2.md, r2.index_in_page, i);
 	}
 
 	return 1;
 
 err_free_r2_pages:
-	for (i = end; i < r2_end; i += clu_size) {
+	for (i = new_md_start; i < new_md_end; i += PAGE_SIZE) {
 		ret = handle_md_page(qcow2, i >> PAGE_SHIFT, NULL, &md);
 		if (ret <= 0)
 			break;
@@ -2426,6 +2428,7 @@ static int relocate_refcount_table(struct qcow2 *qcow2, struct qio **qio)
 		spin_unlock_irq(&qcow2->md_pages_lock);
 		qcow2_free_md_page(md);
 	}
+err_eio:
 	/* TODO: switch to RO */
 	return -EIO;
 }
-- 
2.53.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v3 2/5] dm-qcow2: fix backward merge of an inner image racing its reads

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

set_backward_merge_in_process() sets the flag of an image other than the
top one without stopping submission, as nothing writes to it. It is
still read: a read of a cluster the top image does not map goes on to
the lower image, and when that image is writable its worker tests the
flag twice for the qio. When the merge starts between the two tests, the
first one, in handle_metadata(), skips prepare_backward_merge() and
leaves qio->ext NULL, the second one, in process_one_qio(), takes the
merge path and submit_top_delta_read() dereferences qio->ext.

Pause submission and switch the inflight refs for every image. The
pause flag is only tested on the top image, where all qios enter, so
pause that one; clearing the flag of an inner image paused the inner
image itself, which held nothing back.

Starting or stopping a merge of an inner image now pauses guest I/O to
the top image for the inflight ref switch, as a merge of the top image
always did.

Fixes: 8e522723a085 ("dm-qcow2: allow specifying depth to merge intermediate images")
https://virtuozzo.atlassian.net/browse/VSTOR-153049
Feature: dm-qcow2: block device over QCOW2 files driver
Reviewed-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/md/dm-qcow2-cmd.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/md/dm-qcow2-cmd.c b/drivers/md/dm-qcow2-cmd.c
index e520d8fd6929..0e2c810f657b 100644
--- a/drivers/md/dm-qcow2-cmd.c
+++ b/drivers/md/dm-qcow2-cmd.c
@@ -149,34 +149,28 @@ static int qcow2_break_l1cow(struct qcow2_target *tgt, struct qcow2 *qcow2)
 static void set_backward_merge_in_process(struct qcow2_target *tgt,
 				     struct qcow2 *qcow2, bool set)
 {
+	struct qcow2 *top = tgt->top;
 	LIST_HEAD(list);
 
-	/*
-	 * There are no writes if it is not the top qcow2 image.
-	 * so we do not need to stop and flush requests when setting
-	 */
-	if (qcow2 != tgt->top && set) {
-		qcow2->backward_merge_in_process = set;
-		return;
-	}
-
 	/*
 	 * To avoid race between allocations and COWS
 	 * we completely stop queueing qios and wait
 	 * for pending qios. Lock is for visability.
+	 * A lower image has no guest writes, but it is read by
+	 * qios of the top one, so stop the top image even then.
 	 */
-	spin_lock_irq(&qcow2->deferred_lock);
-	qcow2->pause_submitting_qios = true;
-	spin_unlock_irq(&qcow2->deferred_lock);
+	spin_lock_irq(&top->deferred_lock);
+	top->pause_submitting_qios = true;
+	spin_unlock_irq(&top->deferred_lock);
 	qcow2_inflight_ref_switch(tgt);
 
 	/* queue is stopped */
-	spin_lock_irq(&qcow2->deferred_lock);
+	spin_lock_irq(&top->deferred_lock);
 	WARN_ON_ONCE(qcow2->backward_merge_in_process == set);
 	qcow2->backward_merge_in_process = set;
-	qcow2->pause_submitting_qios = false;
-	list_splice_init(&qcow2->paused_qios, &list);
-	spin_unlock_irq(&qcow2->deferred_lock);
+	top->pause_submitting_qios = false;
+	list_splice_init(&top->paused_qios, &list);
+	spin_unlock_irq(&top->deferred_lock);
 
 	qcow2_submit_embedded_qios(tgt, &list);
 }
-- 
2.53.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v3 3/5] dm-qcow2: fix SEEK_HOLE/SEEK_DATA using an image freed by a merge

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

qcow2_llseek_hole() starts at tgt->top and follows qcow2->lower, parsing
the metadata of each image, with nothing keeping the images alive. dm
core holds only the table SRCU around the call, which does not keep a
target message out. merge_backward complete unlinks the merged image,
waits in qcow2_inflight_ref_switch() for the qios holding refs and
destroys the image, so a seek already walking it goes on with a freed
struct qcow2, its md pages and its file. A SEEK_DATA scan of a large
sparse image keeps the walk running long enough to hit it.

Hold a per-target rw_semaphore for read over the walk, and for write
while merge_backward complete unlinks and destroys the image. An
inflight ref held over the walk would also do, but every inflight ref
switch would then wait for the whole scan, including the one starting a
backward merge with the top image paused, which stalls guest I/O for as
long as the scan runs.

merge_backward complete now waits for a running seek while it holds
ctl_mutex, so target messages and the presuspend of the device queue
behind it until the scan ends.

Fixes: 412d5a4f7bf1 ("dm/dm-qcow2: add llseek_hole")
https://virtuozzo.atlassian.net/browse/VSTOR-153050
Feature: dm-qcow2: block device over QCOW2 files driver
Reviewed-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/md/dm-qcow2-cmd.c    |  2 ++
 drivers/md/dm-qcow2-map.c    | 10 ++++++++--
 drivers/md/dm-qcow2-target.c |  1 +
 drivers/md/dm-qcow2.h        |  2 ++
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-qcow2-cmd.c b/drivers/md/dm-qcow2-cmd.c
index 0e2c810f657b..45c517ce0ef8 100644
--- a/drivers/md/dm-qcow2-cmd.c
+++ b/drivers/md/dm-qcow2-cmd.c
@@ -373,6 +373,7 @@ static int qcow2_merge_backward_complete(struct qcow2_target *tgt)
 	if (tgt->backward_merge.state != BACKWARD_MERGE_WAIT_COMPLETION)
 		return -EBUSY;
 
+	down_write(&tgt->seek_rwsem);
 	qcow2 = *tgt->backward_merge.pqcow2;
 	*tgt->backward_merge.pqcow2 = qcow2->lower;
 
@@ -394,6 +395,7 @@ static int qcow2_merge_backward_complete(struct qcow2_target *tgt)
 			QC_ERR(tgt->ti, "Can't unuse merged img (%d)", ret);
 	}
 	qcow2_destroy(qcow2);
+	up_write(&tgt->seek_rwsem);
 
 	tgt->backward_merge.state = BACKWARD_MERGE_STOPPED;
 	__backward_merge_update_stage(tgt, BACKWARD_MERGE_STAGE_NONE);
diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index ac4dbe0623ea..fa535d7716da 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -4844,18 +4844,24 @@ static int qcow2_llseek_hole_qio(struct qio *qio, int whence, loff_t *result)
 
 loff_t qcow2_llseek_hole(struct dm_target *ti, loff_t offset, int whence)
 {
-	struct qcow2 *qcow2 = to_qcow2_target(ti)->top;
+	struct qcow2_target *tgt = to_qcow2_target(ti);
 	loff_t result = -EINVAL;
+	struct qcow2 *qcow2;
 	struct qio *qio;
 	int ret;
 
+	down_read(&tgt->seek_rwsem);
+	qcow2 = tgt->top;
 	qio = alloc_seek_qio(qcow2, NULL, qcow2->hdr.size);
-	if (!qio)
+	if (!qio) {
+		up_read(&tgt->seek_rwsem);
 		return -ENOMEM;
+	}
 
 	seek_qio_set_sector(qio, to_sector(offset));
 
 	ret = qcow2_llseek_hole_qio(qio, whence, &result);
+	up_read(&tgt->seek_rwsem);
 	/* In case of error remap ENXIO as it have special meaning for llseek */
 	if (ret < 0)
 		return (ret == -ENXIO) ? -EINVAL : ret;
diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
index 4a0e5747b77a..7c572bd66e1c 100644
--- a/drivers/md/dm-qcow2-target.c
+++ b/drivers/md/dm-qcow2-target.c
@@ -491,6 +491,7 @@ static struct qcow2_target *alloc_qcow2_target(struct dm_target *ti)
 	init_completion(&tgt->inflight_ref_comp);
 	spin_lock_init(&tgt->event_lock);
 	mutex_init(&tgt->ctl_mutex);
+	init_rwsem(&tgt->seek_rwsem);
 	init_waitqueue_head(&tgt->service_wq);
 	INIT_WORK(&tgt->event_work, qcow2_event_work);
 	INIT_LIST_HEAD(&tgt->enospc_qios);
diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
index 8f10c5812a1b..ad5286b16883 100644
--- a/drivers/md/dm-qcow2.h
+++ b/drivers/md/dm-qcow2.h
@@ -212,6 +212,8 @@ struct qcow2_target {
 	struct work_struct event_work;
 	spinlock_t event_lock;
 	struct mutex ctl_mutex;
+	/* Keeps merge_backward complete from freeing images a seek walks */
+	struct rw_semaphore seek_rwsem;
 
 	struct qcow2_backward_merge backward_merge;
 };
-- 
2.53.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v3 4/5] dm-qcow2: fix SEEK_HOLE/SEEK_DATA hanging after a failed metadata read

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

qcow2_llseek_hole_qio() parks its seek qio on an md page that is not
loaded yet and sleeps until the qio is dispatched to QLIST_SEEK, where
process_seek_qios() completes it. When the page read fails,
do_md_page_read_complete() ends the waiting qios instead. A seek qio
has no endio_cb, so nothing wakes the walk, and the lseek() caller
sleeps uninterruptibly forever while it holds seek_rwsem: the next
merge_backward complete and every target message after it hang under
ctl_mutex as well.

Complete the seek from an endio_cb and return the error the qio was
ended with.

Fixes: 412d5a4f7bf1 ("dm/dm-qcow2: add llseek_hole")
Reported-by: Konstantin Khorenko <khorenko@virtuozzo.com>
https://virtuozzo.atlassian.net/browse/VSTOR-153422
Feature: dm-qcow2: block device over QCOW2 files driver
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/md/dm-qcow2-map.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index fa535d7716da..eee865989ef7 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -4600,6 +4600,13 @@ static void seek_qio_set_sector(struct qio *qio, sector_t bi_sector)
 	qio->bi_iter.bi_size = end > pos ? end - pos : 0;
 }
 
+/* A failed md page read ends the qios waiting for it */
+static void seek_qio_endio(struct qcow2_target *tgt, struct qio *qio,
+			   void *data, blk_status_t bi_status)
+{
+	complete(&((struct qio_data_llseek_hole *)data)->compl);
+}
+
 static struct qio *alloc_seek_qio(struct qcow2 *qcow2, struct qio *parent, loff_t new_lim)
 {
 	struct qio_data_llseek_hole *data;
@@ -4619,6 +4626,8 @@ static struct qio *alloc_seek_qio(struct qcow2 *qcow2, struct qio *parent, loff_
 	}
 
 	qio->data = data;
+	qio->endio_cb = seek_qio_endio;
+	qio->endio_cb_data = data;
 	init_completion(&data->compl);
 	data->lim = new_lim;
 
@@ -4752,6 +4761,10 @@ static int qcow2_llseek_hole_qio(struct qio *qio, int whence, loff_t *result)
 			/* one of metadata pages is not loaded and qio is postponed*/
 			wait_for_completion(&SEEK_QIO_DATA(qio)->compl);
 			reinit_completion(&SEEK_QIO_DATA(qio)->compl);
+			if (qio->bi_status != BLK_STS_OK) {
+				ret = blk_status_to_errno(qio->bi_status);
+				break;
+			}
 			continue;
 		}
 
-- 
2.53.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v3 5/5] dm-qcow2: fix a refcount table relocation failing on a valid image

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

relocate_refcount_table() puts the new reftable at pos, the start of
the first refcount group the old table does not cover, and the new
refblocks right after it. It then gives each new refblock to the group
the refblock itself sits in. When the new reftable fills the group of
pos by itself, the refblocks all land in the next group, and the group
of pos gets none. Marking the reftable clusters used then fails with a
WARN, and every allocating write past the old refcount coverage fails
with EIO.

A valid image gets there: with 4K clusters and refcount_bits=64 a
refcount group is 512 clusters, and the reftable reaches 512 clusters
when the image file grows to about 511G; with refcount_bits=32, at
about 2T.

Give the new refblocks to the groups the new reftable and refblocks
span, one each, in order. pos is group-aligned, and r2_clus is computed
for the refblocks to count both themselves and the new table, so there
is a refblock for each of these groups.

Fixes: 222b6f1291c1 ("dm-qcow2: Introduce driver to create block devices over QCOW2 files")
Reported-by: Konstantin Khorenko <khorenko@virtuozzo.com>
https://virtuozzo.atlassian.net/browse/VSTOR-153421
Feature: dm-qcow2: block device over QCOW2 files driver
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/md/dm-qcow2-map.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index eee865989ef7..34d9d9810f60 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -2277,7 +2277,7 @@ static void set_reftable_in_raw_hdr(struct page *page0, loff_t pos, loff_t clus)
 static int relocate_refcount_table(struct qcow2 *qcow2, struct qio **qio)
 {
 	loff_t i, old_pos, old_end, pos, end, r2_end, delta;
-	loff_t new_md_start, new_md_end;
+	loff_t new_md_start, new_md_end, group_size, r2_pos;
 	u32 old_clus, clus, clu_size = qcow2->clu_size;
 	u32 r2_clus, bits = qcow2->refblock_bits;
 	unsigned long nr_pages, index;
@@ -2385,8 +2385,11 @@ static int relocate_refcount_table(struct qcow2 *qcow2, struct qio **qio)
 	if (i != nr_pages)
 		goto err_eio;
 
-	/* Connect new R2 to new R1 */
-	for (i = end; i < r2_end; i += clu_size) {
+	/* Connect new R2 to new R1: one for each group new R1 and R2 span */
+	group_size = (u64)qcow2->refblock_entries * clu_size;
+	for (i = pos, r2_pos = end; i < r2_end; i += group_size, r2_pos += clu_size) {
+		if (WARN_ON_ONCE(r2_pos >= r2_end))
+			goto err_eio;
 		if (calc_refcounters_map(qcow2, i, &r1, &r2) < 0) {
 			WARN_ON_ONCE(1);
 			goto err_eio;
@@ -2395,7 +2398,7 @@ static int relocate_refcount_table(struct qcow2 *qcow2, struct qio **qio)
 		if (WARN_ON_ONCE(ret <= 0))
 			goto err_eio;
 		spin_lock_irq(&qcow2->md_pages_lock);
-		set_u64_to_be_page(md->page, r1.index_in_page, i);
+		set_u64_to_be_page(md->page, r1.index_in_page, r2_pos);
 		qcow2_md_make_dirty(qcow2, md, true);
 		spin_unlock_irq(&qcow2->md_pages_lock);
 	}
-- 
2.53.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v3 0/5] dm-qcow2: fix merges not stopping the users of the images they change

[Konstantin Khorenko]

On Wed, 04 Nov 2026 19:01:00 +0100, Denis V. Lunev wrote:
> dm-qcow2: fix merges not stopping the users of the images they change
> 
> Checking dm-qcow2 for the same problem showed that a merge does not
> stop everything using the images it changes (VSTOR-153049,
> VSTOR-153050):
> 
> - a backward merge of an inner image starts without pausing anything,
>   while reads enter through the top image. A read reaching the inner
>   image as the merge starts oopses on a NULL qio->ext (patch 2);
> 
> [...]

Applied to branch-rh10-6.12.0-211.52.1.22.x.vz10-ovz, will be released in rh10-6.12.0-211.52.1.22.18.vz10, thanks!

[1/5] dm-qcow2: fix a failed refcount table relocation leaking md pages
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/beada5e3721a
[2/5] dm-qcow2: fix backward merge of an inner image racing its reads
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/917b0f940acc
[3/5] dm-qcow2: fix SEEK_HOLE/SEEK_DATA using an image freed by a merge
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/5400a597e4d6
[4/5] dm-qcow2: fix SEEK_HOLE/SEEK_DATA hanging after a failed metadata read
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/334bce6208ed
[5/5] dm-qcow2: fix a refcount table relocation failing on a valid image
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/6f2dd6d659e1

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v3 2/5] dm-qcow2: fix backward merge of an inner image racing its reads

[no-reply]

> From: Denis V. Lunev <den@openvz.org>
> 
> set_backward_merge_in_process() sets the flag of an image other than the
> top one without stopping submission, as nothing writes to it. It is
> still read: a read of a cluster the top image does not map goes on to
> the lower image, and when that image is writable its worker tests the
> flag twice for the qio. When the merge starts between the two tests, the
> first one, in handle_metadata(), skips prepare_backward_merge() and
> leaves qio->ext NULL, the second one, in process_one_qio(), takes the
> merge path and submit_top_delta_read() dereferences qio->ext.
> 
> Pause submission and switch the inflight refs for every image. The
> pause flag is only tested on the top image, where all qios enter, so
> pause that one; clearing the flag of an inner image paused the inner
> image itself, which held nothing back.
> 
> Starting or stopping a merge of an inner image now pauses guest I/O to
> the top image for the inflight ref switch, as a merge of the top image
> always did.
> 
> Fixes: 8e522723a085 ("dm-qcow2: allow specifying depth to merge intermediate images")
> https://virtuozzo.atlassian.net/browse/VSTOR-153049
> Feature: dm-qcow2: block device over QCOW2 files driver
> Reviewed-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
> Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261104180105.1807820-1-den@openvz.org?part=2


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v3 1/5] dm-qcow2: fix a failed refcount table relocation leaking md pages

[no-reply]

> From: Denis V. Lunev <den@openvz.org>
> 
> relocate_refcount_table() inserts md pages for the last cluster of the
> new reftable and for the new refblocks, one page at a time from
> pos + old_clus * clu_size up to r2_end. On error it frees pages from
> end only, one per cluster, so the pages of the new reftable cluster
> and all but the first page of each refblock cluster bigger than a page
> stay in md_pages.
> 
> The header is rolled back, so the next allocation past the old refcount
> coverage relocates the table again. Its first insert finds a leaked page
> and fails with -EEXIST, the relocation returns -EIO, and every
> allocating write beyond the old coverage fails with an I/O error until
> the device is suspended and resumed or the table is reloaded. A failed
> page allocation or a failed header write is enough to get there.
> 
> Free exactly the pages the allocation loop inserted when the relocation
> fails before the header write. Once the header write has succeeded, the
> error paths skip the freeing and only return -EIO: the header on disk
> already points at the new table, and its pages may be dirty and queued
> for writeback by then.
> 
> Fixes: 222b6f1291c1 ("dm-qcow2: Introduce driver to create block devices over QCOW2 files")
> https://virtuozzo.atlassian.net/browse/VSTOR-153051
> Feature: dm-qcow2: block device over QCOW2 files driver
> Signed-off-by: Denis V. Lunev <den@openvz.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261104180105.1807820-1-den@openvz.org?part=1


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v3 3/5] dm-qcow2: fix SEEK_HOLE/SEEK_DATA using an image freed by a merge

[no-reply]

> From: Denis V. Lunev <den@openvz.org>
> 
> qcow2_llseek_hole() starts at tgt->top and follows qcow2->lower, parsing
> the metadata of each image, with nothing keeping the images alive. dm
> core holds only the table SRCU around the call, which does not keep a
> target message out. merge_backward complete unlinks the merged image,
> waits in qcow2_inflight_ref_switch() for the qios holding refs and
> destroys the image, so a seek already walking it goes on with a freed
> struct qcow2, its md pages and its file. A SEEK_DATA scan of a large
> sparse image keeps the walk running long enough to hit it.
> 
> Hold a per-target rw_semaphore for read over the walk, and for write
> while merge_backward complete unlinks and destroys the image. An
> inflight ref held over the walk would also do, but every inflight ref
> switch would then wait for the whole scan, including the one starting a
> backward merge with the top image paused, which stalls guest I/O for as
> long as the scan runs.
> 
> merge_backward complete now waits for a running seek while it holds
> ctl_mutex, so target messages and the presuspend of the device queue
> behind it until the scan ends.
> 
> Fixes: 412d5a4f7bf1 ("dm/dm-qcow2: add llseek_hole")
> https://virtuozzo.atlassian.net/browse/VSTOR-153050
> Feature: dm-qcow2: block device over QCOW2 files driver
> Reviewed-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
> Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261104180105.1807820-1-den@openvz.org?part=3


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v3 4/5] dm-qcow2: fix SEEK_HOLE/SEEK_DATA hanging after a failed metadata read

[no-reply]

> From: Denis V. Lunev <den@openvz.org>
> 
> qcow2_llseek_hole_qio() parks its seek qio on an md page that is not
> loaded yet and sleeps until the qio is dispatched to QLIST_SEEK, where
> process_seek_qios() completes it. When the page read fails,
> do_md_page_read_complete() ends the waiting qios instead. A seek qio
> has no endio_cb, so nothing wakes the walk, and the lseek() caller
> sleeps uninterruptibly forever while it holds seek_rwsem: the next
> merge_backward complete and every target message after it hang under
> ctl_mutex as well.
> 
> Complete the seek from an endio_cb and return the error the qio was
> ended with.
> 
> Fixes: 412d5a4f7bf1 ("dm/dm-qcow2: add llseek_hole")
> Reported-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> https://virtuozzo.atlassian.net/browse/VSTOR-153422
> Feature: dm-qcow2: block device over QCOW2 files driver
> Signed-off-by: Denis V. Lunev <den@openvz.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261104180105.1807820-1-den@openvz.org?part=4


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v3 5/5] dm-qcow2: fix a refcount table relocation failing on a valid image

[no-reply]

> From: Denis V. Lunev <den@openvz.org>
> 
> relocate_refcount_table() puts the new reftable at pos, the start of
> the first refcount group the old table does not cover, and the new
> refblocks right after it. It then gives each new refblock to the group
> the refblock itself sits in. When the new reftable fills the group of
> pos by itself, the refblocks all land in the next group, and the group
> of pos gets none. Marking the reftable clusters used then fails with a
> WARN, and every allocating write past the old refcount coverage fails
> with EIO.
> 
> A valid image gets there: with 4K clusters and refcount_bits=64 a
> refcount group is 512 clusters, and the reftable reaches 512 clusters
> when the image file grows to about 511G; with refcount_bits=32, at
> about 2T.
> 
> Give the new refblocks to the groups the new reftable and refblocks
> span, one each, in order. pos is group-aligned, and r2_clus is computed
> for the refblocks to count both themselves and the new table, so there
> is a refblock for each of these groups.
> 
> Fixes: 222b6f1291c1 ("dm-qcow2: Introduce driver to create block devices over QCOW2 files")
> Reported-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> https://virtuozzo.atlassian.net/browse/VSTOR-153421
> Feature: dm-qcow2: block device over QCOW2 files driver
> Signed-off-by: Denis V. Lunev <den@openvz.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261104180105.1807820-1-den@openvz.org?part=5


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel