On 20.10.23 23:56, Andrey Drobyshev wrote:
This commit makes the discard operation work on the subcluster level
rather than cluster level. It introduces discard_l2_subclusters()
function and makes use of it in qcow2 discard implementation, much like
it's done with zero_in_l2_slice() / zero_l2_subclusters(). It also
changes the qcow2 driver pdiscard_alignment to subcluster_size. That
way subcluster-aligned discards lead to actual fallocate(PUNCH_HOLE)
operation and free host disk space.
This feature will let us gain additional disk space on guest
TRIM/discard requests, especially when using large enough clusters
(1M, 2M) with subclusters enabled.
Signed-off-by: Andrey Drobyshev <andrey.drobys...@virtuozzo.com>
---
block/qcow2-cluster.c | 100 ++++++++++++++++++++++++++++++++++++++++--
block/qcow2.c | 8 ++--
2 files changed, 101 insertions(+), 7 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 7c6fa5524c..cf40f2dc12 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
[...]
+ if (scri.l2_bitmap != new_l2_bitmap) {
+ set_l2_bitmap(s, scri.l2_slice, scri.l2_index, new_l2_bitmap);
+ qcow2_cache_entry_mark_dirty(s->l2_table_cache, scri.l2_slice);
+ }
+
+ if (s->discard_passthrough[type]) {
+ qcow2_queue_discard(bs, (scri.l2_entry & L2E_OFFSET_MASK) +
+ offset_into_cluster(s, offset),
+ nb_subclusters * s->subcluster_size);
Are we sure that the cluster is allocated, i.e. that scri.l2_entry &
L2E_OFFSET_MASK != 0?
As a side note, I guess discard_in_l2_slice() should also use
qcow2_queue_discard() instead of bdrv_pdiscard() then.
+ }
+
+ ret = 0;
+out:
+ qcow2_cache_put(s->l2_table_cache, (void **) &scri.l2_slice);
+
+ return ret;
+}
+
int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, enum qcow2_discard_type type,
bool full_discard)
@@ -2049,19 +2117,36 @@ int qcow2_cluster_discard(BlockDriverState *bs,
uint64_t offset,
BDRVQcow2State *s = bs->opaque;
uint64_t end_offset = offset + bytes;
uint64_t nb_clusters;
+ unsigned head, tail;
int64_t cleared;
int ret;
/* Caller must pass aligned values, except at image end */
- assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
- assert(QEMU_IS_ALIGNED(end_offset, s->cluster_size) ||
+ assert(QEMU_IS_ALIGNED(offset, s->subcluster_size));
+ assert(QEMU_IS_ALIGNED(end_offset, s->subcluster_size) ||
end_offset == bs->total_sectors << BDRV_SECTOR_BITS);
- nb_clusters = size_to_clusters(s, bytes);
+ head = MIN(end_offset, ROUND_UP(offset, s->cluster_size)) - offset;
+ offset += head;
+
+ tail = (end_offset >= bs->total_sectors << BDRV_SECTOR_BITS) ? 0 :
+ end_offset - MAX(offset, start_of_cluster(s, end_offset));
+ end_offset -= tail;
s->cache_discards = true;
+ if (head) {
+ ret = discard_l2_subclusters(bs, offset - head,
+ size_to_subclusters(s, head), type,
+ full_discard, NULL);
+ if (ret < 0) {
+ goto fail;
+ }
+ }
+
/* Each L2 slice is handled by its own loop iteration */
+ nb_clusters = size_to_clusters(s, end_offset - offset);
+
while (nb_clusters > 0) {
I think the comment should stay attached to the `while`.
Hanna
cleared = discard_in_l2_slice(bs, offset, nb_clusters, type,
full_discard);
