[PATCH hci-8.1 0/1] qcow2: fix guest TRIM destroying internal snapshot data #VSTOR-144873

[PATCH hci-8.1 0/1] qcow2: fix guest TRIM destroying internal snapshot data #VSTOR-144873

[Denis V. Lunev]

With extended_l2=on, a discard of part of a cluster that an internal
snapshot still references punches the host range under it. The snapshot
then reads zeroes there, and qemu-img check finds nothing wrong. Every
VM disk has discard='unmap' and the image cache uses cluster_size=1M,
extended_l2=on, so a guest TRIM after a snapshot is enough: CI reverts
gave Windows guests partially zeroed files and left SLES btrfs guests
unbootable.

The bug comes with our "qcow2: make subclusters discardable" series,
which is not upstream. The patch passes the discard to the host only
for clusters owned by the active layer alone, and adds a case to
iotest 271.

iotest 271 on dev-cl04 fails without the fix (the snapshot reads zeroes
after revert) and passes with it.

Signed-off-by: Denis V. Lunev <den@openvz.org>

Denis V. Lunev (1):
  qcow2: fix subcluster discard punching data shared with a snapshot
    #VSTOR-144873

 block/qcow2-cluster.c      | 10 +++++++---
 tests/qemu-iotests/271     | 18 ++++++++++++++++++
 tests/qemu-iotests/271.out |  5 +++++
 3 files changed, 30 insertions(+), 3 deletions(-)


base-commit: 9ad1f5893c80aa612f78cb40c4c8690097ebb9e9
-- 
2.53.0

[PATCH hci-8.1 1/1] qcow2: fix subcluster discard punching data shared with a snapshot #VSTOR-144873

[Denis V. Lunev]

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

discard_l2_subclusters() passes the discarded range down to the host
file even when the cluster is still referenced by an internal snapshot.
The active L2 table is copied before it is changed, so the snapshot
metadata stays correct, but the host range under it is punched out and
the snapshot reads zeroes there afterwards. qemu-img check finds nothing
wrong.

Pass the discard down only for a cluster with QCOW_OFLAG_COPIED set,
i.e. one owned by the active layer alone. A shared cluster gets only
its active L2 bitmap updated, as a whole-cluster discard already does
by dropping the refcount instead of punching the host file.

Cover it in iotest 271.

Fixes: 225b4de1249 ("qcow2: make subclusters discardable")
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 block/qcow2-cluster.c      | 10 +++++++---
 tests/qemu-iotests/271     | 18 ++++++++++++++++++
 tests/qemu-iotests/271.out |  5 +++++
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index e40995904b5..dfe728cd8a5 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -2196,9 +2196,13 @@ discard_l2_subclusters(BlockDriverState *bs, uint64_t offset,
         qcow2_cache_entry_mark_dirty(s->l2_table_cache, scri.l2_slice);
     }
 
-    discard_no_unref_any_file(
-        bs, (scri.l2_entry & L2E_OFFSET_MASK) + offset_into_cluster(s, offset),
-        nb_subclusters * s->subcluster_size, scri.ctype, type);
+    /* A cluster shared with a snapshot must keep its data */
+    if (scri.l2_entry & QCOW_OFLAG_COPIED) {
+        discard_no_unref_any_file(
+            bs,
+            (scri.l2_entry & L2E_OFFSET_MASK) + offset_into_cluster(s, offset),
+            nb_subclusters * s->subcluster_size, scri.ctype, type);
+    }
 
     return 0;
 }
diff --git a/tests/qemu-iotests/271 b/tests/qemu-iotests/271
index 026a40cdaed..2f60fb5fc96 100755
--- a/tests/qemu-iotests/271
+++ b/tests/qemu-iotests/271
@@ -605,6 +605,24 @@ done
 ############################################################
 ############################################################
 
+echo
+echo "### Discarding subclusters shared with an internal snapshot ###"
+echo
+
+_make_test_img -o extended_l2=on 1M
+$QEMU_IO -c "write -q -P 0xab 0 64k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -c snap "$TEST_IMG"
+$QEMU_IO -c "discard -q 0 4k" -c "write -q -z -u 8k 4k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -q -P 0 0 4k" -c "read -q -P 0xab 4k 4k" \
+         -c "read -q -P 0 8k 4k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -a snap "$TEST_IMG"
+$QEMU_IO -c "read -q -P 0xab 0 64k" "$TEST_IMG" | _filter_qemu_io
+_check_test_img
+
+############################################################
+############################################################
+############################################################
+
 # Test that corrupted L2 entries are detected in both read and write
 # operations
 for corruption_test_cmd in read write; do
diff --git a/tests/qemu-iotests/271.out b/tests/qemu-iotests/271.out
index c03e4c9bc2a..e7970d33516 100644
--- a/tests/qemu-iotests/271.out
+++ b/tests/qemu-iotests/271.out
@@ -504,6 +504,11 @@ Image resized.
 Image resized.
 L2 entry #1: 0x0000000000000000 0000000000000000
 
+### Discarding subclusters shared with an internal snapshot ###
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
+No errors were found on the image.
+
 ### Corrupted L2 entries - read test (allocated) ###
 
 # 'cluster is zero' bit set on the standard cluster descriptor
-- 
2.53.0

Re: [PATCH hci-8.1 1/1] qcow2: fix subcluster discard punching data shared with a snapshot #VSTOR-144873

[Andrey Drobyshev]

On 10/26/26 9:37 PM, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
> 
> discard_l2_subclusters() passes the discarded range down to the host
> file even when the cluster is still referenced by an internal snapshot.
> The active L2 table is copied before it is changed, so the snapshot
> metadata stays correct, but the host range under it is punched out and
> the snapshot reads zeroes there afterwards. qemu-img check finds nothing
> wrong.
> 
> Pass the discard down only for a cluster with QCOW_OFLAG_COPIED set,
> i.e. one owned by the active layer alone. A shared cluster gets only
> its active L2 bitmap updated, as a whole-cluster discard already does
> by dropping the refcount instead of punching the host file.
> 
> Cover it in iotest 271.
> 
> Fixes: 225b4de1249 ("qcow2: make subclusters discardable")
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>  [...]
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>