* [Devel] [PATCH hci-8.1 v8 1/6] dm-qcow2: add qcow2_md_page_read_sync() helper
2026-10-26 14:42 [Devel] [PATCH hci-8.1 v8 0/6] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
@ 2026-10-26 14:42 ` Alexander Ivanov
2026-10-26 14:42 ` [Devel] [PATCH hci-8.1 v8 2/6] dm-qcow2: drop duplicate prefix from QC_ERR/QC_INFO messages Alexander Ivanov
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Alexander Ivanov @ 2026-10-26 14:42 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 | 41 +++++++++++++++++++++++++++++++-----
drivers/md/dm-qcow2.h | 2 ++
2 files changed, 38 insertions(+), 5 deletions(-)
diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
index be09615eb8e2..311a23d63a5c 100644
--- a/drivers/md/dm-qcow2-target.c
+++ b/drivers/md/dm-qcow2-target.c
@@ -418,6 +418,41 @@ 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.
+ *
+ * The caller must ensure no read of this page is in flight
+ * (e.g., the device is suspended): if the page is cached but
+ * not MD_UPTODATE yet, qcow2_md_page_find_or_postpone() returns
+ * NULL and the following insert fails.
+ */
+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)
+ return ret;
+
+ ret = qcow2_rw_page_sync(READ, qcow2, id, (*md)->page);
+ spin_lock_irq(&qcow2->md_pages_lock);
+ if (ret)
+ qcow2_md_page_erase(qcow2, *md);
+ else
+ (*md)->status |= MD_UPTODATE;
+ spin_unlock_irq(&qcow2->md_pages_lock);
+ if (ret)
+ qcow2_free_md_page(*md);
+ return ret;
+}
+
static void inflight_ref_exit0(struct percpu_ref *ref)
{
struct qcow2_target *tgt = container_of(ref, struct qcow2_target,
@@ -851,13 +886,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 d9b8c38e093f..69d3ae6fca8a 100644
--- a/drivers/md/dm-qcow2.h
+++ b/drivers/md/dm-qcow2.h
@@ -386,6 +386,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] 10+ messages in thread* [Devel] [PATCH hci-8.1 v8 2/6] dm-qcow2: drop duplicate prefix from QC_ERR/QC_INFO messages
2026-10-26 14:42 [Devel] [PATCH hci-8.1 v8 0/6] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
2026-10-26 14:42 ` [Devel] [PATCH hci-8.1 v8 1/6] dm-qcow2: add qcow2_md_page_read_sync() helper Alexander Ivanov
@ 2026-10-26 14:42 ` Alexander Ivanov
2026-10-26 14:42 ` [Devel] [PATCH hci-8.1 v8 3/6] dm-qcow2: extend the file for a new refblock in allocate_cluster() Alexander Ivanov
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Alexander Ivanov @ 2026-10-26 14:42 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 311a23d63a5c..b667adb5472a 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;
}
@@ -834,19 +834,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;
@@ -879,7 +879,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;
@@ -910,14 +910,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 */
}
}
@@ -939,11 +939,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;
}
}
@@ -976,7 +976,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;
}
@@ -1057,7 +1057,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;
}
@@ -1143,7 +1143,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)
@@ -1152,7 +1152,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;
}
@@ -1173,7 +1173,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] 10+ messages in thread* [Devel] [PATCH hci-8.1 v8 3/6] dm-qcow2: extend the file for a new refblock in allocate_cluster()
2026-10-26 14:42 [Devel] [PATCH hci-8.1 v8 0/6] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
2026-10-26 14:42 ` [Devel] [PATCH hci-8.1 v8 1/6] dm-qcow2: add qcow2_md_page_read_sync() helper Alexander Ivanov
2026-10-26 14:42 ` [Devel] [PATCH hci-8.1 v8 2/6] dm-qcow2: drop duplicate prefix from QC_ERR/QC_INFO messages Alexander Ivanov
@ 2026-10-26 14:42 ` Alexander Ivanov
2026-10-26 14:42 ` [Devel] [PATCH hci-8.1 v8 4/6] dm-qcow2: complete wbd before ending the data qio Alexander Ivanov
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Alexander Ivanov @ 2026-10-26 14:42 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
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 78ee7cf5e881..5952b5feb8bf 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -2085,21 +2085,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);
@@ -2115,7 +2106,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,
@@ -2137,11 +2128,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)
@@ -2153,15 +2146,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);
@@ -2432,6 +2425,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
@@ -2441,13 +2465,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)
@@ -2457,28 +2480,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] 10+ messages in thread* [Devel] [PATCH hci-8.1 v8 4/6] dm-qcow2: complete wbd before ending the data qio
2026-10-26 14:42 [Devel] [PATCH hci-8.1 v8 0/6] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
` (2 preceding siblings ...)
2026-10-26 14:42 ` [Devel] [PATCH hci-8.1 v8 3/6] dm-qcow2: extend the file for a new refblock in allocate_cluster() Alexander Ivanov
@ 2026-10-26 14:42 ` Alexander Ivanov
2026-10-26 14:42 ` [Devel] [PATCH hci-8.1 v8 5/6] dm-qcow2: keep the dirty bit of an image opened dirty Alexander Ivanov
2026-10-26 14:42 ` [Devel] [PATCH hci-8.1 v8 6/6] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
5 siblings, 0 replies; 10+ messages in thread
From: Alexander Ivanov @ 2026-10-26 14:42 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 R2 after the flush, and the change
is never written back.
Complete the wbd first. 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
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 5952b5feb8bf..fa63bd6acfb8 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -2888,13 +2888,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] 10+ messages in thread* [Devel] [PATCH hci-8.1 v8 5/6] dm-qcow2: keep the dirty bit of an image opened dirty
2026-10-26 14:42 [Devel] [PATCH hci-8.1 v8 0/6] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
` (3 preceding siblings ...)
2026-10-26 14:42 ` [Devel] [PATCH hci-8.1 v8 4/6] dm-qcow2: complete wbd before ending the data qio Alexander Ivanov
@ 2026-10-26 14:42 ` Alexander Ivanov
2026-10-28 11:59 ` Andrey Zhadchenko
2026-10-26 14:42 ` [Devel] [PATCH hci-8.1 v8 6/6] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
5 siblings, 1 reply; 10+ messages in thread
From: Alexander Ivanov @ 2026-10-26 14:42 UTC (permalink / raw)
To: andrey.zhadchenko; +Cc: devel
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.
Clear the bit only if the image was clean before the kernel set it.
The parsed header can't tell this: 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 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 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.
This only preserves the on-disk marker. The driver still accepts a
dirty image read-write and allocates from its refcounts, so
find_unused_cluster() may hand out clusters that L2 still references
until the image is checked.
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 | 15 +++++++++++++--
drivers/md/dm-qcow2.h | 2 ++
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
index b667adb5472a..2d69e39a3530 100644
--- a/drivers/md/dm-qcow2-target.c
+++ b/drivers/md/dm-qcow2-target.c
@@ -659,6 +659,7 @@ int qcow2_set_image_file_features(struct qcow2 *qcow2, bool dirty)
struct QCowHeader *raw_hdr;
struct md_page *md;
bool is_ro;
+ int ret;
if (qcow2->hdr.version == 2)
return 0;
@@ -676,12 +677,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)
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 = dirty && !ret;
+ return ret;
}
ALLOW_ERROR_INJECTION(qcow2_set_image_file_features, ERRNO);
@@ -903,6 +907,13 @@ static int qcow2_parse_header(struct dm_target *ti, struct qcow2 *qcow2,
goto out;
qcow2_calc_cached_parameters(qcow2, hdr);
+ /* The header is reread on resume and may carry our own dirty bit */
+ if (!(hdr->incompatible_features & INCOMPATIBLE_FEATURES_DIRTY_BIT)) {
+ qcow2->opened_dirty = false;
+ qcow2->kernel_dirty_bit = false;
+ } else if (!qcow2->kernel_dirty_bit) {
+ 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 69d3ae6fca8a..a2d951ee5541 100644
--- a/drivers/md/dm-qcow2.h
+++ b/drivers/md/dm-qcow2.h
@@ -257,6 +257,8 @@ struct qcow2 {
u32 refblock_bits;
u32 refblock_entries;
bool ext_l2;
+ bool opened_dirty; /* Dirty bit was set before we set it */
+ bool kernel_dirty_bit; /* We set the dirty bit on disk */
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] 10+ messages in thread* Re: [Devel] [PATCH hci-8.1 v8 5/6] dm-qcow2: keep the dirty bit of an image opened dirty
2026-10-26 14:42 ` [Devel] [PATCH hci-8.1 v8 5/6] dm-qcow2: keep the dirty bit of an image opened dirty Alexander Ivanov
@ 2026-10-28 11:59 ` Andrey Zhadchenko
0 siblings, 0 replies; 10+ messages in thread
From: Andrey Zhadchenko @ 2026-10-28 11:59 UTC (permalink / raw)
To: Alexander Ivanov; +Cc: devel
1. Get rid of useless comments (which is all comments in this patch)
2. kernel_dirty_bit should be kernel_dirty_bit_set because without set
it makes no sense.
3. Commit message should be reworded to include the following very
important information:
- kernel_sets_dirty_bit is a module parameter
- we do not set it in Virtuozzo
- ploop sets this from userspace itself (so this is rather a hardening
patch)
On 10/26/26 15:42, Alexander Ivanov wrote:
> 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.
>
> Clear the bit only if the image was clean before the kernel set it.
> The parsed header can't tell this: 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 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 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.
>
> This only preserves the on-disk marker. The driver still accepts a
> dirty image read-write and allocates from its refcounts, so
> find_unused_cluster() may hand out clusters that L2 still references
> until the image is checked.
Completely irrelevant paragraph
>
> 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 | 15 +++++++++++++--
> drivers/md/dm-qcow2.h | 2 ++
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
> index b667adb5472a..2d69e39a3530 100644
> --- a/drivers/md/dm-qcow2-target.c
> +++ b/drivers/md/dm-qcow2-target.c
> @@ -659,6 +659,7 @@ int qcow2_set_image_file_features(struct qcow2 *qcow2, bool dirty)
> struct QCowHeader *raw_hdr;
> struct md_page *md;
> bool is_ro;
> + int ret;
>
> if (qcow2->hdr.version == 2)
> return 0;
> @@ -676,12 +677,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)
> 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 = dirty && !ret;
> + return ret;
> }
> ALLOW_ERROR_INJECTION(qcow2_set_image_file_features, ERRNO);
>
> @@ -903,6 +907,13 @@ static int qcow2_parse_header(struct dm_target *ti, struct qcow2 *qcow2,
> goto out;
>
> qcow2_calc_cached_parameters(qcow2, hdr);
> + /* The header is reread on resume and may carry our own dirty bit */
> + if (!(hdr->incompatible_features & INCOMPATIBLE_FEATURES_DIRTY_BIT)) {
> + qcow2->opened_dirty = false;
> + qcow2->kernel_dirty_bit = false;
> + } else if (!qcow2->kernel_dirty_bit) {
> + 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 69d3ae6fca8a..a2d951ee5541 100644
> --- a/drivers/md/dm-qcow2.h
> +++ b/drivers/md/dm-qcow2.h
> @@ -257,6 +257,8 @@ struct qcow2 {
> u32 refblock_bits;
> u32 refblock_entries;
> bool ext_l2;
> + bool opened_dirty; /* Dirty bit was set before we set it */> + bool kernel_dirty_bit; /* We set the dirty bit on disk */
>
> bool pause_submitting_qios; /* This is used only on top qcow2 image */
> bool backward_merge_in_process;
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Devel] [PATCH hci-8.1 v8 6/6] dm-qcow2: truncate unused tail of image on suspend
2026-10-26 14:42 [Devel] [PATCH hci-8.1 v8 0/6] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
` (4 preceding siblings ...)
2026-10-26 14:42 ` [Devel] [PATCH hci-8.1 v8 5/6] dm-qcow2: keep the dirty bit of an image opened dirty Alexander Ivanov
@ 2026-10-26 14:42 ` Alexander Ivanov
2026-10-28 12:41 ` Andrey Zhadchenko
5 siblings, 1 reply; 10+ messages in thread
From: Alexander Ivanov @ 2026-10-26 14:42 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. Refcount ranges with no allocated refblock
are skipped at once. 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. If the
relocation fails after the new header is written, refcounts no longer
match the header, so set md_writeback_error there.
Truncate the file to the used end. Fall back to the old behaviour on
scan error and after any failed md writeback. A failed R1/R2 write
leaves refcounts on disk stale. A failed L1/L2 write reverts the
refcount of a newly allocated cluster to zero, while L1/L2 on disk may
still point to it. Mark such an L1/L2 page with MD_LX_REVERTED and
clear the mark on its next successful writeback, which rewrites the
whole page, e.g. when a request delayed on ENOSPC is resubmitted. On
md writeback flush a still marked page sets a new lx_writeback_error
flag, which is reported in get_errors and logged when the scan is
skipped. The flag is lost on target reload, after which the scan would
trust the reverted refcount, so include it in wants_check like
md_writeback_error: resume is refused until the image is checked and
the target reloaded.
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. Otherwise userspace sets the dirty
bit on every RW open, so the bit doesn't mean the image is
inconsistent. Userspace checks the image before opening it RW, which
find_unused_cluster() already relies on when it reuses clusters with
zero refcount.
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-qcow2-cmd.c | 4 +-
drivers/md/dm-qcow2-map.c | 96 +++++++++++++++++++++++++++++++++---
drivers/md/dm-qcow2-target.c | 38 +++++++++++---
drivers/md/dm-qcow2.h | 6 ++-
4 files changed, 127 insertions(+), 17 deletions(-)
diff --git a/drivers/md/dm-qcow2-cmd.c b/drivers/md/dm-qcow2-cmd.c
index 2865940cff30..1520733c962c 100644
--- a/drivers/md/dm-qcow2-cmd.c
+++ b/drivers/md/dm-qcow2-cmd.c
@@ -20,9 +20,9 @@ static int qcow2_get_errors(struct qcow2_target *tgt, char *result,
int ret;
ret = DMEMIT("wants_check=%d\nmd_writeback_error=%d\ntruncate_error=%d\n"
- "merge_backward_error=%d\n",
+ "merge_backward_error=%d\nlx_writeback_error=%d\n",
wants_check, tgt->md_writeback_error, tgt->truncate_error,
- tgt->backward_merge.error);
+ tgt->backward_merge.error, tgt->lx_writeback_error);
return ret ? 1 : 0;
}
diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index fa63bd6acfb8..c0669dc630b8 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 */
@@ -1362,6 +1366,8 @@ static void revert_l_entries_update(struct qcow2 *qcow2, struct wb_desc *wbd)
if (cleared)
continue; /* no cluster was allocated */
+ /* L1/L2 on disk may point to the cluster */
+ wbd->md->status |= MD_LX_REVERTED;
spin_unlock(&qcow2->md_pages_lock);
pos &= ~LX_REFCOUNT_EXACTLY_ONE;
@@ -1385,6 +1391,8 @@ static void clear_writeback_status(struct qcow2 *qcow2, struct md_page *md,
lockdep_assert_held(&qcow2->md_pages_lock);
md->status &= ~(MD_WRITEBACK|MD_WRITEBACK_ERROR);
+ if (!ret)
+ md->status &= ~MD_LX_REVERTED;
list_splice_init(&md->wait_list, wait_list);
if (ret && !md->wbd) {
/*
@@ -2201,6 +2209,74 @@ int qcow2_truncate_safe(struct file *file, loff_t new_len)
return do_ftruncate(file, new_len, 0);
}
+/*
+ * Returns the end of the last cluster, which is used according
+ * to refcount table, clamped to file_preallocated_area_start since
+ * the last cluster may be partial. Clusters freed by discard may
+ * leave the tail of file unused. Everything at or above
+ * file_preallocated_area_start is unused by construction, so the scan
+ * starts below it. Failed relocate_refcount_table() leaves it beyond
+ * reftable_max_file_size, so clamp the start to the reftable coverage.
+ * This must be called on suspended device after md writeback, so
+ * nobody changes R1 and R2.
+ */
+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;
+
+ /*
+ * The scan walks R1 entries normal I/O may never touch,
+ * so don't let calc_r2_page_id() WARN on a corrupted one.
+ */
+ 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) {
+ /* No R2 cluster, so all clusters it covers are unused */
+ 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) {
+ /* Last cluster is partial if size isn't cluster-aligned */
+ 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;
@@ -2372,21 +2448,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_hdr_written */
}
spin_unlock_irq(&qcow2->md_pages_lock);
if (i != nr_pages)
- goto err_free_r2_pages;
+ goto err_hdr_written;
/* 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_hdr_written;
}
ret = handle_md_page(qcow2, r1.page_id, NULL, &md);
if (WARN_ON_ONCE(ret <= 0))
- goto err_free_r2_pages;
+ goto err_hdr_written;
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);
@@ -2397,7 +2473,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_hdr_written;
mark_cluster_used(qcow2, r2.md, r2.index_in_page);
}
@@ -2405,12 +2481,16 @@ 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_hdr_written;
mark_cluster_unused(qcow2, r2.md, r2.index_in_page, i);
}
return 1;
+err_hdr_written:
+ /* Header on disk points to new R1, refcounts may be inconsistent */
+ QC_ERR(qcow2->tgt->ti, "failed to relocate refcount table");
+ qcow2->tgt->md_writeback_error = true;
err_free_r2_pages:
for (i = end; i < r2_end; i += clu_size) {
ret = handle_md_page(qcow2, i >> PAGE_SHIFT, NULL, &md);
diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
index 2d69e39a3530..7895fea531ae 100644
--- a/drivers/md/dm-qcow2-target.c
+++ b/drivers/md/dm-qcow2-target.c
@@ -170,6 +170,7 @@ static void qcow2_free_md_pages_tree(struct rb_root *root)
void qcow2_flush_deferred_activity(struct qcow2_target *tgt, struct qcow2 *qcow2)
{
struct rb_node *node;
+ bool wb_error = false;
struct md_page *md;
int i;
@@ -191,13 +192,17 @@ void qcow2_flush_deferred_activity(struct qcow2_target *tgt, struct qcow2 *qcow2
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;
- break;
- }
+ if (md->status & MD_WRITEBACK_ERROR)
+ wb_error = true;
+ if (md->status & MD_LX_REVERTED)
+ tgt->lx_writeback_error = true;
}
spin_unlock_irq(&qcow2->md_pages_lock);
+
+ if (wb_error) {
+ QC_ERR(tgt->ti, "Failed to write dirty pages");
+ tgt->md_writeback_error = true;
+ }
}
static void qcow2_flush_deferred_activity_all(struct qcow2_target *tgt)
@@ -1058,11 +1063,32 @@ static void qcow2_truncate_preallocations(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;
+
+ /*
+ * Refcounts don't match L1/L2 on disk after failed md writeback.
+ * They may also be below real usage in an image opened dirty,
+ * e.g. after a crash. If userspace sets the dirty bit, it is set
+ * on every RW open and userspace checks the image before that.
+ */
+ if (tgt->md_writeback_error) {
+ QC_INFO(ti, "Skip shrinking to used end after md writeback error");
+ } else if (tgt->lx_writeback_error) {
+ QC_INFO(ti, "Skip shrinking to used end after reverted L1/L2 writeback");
+ } 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;
diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
index a2d951ee5541..7db2d9301cad 100644
--- a/drivers/md/dm-qcow2.h
+++ b/drivers/md/dm-qcow2.h
@@ -134,6 +134,7 @@ struct md_page {
#define MD_DIRTY (1U << 1) /* Page contains changes and wants writeback */
#define MD_WRITEBACK (1U << 2) /* Writeback was submitted */
#define MD_WRITEBACK_ERROR (1U << 3) /* Last writeback failed with error */
+#define MD_LX_REVERTED (1U << 4) /* L1/L2 on disk may point to freed cluster */
unsigned int status;
struct page *page;
struct list_head wait_list;
@@ -200,6 +201,7 @@ struct qcow2_target {
bool service_operations_allowed;
bool wants_suspend;
bool md_writeback_error;
+ bool lx_writeback_error;
bool truncate_error;
bool event_enospc;
@@ -413,6 +415,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);
@@ -461,7 +464,8 @@ static inline bool fake_service_qio(struct qio *qio)
static inline bool qcow2_wants_check(struct qcow2_target *tgt)
{
- return !!(tgt->md_writeback_error|tgt->truncate_error);
+ return !!(tgt->md_writeback_error|tgt->lx_writeback_error|
+ tgt->truncate_error);
}
static inline bool qcow2_file_is_writable(struct qcow2 *qcow2)
--
2.53.0
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Devel] [PATCH hci-8.1 v8 6/6] dm-qcow2: truncate unused tail of image on suspend
2026-10-26 14:42 ` [Devel] [PATCH hci-8.1 v8 6/6] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
@ 2026-10-28 12:41 ` Andrey Zhadchenko
2026-10-28 15:08 ` Alexander Ivanov
0 siblings, 1 reply; 10+ messages in thread
From: Andrey Zhadchenko @ 2026-10-28 12:41 UTC (permalink / raw)
To: Alexander Ivanov; +Cc: devel
Patch should be split.
l1/l2 revert tracking and relocation error are pre-existing problems and
should be separated.
On 10/26/26 15:42, Alexander Ivanov wrote:
> 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. Refcount ranges with no allocated refblock
> are skipped at once. 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. If the
> relocation fails after the new header is written, refcounts no longer
> match the header, so set md_writeback_error there.
>
> Truncate the file to the used end. Fall back to the old behaviour on
> scan error and after any failed md writeback. A failed R1/R2 write
> leaves refcounts on disk stale. A failed L1/L2 write reverts the
> refcount of a newly allocated cluster to zero, while L1/L2 on disk may
> still point to it. Mark such an L1/L2 page with MD_LX_REVERTED and
> clear the mark on its next successful writeback, which rewrites the
> whole page, e.g. when a request delayed on ENOSPC is resubmitted. On
> md writeback flush a still marked page sets a new lx_writeback_error
> flag, which is reported in get_errors and logged when the scan is
> skipped. The flag is lost on target reload, after which the scan would
> trust the reverted refcount, so include it in wants_check like
> md_writeback_error: resume is refused until the image is checked and
> the target reloaded.
>
> 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. Otherwise userspace sets the dirty
> bit on every RW open, so the bit doesn't mean the image is
> inconsistent. Userspace checks the image before opening it RW, which
> find_unused_cluster() already relies on when it reuses clusters with
> zero refcount.
>
> 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-qcow2-cmd.c | 4 +-
> drivers/md/dm-qcow2-map.c | 96 +++++++++++++++++++++++++++++++++---
> drivers/md/dm-qcow2-target.c | 38 +++++++++++---
> drivers/md/dm-qcow2.h | 6 ++-
> 4 files changed, 127 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/md/dm-qcow2-cmd.c b/drivers/md/dm-qcow2-cmd.c
> index 2865940cff30..1520733c962c 100644
> --- a/drivers/md/dm-qcow2-cmd.c
> +++ b/drivers/md/dm-qcow2-cmd.c
> @@ -20,9 +20,9 @@ static int qcow2_get_errors(struct qcow2_target *tgt, char *result,
> int ret;
>
> ret = DMEMIT("wants_check=%d\nmd_writeback_error=%d\ntruncate_error=%d\n"
> - "merge_backward_error=%d\n",
> + "merge_backward_error=%d\nlx_writeback_error=%d\n",
> wants_check, tgt->md_writeback_error, tgt->truncate_error,
> - tgt->backward_merge.error);
> + tgt->backward_merge.error, tgt->lx_writeback_error);
>
> return ret ? 1 : 0;
> }
> diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
> index fa63bd6acfb8..c0669dc630b8 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 */
> @@ -1362,6 +1366,8 @@ static void revert_l_entries_update(struct qcow2 *qcow2, struct wb_desc *wbd)
> if (cleared)
> continue; /* no cluster was allocated */
>
> + /* L1/L2 on disk may point to the cluster */
> + wbd->md->status |= MD_LX_REVERTED;
> spin_unlock(&qcow2->md_pages_lock);
> pos &= ~LX_REFCOUNT_EXACTLY_ONE;
>
> @@ -1385,6 +1391,8 @@ static void clear_writeback_status(struct qcow2 *qcow2, struct md_page *md,
> lockdep_assert_held(&qcow2->md_pages_lock);
>
> md->status &= ~(MD_WRITEBACK|MD_WRITEBACK_ERROR);
> + if (!ret)
> + md->status &= ~MD_LX_REVERTED;
> list_splice_init(&md->wait_list, wait_list);
> if (ret && !md->wbd) {
> /*
> @@ -2201,6 +2209,74 @@ int qcow2_truncate_safe(struct file *file, loff_t new_len)
> return do_ftruncate(file, new_len, 0);
> }
>
> +/*
> + * Returns the end of the last cluster, which is used according
> + * to refcount table, clamped to file_preallocated_area_start since
> + * the last cluster may be partial. Clusters freed by discard may
> + * leave the tail of file unused. Everything at or above
> + * file_preallocated_area_start is unused by construction, so the scan
> + * starts below it. Failed relocate_refcount_table() leaves it beyond
> + * reftable_max_file_size, so clamp the start to the reftable coverage.
> + * This must be called on suspended device after md writeback, so
> + * nobody changes R1 and R2.
> + */
> +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;
> +
> + /*
> + * The scan walks R1 entries normal I/O may never touch,
> + * so don't let calc_r2_page_id() WARN on a corrupted one.
> + */
> + 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) {
> + /* No R2 cluster, so all clusters it covers are unused */
> + 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) {
> + /* Last cluster is partial if size isn't cluster-aligned */
> + 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;
> @@ -2372,21 +2448,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_hdr_written */
> }
> spin_unlock_irq(&qcow2->md_pages_lock);
> if (i != nr_pages)
> - goto err_free_r2_pages;
> + goto err_hdr_written;
>
> /* 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_hdr_written;
> }
> ret = handle_md_page(qcow2, r1.page_id, NULL, &md);
> if (WARN_ON_ONCE(ret <= 0))
> - goto err_free_r2_pages;
> + goto err_hdr_written;
> 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);
> @@ -2397,7 +2473,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_hdr_written;
> mark_cluster_used(qcow2, r2.md, r2.index_in_page);
> }
>
> @@ -2405,12 +2481,16 @@ 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_hdr_written;
> mark_cluster_unused(qcow2, r2.md, r2.index_in_page, i);
> }
>
> return 1;
>
> +err_hdr_written:
> + /* Header on disk points to new R1, refcounts may be inconsistent */
> + QC_ERR(qcow2->tgt->ti, "failed to relocate refcount table");
> + qcow2->tgt->md_writeback_error = true;
> err_free_r2_pages:
> for (i = end; i < r2_end; i += clu_size) {
> ret = handle_md_page(qcow2, i >> PAGE_SHIFT, NULL, &md);
> diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
> index 2d69e39a3530..7895fea531ae 100644
> --- a/drivers/md/dm-qcow2-target.c
> +++ b/drivers/md/dm-qcow2-target.c
> @@ -170,6 +170,7 @@ static void qcow2_free_md_pages_tree(struct rb_root *root)
> void qcow2_flush_deferred_activity(struct qcow2_target *tgt, struct qcow2 *qcow2)
> {
> struct rb_node *node;
> + bool wb_error = false;
> struct md_page *md;
> int i;
>
> @@ -191,13 +192,17 @@ void qcow2_flush_deferred_activity(struct qcow2_target *tgt, struct qcow2 *qcow2
> 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;
> - break;
> - }
> + if (md->status & MD_WRITEBACK_ERROR)
> + wb_error = true;
> + if (md->status & MD_LX_REVERTED)
> + tgt->lx_writeback_error = true;
> }
> spin_unlock_irq(&qcow2->md_pages_lock);
> +
> + if (wb_error) {
> + QC_ERR(tgt->ti, "Failed to write dirty pages");
> + tgt->md_writeback_error = true;
> + }
> }
>
> static void qcow2_flush_deferred_activity_all(struct qcow2_target *tgt)
> @@ -1058,11 +1063,32 @@ static void qcow2_truncate_preallocations(struct dm_target *ti)
qcow2_truncate_preallocations should be renamed. It does much more now.
> {
> 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;
> +
> + /*
> + * Refcounts don't match L1/L2 on disk after failed md writeback.
> + * They may also be below real usage in an image opened dirty,
> + * e.g. after a crash. If userspace sets the dirty bit, it is set
> + * on every RW open and userspace checks the image before that.
> + */
Useless comment that just repeats what code does. Why?
> + if (tgt->md_writeback_error) {
> + QC_INFO(ti, "Skip shrinking to used end after md writeback error");
> + } else if (tgt->lx_writeback_error) {
> + QC_INFO(ti, "Skip shrinking to used end after reverted L1/L2 writeback");
> + } 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;
>
> diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
> index a2d951ee5541..7db2d9301cad 100644
> --- a/drivers/md/dm-qcow2.h
> +++ b/drivers/md/dm-qcow2.h
> @@ -134,6 +134,7 @@ struct md_page {
> #define MD_DIRTY (1U << 1) /* Page contains changes and wants writeback */
> #define MD_WRITEBACK (1U << 2) /* Writeback was submitted */
> #define MD_WRITEBACK_ERROR (1U << 3) /* Last writeback failed with error */
> +#define MD_LX_REVERTED (1U << 4) /* L1/L2 on disk may point to freed cluster */
> unsigned int status;
> struct page *page;
> struct list_head wait_list;
> @@ -200,6 +201,7 @@ struct qcow2_target {
> bool service_operations_allowed;
> bool wants_suspend;
> bool md_writeback_error;
> + bool lx_writeback_error;
> bool truncate_error;
> bool event_enospc;
>
> @@ -413,6 +415,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);
> @@ -461,7 +464,8 @@ static inline bool fake_service_qio(struct qio *qio)
> static inline bool qcow2_wants_check(struct qcow2_target *tgt)
> {
>
> - return !!(tgt->md_writeback_error|tgt->truncate_error);
> + return !!(tgt->md_writeback_error|tgt->lx_writeback_error|
> + tgt->truncate_error);
> }
>
> static inline bool qcow2_file_is_writable(struct qcow2 *qcow2)
I would really advise againt commenting every action. The following
example is the worst of the worst.
/* If we have an error skip shrinking */
If (error)
skip_shrinking()
Please read Documentation/process/coding-style.rst section 8 (or have
your agent read it). I am not planning to go on each comment in this
patch, please do the pruning yourself.
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
^ permalink raw reply [flat|nested] 10+ messages in thread