* [Devel] [PATCH hci-8.1 v11 01/10] dm-qcow2: add qcow2_md_page_read_sync() helper
2026-11-09 14:01 [Devel] [PATCH hci-8.1 v11 00/10] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
@ 2026-11-09 14:01 ` Alexander Ivanov
2026-11-09 14:06 ` no-reply
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 02/10] dm-qcow2: drop duplicate prefix from QC_ERR/QC_INFO messages Alexander Ivanov
` (10 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Alexander Ivanov @ 2026-11-09 14:01 UTC (permalink / raw)
To: andrey.zhadchenko; +Cc: devel
Add a helper, which returns an up to date md page from md cache,
or allocates a new one, inserts it and reads it synchronously.
On read failure the page is erased from cache and freed.
Use it in qcow2_parse_header() instead of open-coded allocation
and read of page 0. It will also be used by a later patch in the series.
https://virtuozzo.atlassian.net/browse/VSTOR-148171
Feature: dm-qcow2: block device over QCOW2 files driver
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
drivers/md/dm-qcow2-target.c | 48 ++++++++++++++++++++++++++++++++----
drivers/md/dm-qcow2.h | 2 ++
2 files changed, 45 insertions(+), 5 deletions(-)
diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
index 7c572bd66e1c..0cbbd8e4f493 100644
--- a/drivers/md/dm-qcow2-target.c
+++ b/drivers/md/dm-qcow2-target.c
@@ -418,6 +418,48 @@ int qcow2_alloc_and_insert_md_page(struct qcow2 *qcow2, u64 index, struct md_pag
}
ALLOW_ERROR_INJECTION(qcow2_alloc_and_insert_md_page, ERRNO);
+/*
+ * Returns up to date md page from cache, or allocates
+ * a new one and reads it synchronously. Sets *md to NULL on error.
+ *
+ * The caller must ensure nobody else looks this page up until this
+ * returns (e.g., the device is suspended). A lookup before MD_UPTODATE
+ * is set queues its qio on md->wait_list, which is never woken here,
+ * and the page is freed with the qio still queued on read error.
+ * A cached page that is not MD_UPTODATE yet makes the insert fail.
+ */
+int qcow2_md_page_read_sync(struct qcow2 *qcow2, u64 id, struct md_page **md)
+{
+ int ret;
+
+ spin_lock_irq(&qcow2->md_pages_lock);
+ *md = qcow2_md_page_find_or_postpone(qcow2, id, NULL);
+ spin_unlock_irq(&qcow2->md_pages_lock);
+ if (*md)
+ return 0;
+
+ ret = qcow2_alloc_and_insert_md_page(qcow2, id, md);
+ if (ret)
+ goto err;
+
+ ret = qcow2_rw_page_sync(READ, qcow2, id, (*md)->page);
+ if (ret) {
+ spin_lock_irq(&qcow2->md_pages_lock);
+ qcow2_md_page_erase(qcow2, *md);
+ spin_unlock_irq(&qcow2->md_pages_lock);
+ qcow2_free_md_page(*md);
+ goto err;
+ }
+
+ spin_lock_irq(&qcow2->md_pages_lock);
+ (*md)->status |= MD_UPTODATE;
+ spin_unlock_irq(&qcow2->md_pages_lock);
+ return 0;
+err:
+ *md = NULL;
+ return ret;
+}
+
static void inflight_ref_exit0(struct percpu_ref *ref)
{
struct qcow2_target *tgt = container_of(ref, struct qcow2_target,
@@ -851,13 +893,9 @@ static int qcow2_parse_header(struct dm_target *ti, struct qcow2 *qcow2,
}
qcow2->file_preallocated_area_start = qcow2->file_size;
- ret = qcow2_alloc_and_insert_md_page(qcow2, 0, &md);
- if (ret)
- return ret;
- ret = qcow2_rw_page_sync(READ, qcow2, md->id, md->page);
+ ret = qcow2_md_page_read_sync(qcow2, 0, &md);
if (ret)
return ret;
- md->status |= MD_UPTODATE;
raw_hdr = kmap(md->page);
min_len = to_bytes(ti->len);
diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
index ad5286b16883..5abc7f5ad239 100644
--- a/drivers/md/dm-qcow2.h
+++ b/drivers/md/dm-qcow2.h
@@ -388,6 +388,8 @@ void do_qcow2_work(struct work_struct *ws);
void do_qcow2_fsync_work(struct work_struct *ws);
int qcow2_alloc_and_insert_md_page(struct qcow2 *qcow2, u64 index,
struct md_page **md);
+int qcow2_md_page_read_sync(struct qcow2 *qcow2, u64 id,
+ struct md_page **md);
struct md_page *qcow2_md_page_find_or_postpone(struct qcow2 *qcow2,
u64 id, struct qio **qio);
struct md_page *qcow2_md_page_renumber(struct qcow2 *qcow2, u64 id,
--
2.53.0
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread* [Devel] [PATCH hci-8.1 v11 02/10] dm-qcow2: drop duplicate prefix from QC_ERR/QC_INFO messages
2026-11-09 14:01 [Devel] [PATCH hci-8.1 v11 00/10] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 01/10] dm-qcow2: add qcow2_md_page_read_sync() helper Alexander Ivanov
@ 2026-11-09 14:01 ` Alexander Ivanov
2026-11-09 14:05 ` no-reply
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 03/10] dm-qcow2: extend the file for a new refblock in allocate_cluster() Alexander Ivanov
` (9 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Alexander Ivanov @ 2026-11-09 14:01 UTC (permalink / raw)
To: andrey.zhadchenko; +Cc: devel
QCOW2_FMT already adds the "dm-qcow2: <dev>: " prefix and the trailing
newline, so several messages were printed like:
dm-qcow2: <dev>: dm-qcow2: Can't parse metadata
and some of them were followed by an empty line. Drop the redundant
prefix and newlines.
https://virtuozzo.atlassian.net/browse/VSTOR-148171
Feature: dm-qcow2: block device over QCOW2 files driver
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
drivers/md/dm-qcow2-target.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
index 0cbbd8e4f493..bb322254cea9 100644
--- a/drivers/md/dm-qcow2-target.c
+++ b/drivers/md/dm-qcow2-target.c
@@ -192,7 +192,7 @@ void qcow2_flush_deferred_activity(struct qcow2_target *tgt, struct qcow2 *qcow2
md = rb_entry(node, struct md_page, node);
/* FIXME: call md_make_dirty() and try once again? */
if (md->status & MD_WRITEBACK_ERROR) {
- QC_ERR(tgt->ti, "qcow2: Failed to write dirty pages");
+ QC_ERR(tgt->ti, "Failed to write dirty pages");
tgt->md_writeback_error = true;
break;
}
@@ -840,19 +840,19 @@ static int qcow2_parse_extensions(struct dm_target *ti, struct qcow2 *qcow2,
struct Qcow2BitmapHeaderExt bitmaps_ext;
if (ext.len != sizeof(bitmaps_ext)) {
- QC_ERR(ti, "unexpected bitmap extension size\n");
+ QC_ERR(ti, "unexpected bitmap extension size");
return -EINVAL;
}
if (offset + sizeof(bitmaps_ext) > end) {
- QC_ERR(ti, "unexpected end of extension area\n");
+ QC_ERR(ti, "unexpected end of extension area");
return -EINVAL;
}
memcpy(&bitmaps_ext, addr + offset, sizeof(bitmaps_ext));
ret = qcow2_parse_bitmap_ext(ti, qcow2, &bitmaps_ext);
if (ret) {
- QC_ERR(ti, "unable to pase bitmap extension\n");
+ QC_ERR(ti, "unable to parse bitmap extension");
return ret;
}
break;
@@ -886,7 +886,7 @@ static int qcow2_parse_header(struct dm_target *ti, struct qcow2 *qcow2,
new_size = PAGE_ALIGN(qcow2->file_size);
ret = qcow2_truncate_safe(file, new_size);
if (ret) {
- QC_ERR(ti, "qcow2: Can't truncate file");
+ QC_ERR(ti, "Can't truncate file");
return ret;
} /* See md_page_read_complete() */
qcow2->file_size = new_size;
@@ -918,14 +918,14 @@ static int qcow2_parse_header(struct dm_target *ti, struct qcow2 *qcow2,
ret = -EXDEV;
if (upper) {
if (upper->clu_size != qcow2->clu_size) {
- QC_ERR(ti, "dm-qcow2: clu_size mismatch\n");
+ QC_ERR(ti, "clu_size mismatch");
goto out;
}
if (upper->ext_l2 != qcow2->ext_l2) {
if (is_bottom && upper->ext_l2 && !qcow2->ext_l2) {
- QC_INFO(ti, "dm-qcow2: using bottom delta without ext_l2\n");
+ QC_INFO(ti, "using bottom delta without ext_l2");
} else {
- QC_ERR(ti, "dm-qcow2: upper delta without ext_l2 is unsupported\n");
+ QC_ERR(ti, "upper delta without ext_l2 is unsupported");
goto out; /* This is not supported yet */
}
}
@@ -947,11 +947,11 @@ static int qcow2_parse_header(struct dm_target *ti, struct qcow2 *qcow2,
ext_end = PAGE_SIZE;
if (ext_end > PAGE_SIZE) {
- QC_INFO(ti, "extensions are too long, skipping them\n");
+ QC_INFO(ti, "extensions are too long, skipping them");
} else {
ret = qcow2_parse_extensions(ti, qcow2, ext_start, ext_end, raw_hdr);
if (ret) {
- QC_ERR(ti, "failed to parse extensions\n");
+ QC_ERR(ti, "failed to parse extensions");
goto out;
}
}
@@ -986,7 +986,7 @@ static int qcow2_parse_metadata(struct dm_target *ti, struct qcow2_target *tgt)
ret = 0;
out:
if (ret)
- QC_ERR(ti, "dm-qcow2: Can't parse metadata");
+ QC_ERR(ti, "Can't parse metadata");
return ret;
}
@@ -1067,7 +1067,7 @@ static void qcow2_truncate_preallocations(struct dm_target *ti)
ret = qcow2_truncate_safe(qcow2->file, end);
if (ret) {
- QC_ERR(ti, "dm-qcow2: Can't truncate preallocations");
+ QC_ERR(ti, "Can't truncate preallocations");
tgt->truncate_error = true;
return;
}
@@ -1153,7 +1153,7 @@ static void qcow2_postsuspend(struct dm_target *ti)
if (dm_table_get_mode(ti->table) & FMODE_WRITE) {
ret = qcow2_set_image_file_features(qcow2, false);
if (ret)
- QC_ERR(ti, "qcow2: Can't set features");
+ QC_ERR(ti, "Can't set features");
}
}
static int qcow2_preresume(struct dm_target *ti)
@@ -1162,7 +1162,7 @@ static int qcow2_preresume(struct dm_target *ti)
int ret = 0;
if (qcow2_wants_check(tgt)) {
- QC_ERR(ti, "qcow2: image check and target reload are required");
+ QC_ERR(ti, "image check and target reload are required");
return -EIO;
}
@@ -1183,7 +1183,7 @@ static int qcow2_preresume(struct dm_target *ti)
if (dm_table_get_mode(ti->table) & FMODE_WRITE) {
ret = qcow2_set_image_file_features(tgt->top, true);
if (ret)
- QC_ERR(ti, "qcow2: Can't set features");
+ QC_ERR(ti, "Can't set features");
}
if (!ret)
qcow2_set_wants_suspend(ti, false);
--
2.53.0
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread* [Devel] [PATCH hci-8.1 v11 03/10] dm-qcow2: extend the file for a new refblock in allocate_cluster()
2026-11-09 14:01 [Devel] [PATCH hci-8.1 v11 00/10] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 01/10] dm-qcow2: add qcow2_md_page_read_sync() helper Alexander Ivanov
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 02/10] dm-qcow2: drop duplicate prefix from QC_ERR/QC_INFO messages Alexander Ivanov
@ 2026-11-09 14:01 ` Alexander Ivanov
2026-11-09 14:06 ` no-reply
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 04/10] dm-qcow2: complete wbd before ending the data qio Alexander Ivanov
` (8 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Alexander Ivanov @ 2026-11-09 14:01 UTC (permalink / raw)
To: andrey.zhadchenko; +Cc: devel
place_r2() links a new refblock into R1, but moves neither file_size
nor file_preallocated_area_start. The refblock stays at or above the
watermark if truncate_prealloc_safe() in allocate_cluster() fails
after place_r2(), or if a parallel mark_cluster_unused() lowers
free_cluster_search_pos before the second pass of
find_unused_cluster(). Truncation of preallocations on suspend then
cuts the file under the refblock: R1 on disk points past EOF, and
the next session gets -EIO on allocations in the range it covers.
Make find_unused_cluster() report a position not covered by R2
instead of placing the refblock there itself. allocate_cluster()
prepares that cluster the same way as a data cluster, with the punch,
extend and watermark update moved into prepare_unused_cluster(), and
calls place_r2() before searching again.
https://virtuozzo.atlassian.net/browse/VSTOR-148171
Feature: dm-qcow2: block device over QCOW2 files driver
Fixes: 222b6f1291c1c ("dm-qcow2: Introduce driver to create block devices over QCOW2 files")
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
drivers/md/dm-qcow2-map.c | 88 ++++++++++++++++++++++-----------------
1 file changed, 50 insertions(+), 38 deletions(-)
diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 34d9d9810f60..e8a41954c4c6 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -2083,21 +2083,12 @@ static int parse_metadata(struct qcow2 *qcow2, struct qio **qio,
* and connects it to R1 table entry.
*/
static int place_r2(struct qcow2 *qcow2, struct qcow2_map_item *r1,
- struct qcow2_map_item *r2, loff_t r2_pos, struct qio **qio)
+ struct qcow2_map_item *r2, loff_t r2_pos)
{
struct dm_target *ti = qcow2->tgt->ti;
u64 page_id = r2_pos >> PAGE_SHIFT;
int ret;
- if (delay_if_writeback(qcow2, r1->md, r1->index_in_page, qio, true))
- return 0;
-
- ret = qcow2_punch_hole(qcow2->file, r2_pos, qcow2->clu_size);
- if (ret) {
- QC_ERR(ti, "punch hole: %d", ret);
- return ret;
- }
-
ret = qcow2_alloc_and_insert_md_page(qcow2, page_id, &r2->md);
if (ret < 0) {
QC_ERR(ti, "Can't alloc: ret=%d, page_id=%llu", ret, page_id);
@@ -2113,7 +2104,7 @@ static int place_r2(struct qcow2 *qcow2, struct qcow2_map_item *r1,
spin_unlock_irq(&qcow2->md_pages_lock);
mark_cluster_used(qcow2, r2->md, r2->index_in_page);
- return 1;
+ return 0;
}
static s32 find_unused_block_entry(struct qcow2 *qcow2, struct md_page *md,
@@ -2135,11 +2126,13 @@ static s32 find_unused_block_entry(struct qcow2 *qcow2, struct md_page *md,
static loff_t find_unused_cluster(struct qcow2 *qcow2, struct qio **qio,
struct qcow2_map_item *r1,
- struct qcow2_map_item *r2)
+ struct qcow2_map_item *r2, bool *needs_r2)
{
u32 clu_size = qcow2->clu_size;
s32 index, ret;
loff_t pos;
+
+ *needs_r2 = false;
again:
pos = READ_ONCE(qcow2->free_cluster_search_pos);
if (pos >= qcow2->reftable_max_file_size)
@@ -2151,15 +2144,15 @@ static loff_t find_unused_cluster(struct qcow2 *qcow2, struct qio **qio,
return ret;
/*
* Since pos is not covered by R2, the whole cluster
- * must be unused. Use it to store R2 cluster.
+ * must be unused. The caller stores R2 cluster there.
* Both indexes must be 0 here, because of we allocate
* clusters from small to big.
*/
WARN_ON_ONCE(r2->index_in_page != 0 || r2->index != 0);
- ret = place_r2(qcow2, r1, r2, pos, qio);
- if (ret <= 0)
- return ret;
- goto again;
+ if (delay_if_writeback(qcow2, r1->md, r1->index_in_page, qio, true))
+ return 0;
+ *needs_r2 = true;
+ return pos;
}
spin_lock_irq(&qcow2->md_pages_lock);
@@ -2436,6 +2429,37 @@ static int relocate_refcount_table(struct qcow2 *qcow2, struct qio **qio)
return -EIO;
}
+/*
+ * Zero the unused cluster at @pos and make the file size and
+ * file_preallocated_area_start cover it.
+ */
+static int prepare_unused_cluster(struct qcow2 *qcow2, loff_t pos)
+{
+ loff_t off, end = pos + qcow2->clu_size;
+ loff_t old_size = qcow2->file_size;
+ int ret;
+
+ if (pos < qcow2->file_preallocated_area_start) {
+ /* Clu at @pos may contain dirty data */
+ off = min_t(loff_t, old_size, end);
+ ret = qcow2_punch_hole(qcow2->file, pos, off - pos);
+ if (ret) {
+ QC_ERR(qcow2->tgt->ti, "punch hole: %d", ret);
+ return ret;
+ }
+ }
+
+ if (end > old_size) {
+ ret = truncate_prealloc_safe(qcow2, end, __func__);
+ if (ret)
+ return ret;
+ }
+
+ if (end > qcow2->file_preallocated_area_start)
+ qcow2->file_preallocated_area_start = end;
+ return 0;
+}
+
/*
* This function is aimed to be called only from main work.
* In case of wish to use it from more places, it's needed
@@ -2445,13 +2469,12 @@ static int relocate_refcount_table(struct qcow2 *qcow2, struct qio **qio)
static loff_t allocate_cluster(struct qcow2 *qcow2, struct qio *qio,
struct md_page **r2_md, u32 *r2_index_in_page)
{
- u32 clu_size = qcow2->clu_size;
- struct file *file = qcow2->file;
- loff_t pos, off, end, old_size;
struct qcow2_map_item r1, r2;
+ bool needs_r2;
+ loff_t pos;
int ret;
again:
- pos = find_unused_cluster(qcow2, &qio, &r1, &r2);
+ pos = find_unused_cluster(qcow2, &qio, &r1, &r2, &needs_r2);
if (unlikely(pos == -ENOENT)) {
ret = relocate_refcount_table(qcow2, &qio);
if (ret <= 0)
@@ -2461,28 +2484,17 @@ static loff_t allocate_cluster(struct qcow2 *qcow2, struct qio *qio,
if (pos <= 0)
return pos;
- end = pos + clu_size;
- old_size = qcow2->file_size;
-
- if (pos < qcow2->file_preallocated_area_start) {
- /* Clu at @pos may contain dirty data */
- off = min_t(loff_t, old_size, end);
- ret = qcow2_punch_hole(file, pos, off - pos);
- if (ret) {
- QC_ERR(qcow2->tgt->ti, "punch hole: %d", ret);
- return ret;
- }
- }
+ ret = prepare_unused_cluster(qcow2, pos);
+ if (ret)
+ return ret;
- if (end > old_size) {
- ret = truncate_prealloc_safe(qcow2, end, __func__);
+ if (needs_r2) {
+ ret = place_r2(qcow2, &r1, &r2, pos);
if (ret)
return ret;
+ goto again;
}
- if (end > qcow2->file_preallocated_area_start)
- qcow2->file_preallocated_area_start = end;
-
mark_cluster_used(qcow2, r2.md, r2.index_in_page);
if (r2_md)
*r2_md = r2.md;
--
2.53.0
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread* [Devel] [PATCH hci-8.1 v11 04/10] dm-qcow2: complete wbd before ending the data qio
2026-11-09 14:01 [Devel] [PATCH hci-8.1 v11 00/10] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
` (2 preceding siblings ...)
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 03/10] dm-qcow2: extend the file for a new refblock in allocate_cluster() Alexander Ivanov
@ 2026-11-09 14:01 ` Alexander Ivanov
2026-11-09 14:09 ` no-reply
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 05/10] dm-qcow2: flush lower md before clearing dirty bit on merge failure Alexander Ivanov
` (7 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Alexander Ivanov @ 2026-11-09 14:01 UTC (permalink / raw)
To: andrey.zhadchenko; +Cc: devel
data_rw_complete() calls qio_endio() before complete_wbd(). qio_endio()
drops the inflight ref, so qcow2_flush_deferred_activity() may return
on suspend while complete_wbd() for a failed L1/L2 writeback is still
running in AIO completion context. revert_l_entries_update() then
marks clusters unused and dirties an R2 page after the flush.
That page sits on slow_wb_batch_list with the slow writeback timer
armed. If the timer fires first, the page is written while the device
is suspended, after postsuspend has cleared the dirty bit. If
preresume comes first, qcow2_free_md_pages_all() frees the page
without taking it off the list, and the timer later walks freed
memory.
Complete the wbd first, so the R2 update is in place before the
inflight ref goes away and the suspend flush writes it back. The md
writeback path already does it in this order.
https://virtuozzo.atlassian.net/browse/VSTOR-148171
Feature: dm-qcow2: block device over QCOW2 files driver
Fixes: 222b6f1291c1c ("dm-qcow2: Introduce driver to create block devices over QCOW2 files")
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
drivers/md/dm-qcow2-map.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index e8a41954c4c6..7866c791325b 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -2892,13 +2892,14 @@ static void data_rw_complete(struct qio *qio)
spin_unlock_irqrestore(&qcow2->md_pages_lock, flags);
}
+ /* qio_endio() drops the inflight ref, which suspend waits for */
+ if (finalize_wbd)
+ complete_wbd(qcow2, wbd);
if (call_endio) {
if (bi_status != BLK_STS_OK)
qio->bi_status = bi_status;
qio_endio(qio);
}
- if (finalize_wbd)
- complete_wbd(qcow2, wbd);
}
static void perform_rw_mapped(struct qcow2_map *map, struct qio *qio)
--
2.53.0
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread* [Devel] [PATCH hci-8.1 v11 05/10] dm-qcow2: flush lower md before clearing dirty bit on merge failure
2026-11-09 14:01 [Devel] [PATCH hci-8.1 v11 00/10] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
` (3 preceding siblings ...)
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 04/10] dm-qcow2: complete wbd before ending the data qio Alexander Ivanov
@ 2026-11-09 14:01 ` Alexander Ivanov
2026-11-09 14:08 ` no-reply
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 06/10] dm-qcow2: keep the dirty bit of an image that needs check Alexander Ivanov
` (6 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Alexander Ivanov @ 2026-11-09 14:01 UTC (permalink / raw)
To: andrey.zhadchenko; +Cc: devel
When qcow2_merge_common() fails, qcow2_merge_backward_work() clears the
dirty bit of the lower image right away. qcow2_service_iter() waits only
for the service qios, so delayed md writeback of the lower image may
still be pending. With kernel_sets_dirty_bit, a crash at that point
leaves the lower image marked clean on disk with refblocks that lag
behind the merged data.
Flush the deferred activity of the lower image before writing its
header, as qcow2_merge_backward_complete() does.
https://virtuozzo.atlassian.net/browse/VSTOR-148171
Feature: dm-qcow2: block device over QCOW2 files driver
Fixes: 222b6f1291c1c ("dm-qcow2: Introduce driver to create block devices over QCOW2 files")
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
drivers/md/dm-qcow2-cmd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/md/dm-qcow2-cmd.c b/drivers/md/dm-qcow2-cmd.c
index 45c517ce0ef8..3bccce090f01 100644
--- a/drivers/md/dm-qcow2-cmd.c
+++ b/drivers/md/dm-qcow2-cmd.c
@@ -330,6 +330,7 @@ void qcow2_merge_backward_work(struct work_struct *work)
ret = qcow2_merge_common(tgt, qcow2);
if (ret) {
set_backward_merge_in_process(tgt, qcow2, false);
+ qcow2_flush_deferred_activity(tgt, lower);
ret2 = qcow2_set_image_file_features(lower, false);
if (ret2 < 0)
QC_ERR(tgt->ti, "Can't unuse lower (%d)", ret2);
--
2.53.0
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread* [Devel] [PATCH hci-8.1 v11 06/10] dm-qcow2: keep the dirty bit of an image that needs check
2026-11-09 14:01 [Devel] [PATCH hci-8.1 v11 00/10] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
` (4 preceding siblings ...)
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 05/10] dm-qcow2: flush lower md before clearing dirty bit on merge failure Alexander Ivanov
@ 2026-11-09 14:01 ` Alexander Ivanov
2026-11-09 14:11 ` no-reply
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 07/10] dm-qcow2: flag the image for check if reftable relocation fails late Alexander Ivanov
` (5 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Alexander Ivanov @ 2026-11-09 14:01 UTC (permalink / raw)
To: andrey.zhadchenko; +Cc: devel
The dm-qcow2 module parameter kernel_sets_dirty_bit makes the kernel set
the dirty bit on resume and clear it on postsuspend. It is off by
default and Virtuozzo does not set it: ploop manages the dirty bit from
userspace itself. So this is a hardening patch for the non-default mode.
With kernel_sets_dirty_bit, postsuspend clears the dirty bit
unconditionally. An image that is dirty at open, e.g. after a crash,
has unrepaired refcounts, which may be below real usage, yet it is
marked clean on disk after the first suspend. A userspace check before
the next open then has no reason to repair it. The same happens after
an error that sets wants_check, e.g. md_writeback_error: it is kept only
in the target, so resume is refused, but after a table reload or a host
restart the image is reopened with a clean header.
Clear the bit only if the image was clean before the kernel set it and
wants_check is not set. The parsed header can't tell the former: it is
reread on every preresume and may carry the kernel's own bit, e.g. on
the lower image of a backward merge cancelled by suspend in
waiting_completion or of a merge with depth > 0. So track two flags per
image: kernel_dirty_bit_set is set only while the last header write was
a successful set and the reread header still has the bit, opened_dirty
is set when the header has the bit and kernel_dirty_bit_set is not set.
Clearing the bit of an image opened dirty is left to userspace check.
After a failed write the bit found on disk is taken as opened dirty,
which costs an extra check, rather than as the kernel's, which could
clear a bit set by userspace while the device was suspended.
https://virtuozzo.atlassian.net/browse/VSTOR-148171
Feature: dm-qcow2: block device over QCOW2 files driver
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
drivers/md/dm-qcow2-target.c | 14 ++++++++++++--
drivers/md/dm-qcow2.h | 2 ++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
index bb322254cea9..6824d95193ae 100644
--- a/drivers/md/dm-qcow2-target.c
+++ b/drivers/md/dm-qcow2-target.c
@@ -666,6 +666,7 @@ int qcow2_set_image_file_features(struct qcow2 *qcow2, bool dirty)
u64 dirty_mask = cpu_to_be64(INCOMPATIBLE_FEATURES_DIRTY_BIT);
struct QCowHeader *raw_hdr;
struct md_page *md;
+ int ret;
if (qcow2->hdr.version == 2)
return 0;
@@ -682,12 +683,15 @@ int qcow2_set_image_file_features(struct qcow2 *qcow2, bool dirty)
if (kernel_sets_dirty_bit) {
if (dirty)
raw_hdr->incompatible_features |= dirty_mask;
- else
+ else if (!qcow2->opened_dirty && !qcow2_wants_check(qcow2->tgt))
raw_hdr->incompatible_features &= ~dirty_mask;
}
kunmap(md->page);
- return qcow2_rw_page_sync(WRITE, qcow2, md->id, md->page);
+ ret = qcow2_rw_page_sync(WRITE, qcow2, md->id, md->page);
+ if (kernel_sets_dirty_bit)
+ qcow2->kernel_dirty_bit_set = dirty && !ret;
+ return ret;
}
ALLOW_ERROR_INJECTION(qcow2_set_image_file_features, ERRNO);
@@ -911,6 +915,12 @@ static int qcow2_parse_header(struct dm_target *ti, struct qcow2 *qcow2,
goto out;
qcow2_calc_cached_parameters(qcow2, hdr);
+ if (!(hdr->incompatible_features & INCOMPATIBLE_FEATURES_DIRTY_BIT)) {
+ qcow2->opened_dirty = false;
+ qcow2->kernel_dirty_bit_set = false;
+ } else if (!qcow2->kernel_dirty_bit_set) {
+ qcow2->opened_dirty = true;
+ }
ret = -EOPNOTSUPP;
if (qcow2->clu_size < PAGE_SIZE ||
(qcow2->ext_l2 && qcow2->clu_size < PAGE_SIZE * 32))
diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
index 5abc7f5ad239..2a6203c86e3a 100644
--- a/drivers/md/dm-qcow2.h
+++ b/drivers/md/dm-qcow2.h
@@ -259,6 +259,8 @@ struct qcow2 {
u32 refblock_bits;
u32 refblock_entries;
bool ext_l2;
+ bool opened_dirty;
+ bool kernel_dirty_bit_set;
bool pause_submitting_qios; /* This is used only on top qcow2 image */
bool backward_merge_in_process;
--
2.53.0
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Devel] [PATCH hci-8.1 v11 06/10] dm-qcow2: keep the dirty bit of an image that needs check
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 06/10] dm-qcow2: keep the dirty bit of an image that needs check Alexander Ivanov
@ 2026-11-09 14:11 ` no-reply
0 siblings, 0 replies; 30+ messages in thread
From: no-reply @ 2026-11-09 14:11 UTC (permalink / raw)
To: Alexander Ivanov, andrey.zhadchenko; +Cc: devel
> The dm-qcow2 module parameter kernel_sets_dirty_bit makes the kernel set
> the dirty bit on resume and clear it on postsuspend. It is off by
> default and Virtuozzo does not set it: ploop manages the dirty bit from
> userspace itself. So this is a hardening patch for the non-default mode.
>
> With kernel_sets_dirty_bit, postsuspend clears the dirty bit
> unconditionally. An image that is dirty at open, e.g. after a crash,
> has unrepaired refcounts, which may be below real usage, yet it is
> marked clean on disk after the first suspend. A userspace check before
> the next open then has no reason to repair it. The same happens after
> an error that sets wants_check, e.g. md_writeback_error: it is kept only
> in the target, so resume is refused, but after a table reload or a host
> restart the image is reopened with a clean header.
>
> Clear the bit only if the image was clean before the kernel set it and
> [ ... ]
>
> https://virtuozzo.atlassian.net/browse/VSTOR-148171
> Feature: dm-qcow2: block device over QCOW2 files driver
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261109140143.1408038-2-alexander.ivanov@virtuozzo.com?part=6
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Devel] [PATCH hci-8.1 v11 07/10] dm-qcow2: flag the image for check if reftable relocation fails late
2026-11-09 14:01 [Devel] [PATCH hci-8.1 v11 00/10] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
` (5 preceding siblings ...)
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 06/10] dm-qcow2: keep the dirty bit of an image that needs check Alexander Ivanov
@ 2026-11-09 14:01 ` Alexander Ivanov
2026-11-09 14:11 ` no-reply
2026-11-09 15:42 ` Konstantin Khorenko
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 08/10] dm-qcow2: leak clusters allocated for a failed L1/L2 writeback Alexander Ivanov
` (4 subsequent siblings)
11 siblings, 2 replies; 30+ messages in thread
From: Alexander Ivanov @ 2026-11-09 14:01 UTC (permalink / raw)
To: andrey.zhadchenko; +Cc: devel
Once relocate_refcount_table() has issued the header write pointing to
the new reftable, any failure returns -EIO. The header on disk may
already point to the new reftable even if its own write fails: a short
write is reported as -ENODATA, and the old reftable position is
restored only in the cached header page, which a v2 image never
rewrites on suspend. The new reftable on disk may then be unfilled or
miss the new refblocks, while refcounts of the new and old reftable
clusters may not be updated. Refcounts no longer match the header, but
nothing reports that, so the device can be resumed and the image
reopened as if it were consistent.
Set md_writeback_error on these paths, so wants_check reports the image
and resume is refused until it is checked, and log the failure.
https://virtuozzo.atlassian.net/browse/VSTOR-148171
Feature: dm-qcow2: block device over QCOW2 files driver
Fixes: 222b6f1291c1c ("dm-qcow2: Introduce driver to create block devices over QCOW2 files")
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
drivers/md/dm-qcow2-map.c | 44 ++++++++++++++++++++++++++-------------
1 file changed, 29 insertions(+), 15 deletions(-)
diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 7866c791325b..995f611ae5bb 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -2247,7 +2247,7 @@ static void set_reftable_in_raw_hdr(struct page *page0, loff_t pos, loff_t clus)
* complicated, we should not demonstrate excessive creativity
* and optimize it in prejudice of readability.
*
- * We act in the way to provide safe rollback throughout whole function.
+ * We act in the way to provide safe rollback until new hdr is written.
* Firstly, we cache every related md we're going to use on relocation.
* New reftable (R1) is placed next to max cluster covered by old R1.
* Since new reftable (R1) clusters should be marked as used after
@@ -2256,8 +2256,10 @@ static void set_reftable_in_raw_hdr(struct page *page0, loff_t pos, loff_t clus)
* clusters are in the part of file, which is not covered by old R1.
*
* Then, we try to write new hdr on disk. In case of failure, we
- * restore old hdr in memory and do safe rollback. In case of success,
- * there should not be more reasons to fail (only this driver's bugs).
+ * restore old hdr in memory, but the write may have reached the disk,
+ * so the image is flagged for check (md_writeback_error). In case of
+ * success, there should not be more reasons to fail (only this driver's
+ * bugs); such a failure flags the image for check too.
* Cached old reftable (R1) pages we renumber to point to new reftable (R1)
* place. Then we mark old reftable (R1) clusters as unused, while
* new reftable (R1) clusters as used.
@@ -2351,6 +2353,10 @@ static int relocate_refcount_table(struct qcow2 *qcow2, struct qio **qio)
if (ret) {
/* Restore old R1 */
set_reftable_in_raw_hdr(md0->page, old_pos, old_clus);
+ QC_ERR(qcow2->tgt->ti,
+ "failed to write header with relocated refcount table: %d",
+ ret);
+ qcow2->tgt->md_writeback_error = true;
goto err_free_r2_pages;
}
@@ -2372,24 +2378,27 @@ 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_eio */
+ break;
}
spin_unlock_irq(&qcow2->md_pages_lock);
- if (i != nr_pages)
- goto err_eio;
+ if (i != nr_pages) {
+ ret = -ENOENT;
+ goto err_hdr_written;
+ }
/* 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;
+ if (WARN_ON_ONCE(r2_pos >= r2_end)) {
+ ret = -ERANGE;
+ goto err_hdr_written;
}
+ ret = calc_refcounters_map(qcow2, i, &r1, &r2);
+ if (WARN_ON_ONCE(ret < 0))
+ goto err_hdr_written;
ret = handle_md_page(qcow2, r1.page_id, NULL, &md);
if (WARN_ON_ONCE(ret <= 0))
- goto err_eio;
+ goto err_hdr_written;
spin_lock_irq(&qcow2->md_pages_lock);
set_u64_to_be_page(md->page, r1.index_in_page, r2_pos);
qcow2_md_make_dirty(qcow2, md, true);
@@ -2400,7 +2409,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_eio;
+ goto err_hdr_written;
mark_cluster_used(qcow2, r2.md, r2.index_in_page);
}
@@ -2408,12 +2417,18 @@ 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_eio;
+ goto err_hdr_written;
mark_cluster_unused(qcow2, r2.md, r2.index_in_page, i);
}
return 1;
+err_hdr_written:
+ /* 0: md page is not cached */
+ QC_ERR(qcow2->tgt->ti, "failed to fill relocated refcount table: %d",
+ ret ?: -ENOENT);
+ qcow2->tgt->md_writeback_error = true;
+ return -EIO;
err_free_r2_pages:
for (i = new_md_start; i < new_md_end; i += PAGE_SIZE) {
ret = handle_md_page(qcow2, i >> PAGE_SHIFT, NULL, &md);
@@ -2424,7 +2439,6 @@ 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
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Devel] [PATCH hci-8.1 v11 07/10] dm-qcow2: flag the image for check if reftable relocation fails late
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 07/10] dm-qcow2: flag the image for check if reftable relocation fails late Alexander Ivanov
@ 2026-11-09 14:11 ` no-reply
2026-11-09 15:42 ` Konstantin Khorenko
1 sibling, 0 replies; 30+ messages in thread
From: no-reply @ 2026-11-09 14:11 UTC (permalink / raw)
To: andrey.zhadchenko, Alexander Ivanov; +Cc: devel
> Once relocate_refcount_table() has issued the header write pointing to
> the new reftable, any failure returns -EIO. The header on disk may
> already point to the new reftable even if its own write fails: a short
> write is reported as -ENODATA, and the old reftable position is
> restored only in the cached header page, which a v2 image never
> rewrites on suspend. The new reftable on disk may then be unfilled or
> miss the new refblocks, while refcounts of the new and old reftable
> clusters may not be updated. Refcounts no longer match the header, but
> nothing reports that, so the device can be resumed and the image
> reopened as if it were consistent.
>
> Set md_writeback_error on these paths, so wants_check reports the image
> and resume is refused until it is checked, and log the failure.
>
> https://virtuozzo.atlassian.net/browse/VSTOR-148171
> Feature: dm-qcow2: block device over QCOW2 files driver
> Fixes: 222b6f1291c1c ("dm-qcow2: Introduce driver to create block devices over QCOW2 files")
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261109140143.1408038-2-alexander.ivanov@virtuozzo.com?part=7
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Devel] [PATCH hci-8.1 v11 07/10] dm-qcow2: flag the image for check if reftable relocation fails late
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 07/10] dm-qcow2: flag the image for check if reftable relocation fails late Alexander Ivanov
2026-11-09 14:11 ` no-reply
@ 2026-11-09 15:42 ` Konstantin Khorenko
1 sibling, 0 replies; 30+ messages in thread
From: Konstantin Khorenko @ 2026-11-09 15:42 UTC (permalink / raw)
To: Alexander Ivanov; +Cc: devel
No changes needed in this patch.
Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
--
Konstantin Khorenko,
Virtuozzo Linux Kernel Team
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Devel] [PATCH hci-8.1 v11 08/10] dm-qcow2: leak clusters allocated for a failed L1/L2 writeback
2026-11-09 14:01 [Devel] [PATCH hci-8.1 v11 00/10] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
` (6 preceding siblings ...)
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 07/10] dm-qcow2: flag the image for check if reftable relocation fails late Alexander Ivanov
@ 2026-11-09 14:01 ` Alexander Ivanov
2026-11-09 14:14 ` no-reply
2026-11-09 15:42 ` Konstantin Khorenko
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 09/10] dm-qcow2: retry R1/R2 writeback that failed with ENOSPC Alexander Ivanov
` (3 subsequent siblings)
11 siblings, 2 replies; 30+ messages in thread
From: Alexander Ivanov @ 2026-11-09 14:01 UTC (permalink / raw)
To: andrey.zhadchenko; +Cc: devel
A failed L1/L2 writeback reverts the page in memory and frees each
cluster allocated for it: revert_l_entries_update() drops the refcount
of a newly allocated cluster, and a COW qio waiting on the page frees
its cluster through FREE_ALLOCATED_CLU when it ends with the error. The
write may still have partially reached the disk, so L1/L2 on disk may
point to such a cluster. Until the page is rewritten, the allocator may
give the cluster to another offset, and two guest offsets then share
it. If the page is not rewritten before suspend, the image is left with
a reference to a free cluster.
Keep the refcount of these clusters instead. If L1/L2 on disk points to
such a cluster, the image stays consistent; otherwise the cluster is
leaked, which check repairs by freeing it. So the image needs no check
and resume needs no refusal.
https://virtuozzo.atlassian.net/browse/VSTOR-148171
Feature: dm-qcow2: block device over QCOW2 files driver
Fixes: 222b6f1291c1c ("dm-qcow2: Introduce driver to create block devices over QCOW2 files")
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
drivers/md/dm-qcow2-map.c | 51 +++++++-----------------------------
drivers/md/dm-qcow2-target.c | 4 ---
2 files changed, 9 insertions(+), 46 deletions(-)
diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 995f611ae5bb..4ab8051f08c4 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -1331,50 +1331,15 @@ static void do_md_page_read_complete(int ret, struct qcow2 *qcow2,
/* Be careful with dirty_or_writeback()/etc! Check races. */
static void revert_l_entries_update(struct qcow2 *qcow2, struct wb_desc *wbd)
{
- struct qcow2_map_item r1, r2;
- struct page *pe_page;
- bool skip_odd;
- u64 pos, old;
- bool cleared;
- int i, ret;
-
- skip_odd = qcow2->ext_l2 && wbd->lx_level == L2_LEVEL;
+ struct page *pe_page = wbd->pe_page;
+ u64 old;
+ int i;
lockdep_assert_held(&qcow2->md_pages_lock);
for_each_set_bit(i, wbd->changed_indexes, LX_INDEXES_PER_PAGE) {
- pos = get_u64_from_be_page(wbd->md->page, i);
-
- cleared = !(pos & ~(u64)L2_READS_ALL_ZEROES);
-
/* Here we restore prealloced and compressed clu mappings */
- pe_page = wbd->pe_page;
- if (pe_page) { /* Only L2 has this. */
- old = get_u64_from_be_page(pe_page, i);
- if (old != 0) {
- set_u64_to_be_page(wbd->md->page, i, old);
- continue; /* Avoid mark_cluster_unused() */
- }
- }
-
- set_u64_to_be_page(wbd->md->page, i, 0);
- if (skip_odd && (i & 1))
- continue; /* pos contains ext_l2 part of L2 entry */
- if (cleared)
- continue; /* no cluster was allocated */
-
- spin_unlock(&qcow2->md_pages_lock);
- pos &= ~LX_REFCOUNT_EXACTLY_ONE;
-
- /*
- * R1/R2 should be cached, since we was able
- * to submit cluster allocation.
- */
- ret = qcow2_handle_r1r2_maps(qcow2, pos, NULL, &r1, &r2, false);
- if (WARN_ON_ONCE(ret <= 0))
- continue;
-
- mark_cluster_unused(qcow2, r2.md, r2.index_in_page, pos);
- spin_lock(&qcow2->md_pages_lock);
+ old = pe_page ? get_u64_from_be_page(pe_page, i) : 0;
+ set_u64_to_be_page(wbd->md->page, i, old);
}
}
@@ -1408,6 +1373,7 @@ static void complete_wbd(struct qcow2 *qcow2, struct wb_desc *wbd)
LIST_HEAD(wait_list);
LIST_HEAD(end_list);
unsigned long flags;
+ struct qio *qio;
spin_lock_irqsave(&qcow2->md_pages_lock, flags);
revert_l_entries_update(qcow2, wbd);
@@ -1415,6 +1381,8 @@ static void complete_wbd(struct qcow2 *qcow2, struct wb_desc *wbd)
&wait_list, &end_list);
spin_unlock_irqrestore(&qcow2->md_pages_lock, flags);
+ list_for_each_entry(qio, &end_list, link)
+ qio->ext->cleanup_mask &= ~FREE_ALLOCATED_CLU;
qcow2_dispatch_qios(qcow2, NULL, &wait_list);
end_qios(&end_list, errno_to_blk_status(wbd->ret));
}
@@ -1445,8 +1413,7 @@ static void do_md_page_write_complete(int ret, struct qcow2 *qcow2,
/*
* We can finish wb only in case of success.
* Otherwise this is done in finalize_wbd()
- * after data qios stopped use wbd clusters
- * and clusters allocations reverted.
+ * after data qios stopped use wbd clusters.
*/
if (likely(ret == 0)) {
clear_writeback_status(qcow2, md, ret,
diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
index 6824d95193ae..99fb68a5535e 100644
--- a/drivers/md/dm-qcow2-target.c
+++ b/drivers/md/dm-qcow2-target.c
@@ -173,10 +173,6 @@ void qcow2_flush_deferred_activity(struct qcow2_target *tgt, struct qcow2 *qcow2
struct md_page *md;
int i;
- /*
- * We need second iteration, since revert_l_entries_update()
- * may start timer again after failed wb.
- */
for (i = 0; i < 2; i++) {
if (timer_delete_sync_try(&qcow2->slow_wb_timer) != -1)
qcow2_slow_wb_timer_fn(&qcow2->slow_wb_timer);
--
2.53.0
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Devel] [PATCH hci-8.1 v11 08/10] dm-qcow2: leak clusters allocated for a failed L1/L2 writeback
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 08/10] dm-qcow2: leak clusters allocated for a failed L1/L2 writeback Alexander Ivanov
@ 2026-11-09 14:14 ` no-reply
2026-11-09 14:36 ` Alexander Ivanov
2026-11-09 15:42 ` Konstantin Khorenko
1 sibling, 1 reply; 30+ messages in thread
From: no-reply @ 2026-11-09 14:14 UTC (permalink / raw)
To: andrey.zhadchenko, Alexander Ivanov; +Cc: devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dm-qcow2: clusters leaked on failed L1/L2 writeback; L1 alloc retry repeats leak
--
Subject: Re: [PATCH] dm-qcow2: leak clusters allocated for a failed L1/L2 writeback
commit 8f9e1298a69365ad6d96d693fc232f0ee7761431
Author: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
dm-qcow2: leak clusters allocated for a failed L1/L2 writeback
This commit stops dropping the refcount of clusters allocated for a failed
L1/L2 writeback. revert_l_entries_update() now only restores the in-memory
entries, and complete_wbd() clears FREE_ALLOCATED_CLU on the qios ended
with the error.
> diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
> --- a/drivers/md/dm-qcow2-map.c
> +++ b/drivers/md/dm-qcow2-map.c
[ ... ]
> static void revert_l_entries_update(struct qcow2 *qcow2, struct wb_desc *wbd)
> {
> + struct page *pe_page = wbd->pe_page;
> + u64 old;
> + int i;
>
> lockdep_assert_held(&qcow2->md_pages_lock);
> for_each_set_bit(i, wbd->changed_indexes, LX_INDEXES_PER_PAGE) {
> /* Here we restore prealloced and compressed clu mappings */
> + old = pe_page ? get_u64_from_be_page(pe_page, i) : 0;
> + set_u64_to_be_page(wbd->md->page, i, old);
> }
> }
[ ... ]
> @@ -1408,6 +1373,7 @@ static void complete_wbd(struct qcow2 *qcow2, struct wb_desc *wbd)
> LIST_HEAD(wait_list);
> LIST_HEAD(end_list);
> unsigned long flags;
> + struct qio *qio;
>
> spin_lock_irqsave(&qcow2->md_pages_lock, flags);
> revert_l_entries_update(qcow2, wbd);
> @@ -1415,6 +1381,8 @@ static void complete_wbd(struct qcow2 *qcow2, struct wb_desc *wbd)
> &wait_list, &end_list);
> spin_unlock_irqrestore(&qcow2->md_pages_lock, flags);
>
> + list_for_each_entry(qio, &end_list, link)
> + qio->ext->cleanup_mask &= ~FREE_ALLOCATED_CLU;
> qcow2_dispatch_qios(qcow2, NULL, &wait_list);
> end_qios(&end_list, errno_to_blk_status(wbd->ret));
[Severity: Medium]
Can this leak more than one cluster per failed writeback in the L1 case?
In prepare_l1l2_allocation(), the qio that allocated an L2-table cluster is
queued on the L1 page wait list, not on wbd->dependent_list:
list_add_tail(&qio->link, &map->l1.md->wait_list);
On a failed writeback, complete_wbd() calls clear_writeback_status(), which
does:
list_splice_init(&md->wait_list, wait_list);
...
list_splice_init(&md->wbd->dependent_list, end_list);
Only dependent_list is ended with the error. The qios on md->wait_list are
re-dispatched through qcow2_dispatch_qios().
revert_l_entries_update() has already reset the L1 entry to 0, so the
re-dispatched qio finds it empty and allocates another L2-table cluster,
dirties L1 and attempts writeback again. The cluster from the earlier attempt
is no longer freed, since mark_cluster_unused() is gone.
With a persistent L1 writeback error, does each retry cycle leak one more
cluster? Before this patch the cluster was freed, so the retry reused the
same one.
The L2 data-cluster and COW cases look bounded, since those qios sit on
dependent_list and end with the error, losing one cluster per failed request.
I did not find anything later in the series that reclaims the leaked
clusters. Nothing appears to roll back free_cluster_search_pos either.
The commit message says:
> So the image needs no check and resume needs no refusal.
Is this accurate? The leaked case is exactly the one that check repairs,
and the message does not mention that a repeated L1 allocation retry can
leak further clusters.
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261109140143.1408038-2-alexander.ivanov@virtuozzo.com?part=8
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Devel] [PATCH hci-8.1 v11 08/10] dm-qcow2: leak clusters allocated for a failed L1/L2 writeback
2026-11-09 14:14 ` no-reply
@ 2026-11-09 14:36 ` Alexander Ivanov
0 siblings, 0 replies; 30+ messages in thread
From: Alexander Ivanov @ 2026-11-09 14:36 UTC (permalink / raw)
To: no-reply, andrey.zhadchenko; +Cc: devel
>> @@ -1408,6 +1373,7 @@ static void complete_wbd(struct qcow2 *qcow2, struct wb_desc *wbd)
>> LIST_HEAD(wait_list);
>> LIST_HEAD(end_list);
>> unsigned long flags;
>> + struct qio *qio;
>>
>> spin_lock_irqsave(&qcow2->md_pages_lock, flags);
>> revert_l_entries_update(qcow2, wbd);
>> @@ -1415,6 +1381,8 @@ static void complete_wbd(struct qcow2 *qcow2, struct wb_desc *wbd)
>> &wait_list, &end_list);
>> spin_unlock_irqrestore(&qcow2->md_pages_lock, flags);
>>
>> + list_for_each_entry(qio, &end_list, link)
>> + qio->ext->cleanup_mask &= ~FREE_ALLOCATED_CLU;
>> qcow2_dispatch_qios(qcow2, NULL, &wait_list);
>> end_qios(&end_list, errno_to_blk_status(wbd->ret));
> [Severity: Medium]
> Can this leak more than one cluster per failed writeback in the L1 case?
>
> In prepare_l1l2_allocation(), the qio that allocated an L2-table cluster is
> queued on the L1 page wait list, not on wbd->dependent_list:
>
> list_add_tail(&qio->link, &map->l1.md->wait_list);
>
> On a failed writeback, complete_wbd() calls clear_writeback_status(), which
> does:
>
> list_splice_init(&md->wait_list, wait_list);
> ...
> list_splice_init(&md->wbd->dependent_list, end_list);
>
> Only dependent_list is ended with the error. The qios on md->wait_list are
> re-dispatched through qcow2_dispatch_qios().
>
> revert_l_entries_update() has already reset the L1 entry to 0, so the
> re-dispatched qio finds it empty and allocates another L2-table cluster,
> dirties L1 and attempts writeback again. The cluster from the earlier attempt
> is no longer freed, since mark_cluster_unused() is gone.
>
> With a persistent L1 writeback error, does each retry cycle leak one more
> cluster? Before this patch the cluster was freed, so the retry reused the
> same one.
>
> The L2 data-cluster and COW cases look bounded, since those qios sit on
> dependent_list and end with the error, losing one cluster per failed request.
>
> I did not find anything later in the series that reclaims the leaked
> clusters. Nothing appears to roll back free_cluster_search_pos either.
>
> The commit message says:
>
>> So the image needs no check and resume needs no refusal.
> Is this accurate? The leaked case is exactly the one that check repairs,
> and the message does not mention that a repeated L1 allocation retry can
> leak further clusters.
Added "fail the L2 table allocation request on L1 writeback error" patch.
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v11 08/10] dm-qcow2: leak clusters allocated for a failed L1/L2 writeback
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 08/10] dm-qcow2: leak clusters allocated for a failed L1/L2 writeback Alexander Ivanov
2026-11-09 14:14 ` no-reply
@ 2026-11-09 15:42 ` Konstantin Khorenko
1 sibling, 0 replies; 30+ messages in thread
From: Konstantin Khorenko @ 2026-11-09 15:42 UTC (permalink / raw)
To: Alexander Ivanov; +Cc: devel
I won't apply this version, please send v12 with the changes below.
On 11/9/26 15:01, Alexander Ivanov wrote:
> spin_unlock_irqrestore(&qcow2->md_pages_lock, flags);
>
> + list_for_each_entry(qio, &end_list, link)
> + qio->ext->cleanup_mask &= ~FREE_ALLOCATED_CLU;
> qcow2_dispatch_qios(qcow2, NULL, &wait_list);
Item 1 of the cover letter reply: queue the L1-allocation qio on
l1.md->wbd->dependent_list instead of l1.md->wait_list.
Why: that qio does not reach end_list. It waits on l1.md->wait_list,
and on a failed L1 writeback clear_writeback_status() sends it back to
work. The reverted L1 entry is 0 again, so it allocates another
L2-table cluster, and the L1 page is written again in the same worker
pass. Nothing slows this down: with a persistent L1 write error every
round leaks one more cluster and grows the file. Before this patch the
same loop freed the cluster and reused it.
On success a dependent qio goes back to work the same way as a waiter,
so moving it costs nothing there. Today only the qios added in
process_indexes_write() sit on dependent_list, and they all have ext.
A plain write that allocates an L2 table may have none, so check
qio->ext in the new loop.
> Keep the refcount of these clusters instead. If L1/L2 on disk points to
> such a cluster, the image stays consistent; otherwise the cluster is
> leaked, which check repairs by freeing it. So the image needs no check
> and resume needs no refusal.
Item 2 of the cover letter reply: say what is leaked, one cluster per
failed L1/L2 write of a request.
Why: your v10 answer bounded the leak by "refcounts reach the disk
before L1/L2". In this driver it is the other way round: R1/R2 pages
wait on slow_wb_batch_list for the timer (see the comment above
mark_cluster_used()), while L1/L2 pages are written in the same worker
pass. And an L1 update has no data write that could fail first.
--
Konstantin Khorenko,
Virtuozzo Linux Kernel Team
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Devel] [PATCH hci-8.1 v11 09/10] dm-qcow2: retry R1/R2 writeback that failed with ENOSPC
2026-11-09 14:01 [Devel] [PATCH hci-8.1 v11 00/10] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
` (7 preceding siblings ...)
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 08/10] dm-qcow2: leak clusters allocated for a failed L1/L2 writeback Alexander Ivanov
@ 2026-11-09 14:01 ` Alexander Ivanov
2026-11-09 14:16 ` no-reply
2026-11-09 15:42 ` Konstantin Khorenko
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 10/10] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
` (2 subsequent siblings)
11 siblings, 2 replies; 30+ messages in thread
From: Alexander Ivanov @ 2026-11-09 14:01 UTC (permalink / raw)
To: andrey.zhadchenko; +Cc: devel
A failed R1/R2 writeback sets MD_WRITEBACK_ERROR and drops MD_DIRTY.
Nothing writes the page again unless its refcounts change. On suspend
the md writeback flush finds the error and sets md_writeback_error, so
resume is refused until the image is checked and the target reloaded,
even if the failure was a transient ENOSPC of the underlying fs and
the space has been freed since.
Redirty the page on -ENOSPC, so the slow writeback timer retries it,
as data writes delayed on ENOSPC are retried. Flushing deferred md
activity on suspend, merge and target destruction fires the timer,
which gives the page one more try. A page that still fails then is not
redirtied, so no timer is left armed after the flush, before the image
is freed, and md_writeback_error is set as before.
complete_metadata_writeback() turned every failed write into -EIO.
Pass the errno of the write through and keep -EIO for a short write.
A data request depending on a failed L1/L2 write then ends with
BLK_STS_NOSPC and is delayed and resubmitted like any other write
failing with ENOSPC, as already happens when fsync fails with it.
https://virtuozzo.atlassian.net/browse/VSTOR-148171
Feature: dm-qcow2: block device over QCOW2 files driver
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
drivers/md/dm-qcow2-map.c | 6 ++++--
drivers/md/dm-qcow2-target.c | 4 +++-
drivers/md/dm-qcow2.h | 1 +
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 4ab8051f08c4..3bcaceab1a1d 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -1357,6 +1357,8 @@ static void clear_writeback_status(struct qcow2 *qcow2, struct md_page *md,
* so here we care about R1R2 only.
*/
md->status |= MD_WRITEBACK_ERROR;
+ if (ret == -ENOSPC && !READ_ONCE(qcow2->md_wb_no_retry))
+ qcow2_md_make_dirty(qcow2, md, true);
}
if (md->wbd) {
if (likely(ret == 0))
@@ -3867,9 +3869,9 @@ static int complete_metadata_writeback(struct qcow2 *qcow2)
md = qio->ext->md;
qvec = qio->data;
ret = qio->ret;
- if (unlikely(ret != PAGE_SIZE))
+ if (unlikely(ret >= 0 && ret != PAGE_SIZE))
ret = -EIO;
- else
+ else if (ret == PAGE_SIZE)
ret = fsync_ret;
do_md_page_write_complete(ret, qcow2, md);
diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
index 99fb68a5535e..e28c47d4e728 100644
--- a/drivers/md/dm-qcow2-target.c
+++ b/drivers/md/dm-qcow2-target.c
@@ -173,7 +173,9 @@ void qcow2_flush_deferred_activity(struct qcow2_target *tgt, struct qcow2 *qcow2
struct md_page *md;
int i;
+ /* Retry R1/R2 writeback failed with ENOSPC once, then give up */
for (i = 0; i < 2; i++) {
+ WRITE_ONCE(qcow2->md_wb_no_retry, i == 1);
if (timer_delete_sync_try(&qcow2->slow_wb_timer) != -1)
qcow2_slow_wb_timer_fn(&qcow2->slow_wb_timer);
/* Start md writeback */
@@ -181,12 +183,12 @@ void qcow2_flush_deferred_activity(struct qcow2_target *tgt, struct qcow2 *qcow2
/* Wait AIO of md wb */
qcow2_inflight_ref_switch(tgt);
}
+ WRITE_ONCE(qcow2->md_wb_no_retry, false);
spin_lock_irq(&qcow2->md_pages_lock);
for (node = rb_first(&qcow2->md_pages);
node; node = rb_next(node)) {
md = rb_entry(node, struct md_page, node);
- /* FIXME: call md_make_dirty() and try once again? */
if (md->status & MD_WRITEBACK_ERROR) {
QC_ERR(tgt->ti, "Failed to write dirty pages");
tgt->md_writeback_error = true;
diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
index 2a6203c86e3a..ebab22d505e8 100644
--- a/drivers/md/dm-qcow2.h
+++ b/drivers/md/dm-qcow2.h
@@ -281,6 +281,7 @@ struct qcow2 {
struct list_head wb_batch_list;
struct list_head slow_wb_batch_list;
struct timer_list slow_wb_timer;
+ bool md_wb_no_retry;
struct work_struct worker;
struct work_struct fsync_worker;
--
2.53.0
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Devel] [PATCH hci-8.1 v11 09/10] dm-qcow2: retry R1/R2 writeback that failed with ENOSPC
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 09/10] dm-qcow2: retry R1/R2 writeback that failed with ENOSPC Alexander Ivanov
@ 2026-11-09 14:16 ` no-reply
2026-11-09 14:39 ` Alexander Ivanov
2026-11-09 15:42 ` Konstantin Khorenko
1 sibling, 1 reply; 30+ messages in thread
From: no-reply @ 2026-11-09 14:16 UTC (permalink / raw)
To: andrey.zhadchenko, Alexander Ivanov; +Cc: devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dm-qcow2: flush gives failed R1/R2 page two retries, not one
--
commit 2997fd4391d8fcb3d55f988df71bac728d0346e1
Author: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
dm-qcow2: retry R1/R2 writeback that failed with ENOSPC
This patch redirties an R1/R2 metadata page whose writeback failed with
-ENOSPC, so the slow writeback timer retries it. It adds a flag,
md_wb_no_retry, that qcow2_flush_deferred_activity() sets on its second
pass to stop further redirtying. It also passes the errno of a failed
metadata write through complete_metadata_writeback() instead of
converting it to -EIO.
> diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
> --- a/drivers/md/dm-qcow2-target.c
> +++ b/drivers/md/dm-qcow2-target.c
[ ... ]
> @@ -173,7 +173,9 @@ void qcow2_flush_deferred_activity(struct qcow2_target *tgt, struct qcow2 *qcow2
> struct md_page *md;
> int i;
>
> + /* Retry R1/R2 writeback failed with ENOSPC once, then give up */
> for (i = 0; i < 2; i++) {
> + WRITE_ONCE(qcow2->md_wb_no_retry, i == 1);
> if (timer_delete_sync_try(&qcow2->slow_wb_timer) != -1)
> qcow2_slow_wb_timer_fn(&qcow2->slow_wb_timer);
[Severity: Low]
This isn't a functional bug, but the comment and the commit message don't
match what the loop does.
Consider a page that failed with -ENOSPC at runtime and was redirtied onto
slow_wb_batch_list:
qcow2_flush_deferred_activity()
pass i == 0: md_wb_no_retry is false, the timer is fired and the page
is rewritten
clear_writeback_status()
if (ret == -ENOSPC && !READ_ONCE(qcow2->md_wb_no_retry))
qcow2_md_make_dirty(qcow2, md, true);
pass i == 1: md_wb_no_retry is true, the timer is fired again and the
page is rewritten a second time
If the write in pass 0 fails with -ENOSPC again, the page is redirtied. So
a page that had already failed gets two flush-time attempts. Only the
failure in pass 1 is final.
The code comment says "Retry R1/R2 writeback failed with ENOSPC once, then
give up". The commit message says the flush "gives the page one more try"
and that "A page that still fails then is not redirtied".
Could the comment and the commit message be reworded to say that the flush
makes up to two attempts, or could the loop be changed so that only one
attempt is made?
[ ... ]
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261109140143.1408038-2-alexander.ivanov@virtuozzo.com?part=9
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Devel] [PATCH hci-8.1 v11 09/10] dm-qcow2: retry R1/R2 writeback that failed with ENOSPC
2026-11-09 14:16 ` no-reply
@ 2026-11-09 14:39 ` Alexander Ivanov
0 siblings, 0 replies; 30+ messages in thread
From: Alexander Ivanov @ 2026-11-09 14:39 UTC (permalink / raw)
To: no-reply, andrey.zhadchenko; +Cc: devel
On 11/9/26 15:16, no-reply@sashiko.virtuozzo.com wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] dm-qcow2: flush gives failed R1/R2 page two retries, not one
> --
>
> commit 2997fd4391d8fcb3d55f988df71bac728d0346e1
> Author: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
>
> dm-qcow2: retry R1/R2 writeback that failed with ENOSPC
>
> This patch redirties an R1/R2 metadata page whose writeback failed with
> -ENOSPC, so the slow writeback timer retries it. It adds a flag,
> md_wb_no_retry, that qcow2_flush_deferred_activity() sets on its second
> pass to stop further redirtying. It also passes the errno of a failed
> metadata write through complete_metadata_writeback() instead of
> converting it to -EIO.
>
>> diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
>> --- a/drivers/md/dm-qcow2-target.c
>> +++ b/drivers/md/dm-qcow2-target.c
> [ ... ]
>
>> @@ -173,7 +173,9 @@ void qcow2_flush_deferred_activity(struct qcow2_target *tgt, struct qcow2 *qcow2
>> struct md_page *md;
>> int i;
>>
>> + /* Retry R1/R2 writeback failed with ENOSPC once, then give up */
>> for (i = 0; i < 2; i++) {
>> + WRITE_ONCE(qcow2->md_wb_no_retry, i == 1);
>> if (timer_delete_sync_try(&qcow2->slow_wb_timer) != -1)
>> qcow2_slow_wb_timer_fn(&qcow2->slow_wb_timer);
> [Severity: Low]
> This isn't a functional bug, but the comment and the commit message don't
> match what the loop does.
>
> Consider a page that failed with -ENOSPC at runtime and was redirtied onto
> slow_wb_batch_list:
>
> qcow2_flush_deferred_activity()
> pass i == 0: md_wb_no_retry is false, the timer is fired and the page
> is rewritten
> clear_writeback_status()
> if (ret == -ENOSPC && !READ_ONCE(qcow2->md_wb_no_retry))
> qcow2_md_make_dirty(qcow2, md, true);
> pass i == 1: md_wb_no_retry is true, the timer is fired again and the
> page is rewritten a second time
>
> If the write in pass 0 fails with -ENOSPC again, the page is redirtied. So
> a page that had already failed gets two flush-time attempts. Only the
> failure in pass 1 is final.
>
> The code comment says "Retry R1/R2 writeback failed with ENOSPC once, then
> give up". The commit message says the flush "gives the page one more try"
> and that "A page that still fails then is not redirtied".
>
> Could the comment and the commit message be reworded to say that the flush
> makes up to two attempts, or could the loop be changed so that only one
> attempt is made?
>
Fixed the commit message.
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v11 09/10] dm-qcow2: retry R1/R2 writeback that failed with ENOSPC
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 09/10] dm-qcow2: retry R1/R2 writeback that failed with ENOSPC Alexander Ivanov
2026-11-09 14:16 ` no-reply
@ 2026-11-09 15:42 ` Konstantin Khorenko
1 sibling, 0 replies; 30+ messages in thread
From: Konstantin Khorenko @ 2026-11-09 15:42 UTC (permalink / raw)
To: Alexander Ivanov; +Cc: devel
I won't apply this version, please send v12 with the changes below.
On 11/9/26 15:01, Alexander Ivanov wrote:
> A failed R1/R2 writeback sets MD_WRITEBACK_ERROR and drops MD_DIRTY.
> which gives the page one more try. A page that still fails then is not
> redirtied, so no timer is left armed after the flush, before the image
> is freed, and md_writeback_error is set as before.
> + /* Retry R1/R2 writeback failed with ENOSPC once, then give up */
Item 3 of the cover letter reply: say "up to two attempts", and that
MD_DIRTY is dropped at submit.
Why: a page that failed with ENOSPC before the flush is redirtied. Pass
0 fires the timer and writes it, and since md_wb_no_retry is still
false there, a second failure redirties it again. Pass 1 writes it once
more. MD_DIRTY is cleared in submit_metadata_writeback(), before the
write, not by its failure.
Item 4 of the cover letter reply: mention the leak per resubmission.
Why: with this patch a request whose L2 write fails with ENOSPC ends
with BLK_STS_NOSPC and is resubmitted after ENOSPC_TIMEOUT_JI. The L2
entry was reverted to 0, so the resubmitted request allocates a new
data cluster, and patch 8 keeps the old one. One sentence in the
message is enough; it lasts only as long as the host is out of space.
--
Konstantin Khorenko,
Virtuozzo Linux Kernel Team
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Devel] [PATCH hci-8.1 v11 10/10] dm-qcow2: truncate unused tail of image on suspend
2026-11-09 14:01 [Devel] [PATCH hci-8.1 v11 00/10] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
` (8 preceding siblings ...)
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 09/10] dm-qcow2: retry R1/R2 writeback that failed with ENOSPC Alexander Ivanov
@ 2026-11-09 14:01 ` Alexander Ivanov
2026-11-09 14:18 ` no-reply
2026-11-09 15:42 ` Konstantin Khorenko
2026-11-09 15:42 ` [Devel] [PATCH hci-8.1 v11 00/10] " Konstantin Khorenko
2026-11-09 15:54 ` Konstantin Khorenko
11 siblings, 2 replies; 30+ messages in thread
From: Alexander Ivanov @ 2026-11-09 14:01 UTC (permalink / raw)
To: andrey.zhadchenko; +Cc: devel
Discard punches holes in place of freed clusters, so they hold no
physical extents, but it doesn't change the file size. On postsuspend
we truncate the image file only down to file_preallocated_area_start,
i.e. we drop preallocations only. So if clusters at the end of the
file are freed (refcount dropped to zero), the file size stays the
same and the image never shrinks.
Add qcow2_calc_used_end(), which scans refcount blocks backward from
file_preallocated_area_start and returns the end of the last cluster
with non-zero refcount, clamped to file_preallocated_area_start since
the last cluster may be partial. Refcount ranges with no allocated
refblock are skipped at once.
The scan walks R1 entries normal I/O may never touch, so it validates
them itself instead of letting calc_r2_page_id() WARN on a corrupted
one. Metadata pages are taken from md cache or read synchronously.
This is called on suspended device after md writeback, so R1 and R2
can't change under us. A failed relocation of the reftable leaves
file_preallocated_area_start beyond the space covered by the reftable,
so the scan starts at most from its end.
Truncate the file to the used end. Fall back to the old behaviour on
scan error and after a failed R1/R2 writeback, which leaves refcounts
on disk stale.
Refcounts may also be below real usage after a crash, since both QEMU
with lazy_refcounts and this driver write L2 before refcounts. The scan
would then cut off clusters L2 still points to, even in a session
without guest writes. With kernel_sets_dirty_bit such an image has the
dirty bit set at open, and the disabled dirty bit check lets it be
opened RW, so skip the scan for it.
In the default mode userspace sets the dirty bit on every RW open, so
the bit doesn't mean the image is inconsistent. There the scan is safe
only because userspace checks the image before an RW open. A v2 image
has no dirty bit, so the opened_dirty guard never applies to it, and
it is covered only by that check too.
The scan trusts refcounts, as the rest of the driver does. Only a walk
over all L2 tables would catch a zeroed or misplaced R1 entry, so
corrupted refcounts are left to the same image check as stale ones.
Close and reload to RO go through postsuspend too. On remove,
__dm_destroy() calls the presuspend and postsuspend hooks if the live
table isn't suspended yet. It neither stops the queue nor waits for
I/O, but the device is idle there, because dm_lock_for_deletion()
refuses to remove an open device.
On table swap, do_resume() suspends the old (RW) table before
dm_swap_table(). So the file is shrunk in these cases as well.
https://virtuozzo.atlassian.net/browse/VSTOR-148171
Feature: dm-qcow2: block device over QCOW2 files driver
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
drivers/md/dm-ploop-target.c | 2 +-
drivers/md/dm-qcow2-map.c | 63 ++++++++++++++++++++++++++++++++++--
drivers/md/dm-qcow2-target.c | 21 +++++++++---
drivers/md/dm-qcow2.h | 1 +
4 files changed, 80 insertions(+), 7 deletions(-)
diff --git a/drivers/md/dm-ploop-target.c b/drivers/md/dm-ploop-target.c
index d8279c79ef56..b869c92bbe72 100644
--- a/drivers/md/dm-ploop-target.c
+++ b/drivers/md/dm-ploop-target.c
@@ -447,7 +447,7 @@ static struct ploop_worker *ploop_worker_create(struct ploop *ploop,
*
* FIXME: We must implement metadata reload on resume. After that we can
* implement preallocation truncation on suspend (see dm-qcow2 as it already
- * implements that in qcow2_parse_metadata and qcow2_truncate_preallocations).
+ * implements that in qcow2_parse_metadata and qcow2_truncate_unused_tail).
*/
static int ploop_ctr(struct dm_target *ti, unsigned int argc, char **argv)
{
diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 3bcaceab1a1d..621bc9b18e26 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -1090,6 +1090,11 @@ static int calc_refcounters_map(struct qcow2 *qcow2, loff_t pos,
return -EIO;
}
+static bool r1_entry_is_valid(struct qcow2 *qcow2, u64 entry)
+{
+ return !(entry & R1_RESERVED_ZERO_MASK) && CLU_OFF(qcow2, entry) == 0;
+}
+
static int calc_r2_page_id(struct qcow2 *qcow2, struct qcow2_map_item *r1,
struct qcow2_map_item *r2)
{
@@ -1097,8 +1102,7 @@ static int calc_r2_page_id(struct qcow2 *qcow2, struct qcow2_map_item *r1,
u32 bits = qcow2->refblock_bits;
loff_t pos;
- if (WARN_ON_ONCE((entry & R1_RESERVED_ZERO_MASK) ||
- (CLU_OFF(qcow2, entry) != 0)))
+ if (WARN_ON_ONCE(!r1_entry_is_valid(qcow2, entry)))
return -EIO;
/* The corresponding refcount block has not yet been allocated */
@@ -2168,6 +2172,61 @@ int qcow2_truncate_safe(struct file *file, loff_t new_len)
return do_ftruncate(file, new_len, 0);
}
+/*
+ * Return the end of the last cluster with non-zero refcount. Must be
+ * called on a suspended device after md writeback.
+ */
+loff_t qcow2_calc_used_end(struct qcow2 *qcow2)
+{
+ u32 clu_size = qcow2->clu_size;
+ struct qcow2_map_item r1, r2;
+ struct md_page *r1_md = NULL;
+ loff_t pos;
+ s32 i;
+ int ret;
+
+ pos = min(qcow2->file_preallocated_area_start,
+ qcow2->reftable_max_file_size);
+ pos = round_down(pos - 1, clu_size);
+ while (pos >= 0) {
+ if (calc_refcounters_map(qcow2, pos, &r1, &r2) < 0)
+ return -EIO;
+
+ if (!r1_md || r1_md->id != r1.page_id) {
+ ret = qcow2_md_page_read_sync(qcow2, r1.page_id, &r1_md);
+ if (ret)
+ return ret;
+ }
+ r1.md = r1_md;
+
+ if (!r1_entry_is_valid(qcow2, get_u64_from_be_page(r1_md->page,
+ r1.index_in_page)))
+ return -EIO;
+
+ ret = calc_r2_page_id(qcow2, &r1, &r2);
+ if (ret == -ENOENT) {
+ pos = (loff_t)r1.index * qcow2->refblock_entries * clu_size;
+ pos -= clu_size;
+ continue;
+ }
+
+ if (ret < 0)
+ return ret;
+
+ ret = qcow2_md_page_read_sync(qcow2, r2.page_id, &r2.md);
+ if (ret)
+ return ret;
+
+ for (i = r2.index_in_page; i >= 0; i--, pos -= clu_size) {
+ if (get_r2_entry(qcow2, r2.md, i))
+ return min_t(loff_t, pos + clu_size,
+ qcow2->file_preallocated_area_start);
+ }
+ }
+
+ return -EIO;
+}
+
static int truncate_prealloc_safe(struct qcow2 *qcow2, loff_t len, const char *func)
{
loff_t prealloc_len, max_prealloc_len = qcow2->reftable_max_file_size;
diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
index e28c47d4e728..30b6bb5c98f4 100644
--- a/drivers/md/dm-qcow2-target.c
+++ b/drivers/md/dm-qcow2-target.c
@@ -1061,21 +1061,34 @@ static void qcow2_dtr(struct dm_target *ti)
qcow2_tgt_destroy(tgt);
}
-static void qcow2_truncate_preallocations(struct dm_target *ti)
+static void qcow2_truncate_unused_tail(struct dm_target *ti)
{
struct qcow2_target *tgt = to_qcow2_target(ti);
struct qcow2 *qcow2 = top_qcow2_protected(ti);
- loff_t end = qcow2->file_preallocated_area_start;
+ loff_t used_end, end = qcow2->file_preallocated_area_start;
int ret;
if (!(dm_table_get_mode(ti->table) & FMODE_WRITE))
return;
+
+ if (tgt->md_writeback_error) {
+ QC_INFO(ti, "Skip shrinking to used end after md writeback error");
+ } else if (kernel_sets_dirty_bit && qcow2->opened_dirty) {
+ QC_INFO(ti, "Skip shrinking to used end of dirty image");
+ } else {
+ used_end = qcow2_calc_used_end(qcow2);
+ if (used_end < 0)
+ QC_ERR(ti, "Can't find used end: %pe", ERR_PTR(used_end));
+ else
+ end = used_end;
+ }
+
if (end == qcow2->file_size)
return;
ret = qcow2_truncate_safe(qcow2->file, end);
if (ret) {
- QC_ERR(ti, "Can't truncate preallocations");
+ QC_ERR(ti, "Can't truncate unused tail");
tgt->truncate_error = true;
return;
}
@@ -1156,7 +1169,7 @@ static void qcow2_postsuspend(struct dm_target *ti)
int ret;
qcow2_flush_deferred_activity_all(to_qcow2_target(ti));
- qcow2_truncate_preallocations(ti);
+ qcow2_truncate_unused_tail(ti);
if (dm_table_get_mode(ti->table) & FMODE_WRITE) {
ret = qcow2_set_image_file_features(qcow2, false);
diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
index ebab22d505e8..ed427a0a7699 100644
--- a/drivers/md/dm-qcow2.h
+++ b/drivers/md/dm-qcow2.h
@@ -416,6 +416,7 @@ void qcow2_ref_dec(struct qcow2_target *tgt, u8 ref_index);
int qcow2_inflight_ref_switch(struct qcow2_target *tgt);
void qcow2_flush_deferred_activity(struct qcow2_target *tgt, struct qcow2 *qcow2);
int qcow2_truncate_safe(struct file *file, loff_t new_len);
+loff_t qcow2_calc_used_end(struct qcow2 *qcow2);
void qcow2_merge_backward_work(struct work_struct *work);
void qcow2_merge_backward_cancel(struct qcow2_target *tgt);
--
2.53.0
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Devel] [PATCH hci-8.1 v11 10/10] dm-qcow2: truncate unused tail of image on suspend
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 10/10] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
@ 2026-11-09 14:18 ` no-reply
2026-11-09 14:47 ` Alexander Ivanov
2026-11-09 15:42 ` Konstantin Khorenko
1 sibling, 1 reply; 30+ messages in thread
From: no-reply @ 2026-11-09 14:18 UTC (permalink / raw)
To: Alexander Ivanov, andrey.zhadchenko; +Cc: devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dm-qcow2: unused-tail truncation on suspend trusts refcounts with no kernel-side guard
- [Medium] dm-qcow2: GFP_KERNEL md page reads in postsuspend can deadlock on reclaim
--
commit 74d743673c019d9879114318e501a9a5de73c937
Author: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
dm-qcow2: truncate unused tail of image on suspend
This patch adds qcow2_calc_used_end(), which scans the refcount blocks backward from file_preallocated_area_start. It returns the end of the last cluster with a non-zero refcount. qcow2_truncate_unused_tail() now truncates the image file to that point on postsuspend. It falls back to the old behaviour on scan errors, after a metadata writeback error, and for dirty images when kernel_sets_dirty_bit is set.
> diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
> --- a/drivers/md/dm-qcow2-map.c
> +++ b/drivers/md/dm-qcow2-map.c
[ ... ]
> +loff_t qcow2_calc_used_end(struct qcow2 *qcow2)
> +{
[ ... ]
> + if (!r1_md || r1_md->id != r1.page_id) {
> + ret = qcow2_md_page_read_sync(qcow2, r1.page_id, &r1_md);
> + if (ret)
> + return ret;
> + }
[ ... ]
> + ret = qcow2_md_page_read_sync(qcow2, r2.page_id, &r2.md);
> + if (ret)
> + return ret;
> +
> + for (i = r2.index_in_page; i >= 0; i--, pos -= clu_size) {
> + if (get_r2_entry(qcow2, r2.md, i))
> + return min_t(loff_t, pos + clu_size,
> + qcow2->file_preallocated_area_start);
> + }
[Severity: Medium]
Can this scan allocate memory in a way that deadlocks under reclaim? qcow2_calc_used_end() is called from the postsuspend hook:
qcow2_postsuspend()
qcow2_truncate_unused_tail()
qcow2_calc_used_end()
qcow2_md_page_read_sync()
qcow2_alloc_and_insert_md_page()
kmalloc(sizeof(**md), GFP_KERNEL)
alloc_page(GFP_KERNEL)
Every R1 or R2 page that is not already in the md cache is read this way, and then read from the image file. Neither the dm core nor the driver sets a memalloc_noio or memalloc_nofs scope around postsuspend.
Direct reclaim from these GFP_KERNEL allocations could issue writeback to a filesystem or swap on this same suspended device, or on a device stacked on it. That I/O cannot complete until resume, and resume waits for postsuspend.
Before this patch, postsuspend only used the already cached page 0 through qcow2_set_image_file_features(). The other allocations in the I/O and metadata paths use GFP_NOIO or run under PF_MEMALLOC_NOIO.
Should the scan run under a memalloc_noio_save()/memalloc_noio_restore() scope, or use GFP_NOIO allocations?
> diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
> --- a/drivers/md/dm-qcow2-target.c
> +++ b/drivers/md/dm-qcow2-target.c
[ ... ]
> + if (tgt->md_writeback_error) {
> + QC_INFO(ti, "Skip shrinking to used end after md writeback error");
> + } else if (kernel_sets_dirty_bit && qcow2->opened_dirty) {
> + QC_INFO(ti, "Skip shrinking to used end of dirty image");
> + } else {
> + used_end = qcow2_calc_used_end(qcow2);
[Severity: Medium]
Can this truncation cut off live guest data? The used end is derived from refcounts alone, with no cross-check against L2. If a cluster has refcount zero but is still referenced by L2, the cut point moves below live data and qcow2_truncate_safe() discards it permanently.
The commit message says refcounts may be below real usage after a crash, since both QEMU with lazy_refcounts and this driver write L2 before refcounts. The only guards here are md_writeback_error and kernel_sets_dirty_bit && opened_dirty.
A v2 image has no dirty bit. qcow2_check_convert_hdr() returns early for version 2, so opened_dirty stays false.
In the default mode kernel_sets_dirty_bit is false, so the opened_dirty guard is skipped.
The ctr-time check in qcow2_check_convert_hdr() is commented out:
// if (!is_ro && kernel_sets_dirty_bit !=
// !(hdr->incompatible_features & INCOMPATIBLE_FEATURES_DIRTY_BIT))
// return kernel_sets_dirty_bit ? -EUCLEAN : -ENOLCK;
qcow2_wants_check() covers only md_writeback_error and truncate_error, which are errors from the current session. Nothing in the driver detects an on-disk inconsistency left by an earlier crash.
The commit message says the scan is safe "only because userspace checks the image before an RW open". Can the driver verify that assumption?
Would it be possible to refuse the truncation when the cut range is still referenced by L2, or to make the shrink opt-in?
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261109140143.1408038-2-alexander.ivanov@virtuozzo.com?part=10
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Devel] [PATCH hci-8.1 v11 10/10] dm-qcow2: truncate unused tail of image on suspend
2026-11-09 14:18 ` no-reply
@ 2026-11-09 14:47 ` Alexander Ivanov
0 siblings, 0 replies; 30+ messages in thread
From: Alexander Ivanov @ 2026-11-09 14:47 UTC (permalink / raw)
To: no-reply, andrey.zhadchenko; +Cc: devel
>> + ret = qcow2_md_page_read_sync(qcow2, r2.page_id, &r2.md);
>> + if (ret)
>> + return ret;
>> +
>> + for (i = r2.index_in_page; i >= 0; i--, pos -= clu_size) {
>> + if (get_r2_entry(qcow2, r2.md, i))
>> + return min_t(loff_t, pos + clu_size,
>> + qcow2->file_preallocated_area_start);
>> + }
> [Severity: Medium]
> Can this scan allocate memory in a way that deadlocks under reclaim? qcow2_calc_used_end() is called from the postsuspend hook:
>
> qcow2_postsuspend()
> qcow2_truncate_unused_tail()
> qcow2_calc_used_end()
> qcow2_md_page_read_sync()
> qcow2_alloc_and_insert_md_page()
> kmalloc(sizeof(**md), GFP_KERNEL)
> alloc_page(GFP_KERNEL)
>
> Every R1 or R2 page that is not already in the md cache is read this way, and then read from the image file. Neither the dm core nor the driver sets a memalloc_noio or memalloc_nofs scope around postsuspend.
>
> Direct reclaim from these GFP_KERNEL allocations could issue writeback to a filesystem or swap on this same suspended device, or on a device stacked on it. That I/O cannot complete until resume, and resume waits for postsuspend.
>
> Before this patch, postsuspend only used the already cached page 0 through qcow2_set_image_file_features(). The other allocations in the I/O and metadata paths use GFP_NOIO or run under PF_MEMALLOC_NOIO.
>
> Should the scan run under a memalloc_noio_save()/memalloc_noio_restore() scope, or use GFP_NOIO allocations?
Fixed.
>> + if (tgt->md_writeback_error) {
>> + QC_INFO(ti, "Skip shrinking to used end after md writeback error");
>> + } else if (kernel_sets_dirty_bit && qcow2->opened_dirty) {
>> + QC_INFO(ti, "Skip shrinking to used end of dirty image");
>> + } else {
>> + used_end = qcow2_calc_used_end(qcow2);
> [Severity: Medium]
> Can this truncation cut off live guest data? The used end is derived from refcounts alone, with no cross-check against L2. If a cluster has refcount zero but is still referenced by L2, the cut point moves below live data and qcow2_truncate_safe() discards it permanently.
>
> The commit message says refcounts may be below real usage after a crash, since both QEMU with lazy_refcounts and this driver write L2 before refcounts. The only guards here are md_writeback_error and kernel_sets_dirty_bit && opened_dirty.
>
> A v2 image has no dirty bit. qcow2_check_convert_hdr() returns early for version 2, so opened_dirty stays false.
>
> In the default mode kernel_sets_dirty_bit is false, so the opened_dirty guard is skipped.
>
> The ctr-time check in qcow2_check_convert_hdr() is commented out:
>
> // if (!is_ro && kernel_sets_dirty_bit !=
> // !(hdr->incompatible_features & INCOMPATIBLE_FEATURES_DIRTY_BIT))
> // return kernel_sets_dirty_bit ? -EUCLEAN : -ENOLCK;
>
> qcow2_wants_check() covers only md_writeback_error and truncate_error, which are errors from the current session. Nothing in the driver detects an on-disk inconsistency left by an earlier crash.
>
> The commit message says the scan is safe "only because userspace checks the image before an RW open". Can the driver verify that assumption?
>
> Would it be possible to refuse the truncation when the cut range is still referenced by L2, or to make the shrink opt-in?
As I wrtote, the driver already relies on refcounts being consistent:
find_unused_cluster()
reuses any cluster with zero refcount, so an image with refcounts below
L2 usage gets live
data overwritten by the first allocation that reaches it, with or
without this patch.
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v11 10/10] dm-qcow2: truncate unused tail of image on suspend
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 10/10] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
2026-11-09 14:18 ` no-reply
@ 2026-11-09 15:42 ` Konstantin Khorenko
1 sibling, 0 replies; 30+ messages in thread
From: Konstantin Khorenko @ 2026-11-09 15:42 UTC (permalink / raw)
To: Alexander Ivanov; +Cc: devel
I won't apply this version, please send v12 with the changes below.
On 11/9/26 15:01, Alexander Ivanov wrote:
> Truncate the file to the used end. Fall back to the old behaviour on
> scan error and after a failed R1/R2 writeback, which leaves refcounts
> on disk stale.
Item 5 of the cover letter reply: the skip is on md_writeback_error,
which a late reftable relocation failure (patch 7) sets as well.
With that fixed:
Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
--
Konstantin Khorenko,
Virtuozzo Linux Kernel Team
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Devel] [PATCH hci-8.1 v11 00/10] dm-qcow2: truncate unused tail of image on suspend
2026-11-09 14:01 [Devel] [PATCH hci-8.1 v11 00/10] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
` (9 preceding siblings ...)
2026-11-09 14:01 ` [Devel] [PATCH hci-8.1 v11 10/10] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
@ 2026-11-09 15:42 ` Konstantin Khorenko
2026-11-09 15:54 ` Konstantin Khorenko
11 siblings, 0 replies; 30+ messages in thread
From: Konstantin Khorenko @ 2026-11-09 15:42 UTC (permalink / raw)
To: Alexander Ivanov; +Cc: devel
I won't apply this version, please send v12 with the changes below.
On 11/9/26 15:01, Alexander Ivanov wrote:
> Alexander Ivanov (10):
> dm-qcow2: add qcow2_md_page_read_sync() helper
> dm-qcow2: drop duplicate prefix from QC_ERR/QC_INFO messages
> dm-qcow2: extend the file for a new refblock in allocate_cluster()
> dm-qcow2: complete wbd before ending the data qio
> dm-qcow2: flush lower md before clearing dirty bit on merge failure
> dm-qcow2: keep the dirty bit of an image that needs check
> dm-qcow2: flag the image for check if reftable relocation fails late
> dm-qcow2: leak clusters allocated for a failed L1/L2 writeback
> dm-qcow2: retry R1/R2 writeback that failed with ENOSPC
> dm-qcow2: truncate unused tail of image on suspend
What to change for v12:
1. Patch 8, code: in prepare_l1l2_allocation(), queue the qio that
allocates an L2-table cluster for an L1 update on
l1.md->wbd->dependent_list instead of l1.md->wait_list, so a failed
L1 writeback ends it with the error instead of sending it back to
allocate another cluster. The new loop in complete_wbd() then has to
skip a qio without ext.
2. Patch 8, commit message: say what is leaked: one cluster per failed
L1/L2 write of a request.
3. Patch 9, comment and commit message: a page that failed before the
flush gets up to two attempts during it, not "once" or "one more
try". MD_DIRTY is dropped when the write is submitted, not by the
failure.
4. Patch 9, commit message: say that with patch 8 a request whose L2
write keeps failing with ENOSPC leaks one data cluster on every
resubmission, until the space is back.
5. Patch 10, commit message: the scan is skipped on md_writeback_error,
which a late reftable relocation failure (patch 7) sets too, not only
a failed R1/R2 writeback.
6. Cover letter: the patch 9 line of the changelog says "once" as well,
fix it as in item 3.
Separate patches, not needed for v12:
1. A REQ_OP_FLUSH whose fsync fails with ENOSPC ends with
BLK_STS_NOSPC, and qrq_endio() hits WARN_ON_ONCE(!op_is_write())
for it. This is older than the series.
--
Konstantin Khorenko,
Virtuozzo Linux Kernel Team
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Devel] [PATCH hci-8.1 v11 00/10] dm-qcow2: truncate unused tail of image on suspend
2026-11-09 14:01 [Devel] [PATCH hci-8.1 v11 00/10] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
` (10 preceding siblings ...)
2026-11-09 15:42 ` [Devel] [PATCH hci-8.1 v11 00/10] " Konstantin Khorenko
@ 2026-11-09 15:54 ` Konstantin Khorenko
11 siblings, 0 replies; 30+ messages in thread
From: Konstantin Khorenko @ 2026-11-09 15:54 UTC (permalink / raw)
To: Alexander Ivanov; +Cc: devel
Sasha,
One more thing to my v11 review, the item I listed under "Separate
patches": a REQ_OP_FLUSH whose fsync fails with ENOSPC hits
WARN_ON_ONCE(!op_is_write()) in qrq_endio().
I checked this one to the end, it is real. Please send a separate
patch for it.
The path:
- qcow2_clone_and_map() sets endio_cb = qrq_endio for every request,
a flush included;
- qcow2_submit_embedded_qio() puts an empty request on QLIST_FLUSH;
- do_qcow2_fsync_work() ends it with errno_to_blk_status(fsync_ret),
so a failed vfs_fsync() with -ENOSPC gives BLK_STS_NOSPC;
- qrq_endio() then does WARN_ON_ONCE(!op_is_write(qio->bi_op)), and
op_is_write(REQ_OP_FLUSH) is false.
fsync returns ENOSPC on file systems that allocate on writeback (NFS,
FUSE, thin or reflink backends), or when ext4/xfs report a failed
delayed write. The rest of the handling is already right: the flush is
delayed and resubmitted like a write. Only the WARN is wrong, and with
panic_on_warn it takes the node down.
Suggested fix:
WARN_ON_ONCE(!op_is_write(qio->bi_op) &&
qio->bi_op != REQ_OP_FLUSH);
The WARN came with 7d49e18c5c36f ("dm-qcow2: Pause IO and notify about
ENOSPC events"), so that is the Fixes tag.
--
Konstantin Khorenko,
Virtuozzo Linux Kernel Team
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 30+ messages in thread