On 15.05.23 09:36, Jean-Louis Dupond wrote:
When we for example have a sparse qcow2 image and discard: unmap is
enabled,
there can be a lot of fragmentation in the image after some time.
Surely on VM's
s/. Surely/, especially/
that do a lot of writes/deletes.
This causes the qcow2 image to grow even over 110% of its virtual size,
because the free gaps in the image get to small to allocate new
s/to small/too small/
continuous clusters. So it allocates new space as the end of the image.
s/as/at/
Disabling discard is not an option, as discard is needed to keep the
incremental backup size as low as possible. Without discard, the
incremental backups would become large, as qemu thinks it's just dirty
blocks but it doesn't know the blocks are empty/useless.
So we need to avoid fragmentation but also 'empty' the useless blocks in
s/useless/unneeded/ in both lines?
the image to have a small incremental backup.
Next to that we also want to send the discards futher down the stack, so
s/Next to that/In addition/, s/futher/further/
the underlying blocks are still discarded.
Therefor we introduce a new qcow2 option "discard-no-unref". When
setting this option to true (defaults to false), the discard requests
will still be executed, but it will keep the offset of the cluster. And
it will also pass the discard request further down the stack (if
discard:unmap is enabled).
I think this could be more explicit, e.g. “When setting this option to
true, discards will no longer have the qcow2 driver relinquish cluster
allocations. Other than that, the request is handled as normal: All
clusters in range are marked as zero, and, if pass-discard-request is
true, it is passed further down the stack. The only difference is that
the now-zero clusters are preallocated instead of being unallocated.”
This will avoid fragmentation and for example on a fully preallocated
qcow2 image, this will make sure the image is perfectly continuous.
Well, on the qcow2 layer, yes.
+ new_l2_entry = old_l2_entry;
+ } else {
+ new_l2_entry = 0;
+ }
new_l2_bitmap = QCOW_L2_BITMAP_ALL_ZEROES;
} else {
- new_l2_entry = s->qcow_version >= 3 ?
QCOW_OFLAG_ZERO : 0;
+ if (s->qcow_version >= 3) {
+ if (s->discard_no_unref && (type &
QCOW2_DISCARD_REQUEST)) {
+ new_l2_entry |= QCOW_OFLAG_ZERO;
+ } else {
+ new_l2_entry = QCOW_OFLAG_ZERO;
+ }
+ } else {
+ new_l2_entry = 0;
+ }
}
}
Context below:
if (old_l2_entry == new_l2_entry && old_l2_bitmap ==
new_l2_bitmap) {
continue;
}
/* First remove L2 entries */
qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
set_l2_entry(s, l2_slice, l2_index + i, new_l2_entry);
if (has_subclusters(s)) {
set_l2_bitmap(s, l2_slice, l2_index + i, new_l2_bitmap);
}
/* Then decrease the refcount */
qcow2_free_any_cluster(bs, old_l2_entry, type);
If we keep the allocation, I don’t see why we would call
qcow2_free_any_cluster(). If we simply skip the call (if
`qcow2_is_allocated(qcow2_get_cluster_type(bs, new_l2_entry))`), I
think you could drop the modification to update_refcount().
[...]
diff --git a/block/qcow2.c b/block/qcow2.c
index 5bde3b8401..9dde2ac1a5 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
[...]
@@ -725,6 +726,11 @@ static QemuOptsList qcow2_runtime_opts = {
.type = QEMU_OPT_BOOL,
.help = "Generate discard requests when other clusters
are freed",
},
+ {
+ .name = QCOW2_OPT_DISCARD_NO_UNREF,
+ .type = QEMU_OPT_BOOL,
+ .help = "Do not dereference discarded clusters",
I wouldn’t call it “dereference” because of the overloaded meaning in
C, but “unreference” instead.
+#
# @overlap-check: which overlap checks to perform for writes to the
# image, defaults to 'cached' (since 2.2)
#
@@ -3470,6 +3473,7 @@
'*pass-discard-request': 'bool',
'*pass-discard-snapshot': 'bool',
'*pass-discard-other': 'bool',
+ '*discard-no-unref': 'bool',
'*overlap-check': 'Qcow2OverlapChecks',
'*cache-size': 'int',
'*l2-cache-size': 'int',
diff --git a/qemu-options.hx b/qemu-options.hx
index 42b9094c10..17ac701d0d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1431,6 +1431,12 @@ SRST
issued on other occasions where a cluster gets freed
(on/off; default: off)
+ ``discard-no-unref``
+ When enabled, a discard in the guest does not cause the
+ cluster inside the qcow2 image to be dereferenced. This
Like above, I’d prefer “unreferenced”, or “the cluster’s allocation
[…] to be relinquished”.