On 27/10/2026 11:44, Andrey Drobyshev wrote:
Whenever option discard-no-unref=on is enabled, we bypass refcount
machinery and unconditionally punch hole in the data on 'discard' and
'write -z -u' paths. However, when this same cluster is also referenced
by an internal snapshot, such operations lead to problems. 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.
Fix this by guarding the 'keep_reference' flag by the presence of the
QCOW_OFLAG_COPIED bit. I.e. only proceed with unconditional data
punching if we own this cluster exclusively.
Fixes: 42a2890a76f4 ("qcow2: add discard-no-unref option")
Signed-off-by: Andrey Drobyshev <[email protected]>
---
block/qcow2-cluster.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index e02fae6a0c6..cc473c7c796 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1953,6 +1953,7 @@ discard_in_l2_slice(BlockDriverState *bs, uint64_t
offset, uint64_t nb_clusters,
QCow2ClusterType cluster_type =
qcow2_get_cluster_type(bs, old_l2_entry);
bool keep_reference = (cluster_type != QCOW2_CLUSTER_COMPRESSED) &&
+ (old_l2_entry & QCOW_OFLAG_COPIED) &&
!full_discard &&
(s->discard_no_unref &&
type == QCOW2_DISCARD_REQUEST);
Wouldn't it be better to gate only qcow2_discard_cluster when the
cluster is QCOW_OFLAG_COPIED?
Cause with the above change, we fully clear/remove the offset of the
cluster, causing fragmentation again in the image afterwards.
And that was just the goal for discard-no-unref :)
@@ -2097,8 +2098,9 @@ zero_in_l2_slice(BlockDriverState *bs, uint64_t offset,
QCow2ClusterType type = qcow2_get_cluster_type(bs, old_l2_entry);
bool unmap = (type == QCOW2_CLUSTER_COMPRESSED) ||
((flags & BDRV_REQ_MAY_UNMAP) &&
qcow2_cluster_is_allocated(type));
- bool keep_reference =
- (s->discard_no_unref && type != QCOW2_CLUSTER_COMPRESSED);
+ bool keep_reference = s->discard_no_unref &&
+ type != QCOW2_CLUSTER_COMPRESSED &&
+ (old_l2_entry & QCOW_OFLAG_COPIED);
uint64_t new_l2_entry = old_l2_entry;
uint64_t new_l2_bitmap = old_l2_bitmap;
And same here then.
Thanks
Jean-Louis
