* [PATCH v2 00/11] qcow2: make subclusters discardable
@ 2024-05-13 6:31 Andrey Drobyshev
2024-05-13 6:31 ` [PATCH v2 01/11] qcow2: make function update_refcount_discard() global Andrey Drobyshev
` (11 more replies)
0 siblings, 12 replies; 34+ messages in thread
From: Andrey Drobyshev @ 2024-05-13 6:31 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis,
andrey.drobyshev, den
v1: https://lists.nongnu.org/archive/html/qemu-devel/2023-10/msg07223.html
Andrey Drobyshev (11):
qcow2: make function update_refcount_discard() global
qcow2: simplify L2 entries accounting for discard-no-unref
qcow2: put discard requests in the common queue when discard-no-unref
enabled
block/file-posix: add trace event for fallocate() calls
iotests/common.rc: add disk_usage function
iotests/290: add test case to check 'discard-no-unref' option behavior
qcow2: add get_sc_range_info() helper for working with subcluster
ranges
qcow2: zeroize the entire cluster when there're no non-zero
subclusters
qcow2: make subclusters discardable
qcow2: zero_l2_subclusters: fall through to discard operation when
requested
iotests/271: add test cases for subcluster-based discard/unmap
block/file-posix.c | 1 +
block/qcow2-cluster.c | 346 ++++++++++++++++++++++++++++-------
block/qcow2-refcount.c | 8 +-
block/qcow2-snapshot.c | 6 +-
block/qcow2.c | 25 +--
block/qcow2.h | 6 +-
block/trace-events | 1 +
tests/qemu-iotests/250 | 5 -
tests/qemu-iotests/271 | 72 ++++++--
tests/qemu-iotests/271.out | 69 ++++++-
tests/qemu-iotests/290 | 34 ++++
tests/qemu-iotests/290.out | 28 +++
tests/qemu-iotests/common.rc | 6 +
13 files changed, 490 insertions(+), 117 deletions(-)
--
2.39.3
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 01/11] qcow2: make function update_refcount_discard() global
2024-05-13 6:31 [PATCH v2 00/11] qcow2: make subclusters discardable Andrey Drobyshev
@ 2024-05-13 6:31 ` Andrey Drobyshev
2024-05-15 22:26 ` Alberto Garcia
2024-05-21 8:31 ` Alexander Ivanov
2024-05-13 6:31 ` [PATCH v2 02/11] qcow2: simplify L2 entries accounting for discard-no-unref Andrey Drobyshev
` (10 subsequent siblings)
11 siblings, 2 replies; 34+ messages in thread
From: Andrey Drobyshev @ 2024-05-13 6:31 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis,
andrey.drobyshev, den
We are going to need it for discarding separate subclusters. The
function itself doesn't do anything with the refcount tables, it simply
adds a discard request to the queue, so rename it to qcow2_queue_discard().
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
---
block/qcow2-refcount.c | 8 ++++----
block/qcow2.h | 2 ++
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 0266542cee..2026f5fa21 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -754,8 +754,8 @@ void qcow2_process_discards(BlockDriverState *bs, int ret)
}
}
-static void update_refcount_discard(BlockDriverState *bs,
- uint64_t offset, uint64_t length)
+void qcow2_queue_discard(BlockDriverState *bs, uint64_t offset,
+ uint64_t length)
{
BDRVQcow2State *s = bs->opaque;
Qcow2DiscardRegion *d, *p, *next;
@@ -902,7 +902,7 @@ update_refcount(BlockDriverState *bs, int64_t offset, int64_t length,
}
if (s->discard_passthrough[type]) {
- update_refcount_discard(bs, cluster_offset, s->cluster_size);
+ qcow2_queue_discard(bs, cluster_offset, s->cluster_size);
}
}
}
@@ -3619,7 +3619,7 @@ qcow2_discard_refcount_block(BlockDriverState *bs, uint64_t discard_block_offs)
/* discard refblock from the cache if refblock is cached */
qcow2_cache_discard(s->refcount_block_cache, refblock);
}
- update_refcount_discard(bs, discard_block_offs, s->cluster_size);
+ qcow2_queue_discard(bs, discard_block_offs, s->cluster_size);
return 0;
}
diff --git a/block/qcow2.h b/block/qcow2.h
index a9e3481c6e..197bdcdf53 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -891,6 +891,8 @@ int coroutine_fn qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *re
BdrvCheckMode fix);
void GRAPH_RDLOCK qcow2_process_discards(BlockDriverState *bs, int ret);
+void qcow2_queue_discard(BlockDriverState *bs, uint64_t offset,
+ uint64_t length);
int GRAPH_RDLOCK
qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset,
--
2.39.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v2 02/11] qcow2: simplify L2 entries accounting for discard-no-unref
2024-05-13 6:31 [PATCH v2 00/11] qcow2: make subclusters discardable Andrey Drobyshev
2024-05-13 6:31 ` [PATCH v2 01/11] qcow2: make function update_refcount_discard() global Andrey Drobyshev
@ 2024-05-13 6:31 ` Andrey Drobyshev
2024-05-15 22:28 ` Alberto Garcia
2024-05-21 8:32 ` Alexander Ivanov
2024-05-13 6:31 ` [PATCH v2 03/11] qcow2: put discard requests in the common queue when discard-no-unref enabled Andrey Drobyshev
` (9 subsequent siblings)
11 siblings, 2 replies; 34+ messages in thread
From: Andrey Drobyshev @ 2024-05-13 6:31 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis,
andrey.drobyshev, den
Commits 42a2890a and b2b10904 introduce handling of discard-no-unref
option in discard_in_l2_slice() and zero_in_l2_slice(). They add even
more if's when chosing the right l2 entry. What we really need for this
option is the new entry simply to contain the same host cluster offset,
no matter whether we unmap or zeroize the cluster. For that OR'ing with
the old entry is enough.
This patch doesn't change the logic and is pure refactoring.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/qcow2-cluster.c | 34 +++++++++++++++-------------------
1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index ce8c0076b3..5f057ba2fd 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1946,25 +1946,21 @@ discard_in_l2_slice(BlockDriverState *bs, uint64_t offset, uint64_t nb_clusters,
new_l2_entry = new_l2_bitmap = 0;
} else if (bs->backing || qcow2_cluster_is_allocated(cluster_type)) {
if (has_subclusters(s)) {
- if (keep_reference) {
- new_l2_entry = old_l2_entry;
- } else {
- new_l2_entry = 0;
- }
+ new_l2_entry = 0;
new_l2_bitmap = QCOW_L2_BITMAP_ALL_ZEROES;
} else {
- if (s->qcow_version >= 3) {
- if (keep_reference) {
- new_l2_entry |= QCOW_OFLAG_ZERO;
- } else {
- new_l2_entry = QCOW_OFLAG_ZERO;
- }
- } else {
- new_l2_entry = 0;
- }
+ new_l2_entry = s->qcow_version >= 3 ? QCOW_OFLAG_ZERO : 0;
}
}
+ /*
+ * No need to check for the QCOW version since discard-no-unref is
+ * only allowed since version 3.
+ */
+ if (keep_reference) {
+ new_l2_entry |= old_l2_entry;
+ }
+
if (old_l2_entry == new_l2_entry && old_l2_bitmap == new_l2_bitmap) {
continue;
}
@@ -2064,19 +2060,19 @@ zero_in_l2_slice(BlockDriverState *bs, uint64_t offset,
((flags & BDRV_REQ_MAY_UNMAP) && qcow2_cluster_is_allocated(type));
bool keep_reference =
(s->discard_no_unref && type != QCOW2_CLUSTER_COMPRESSED);
- uint64_t new_l2_entry = old_l2_entry;
+ uint64_t new_l2_entry = unmap ? 0 : old_l2_entry;
uint64_t new_l2_bitmap = old_l2_bitmap;
- if (unmap && !keep_reference) {
- new_l2_entry = 0;
- }
-
if (has_subclusters(s)) {
new_l2_bitmap = QCOW_L2_BITMAP_ALL_ZEROES;
} else {
new_l2_entry |= QCOW_OFLAG_ZERO;
}
+ if (keep_reference) {
+ new_l2_entry |= old_l2_entry;
+ }
+
if (old_l2_entry == new_l2_entry && old_l2_bitmap == new_l2_bitmap) {
continue;
}
--
2.39.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v2 03/11] qcow2: put discard requests in the common queue when discard-no-unref enabled
2024-05-13 6:31 [PATCH v2 00/11] qcow2: make subclusters discardable Andrey Drobyshev
2024-05-13 6:31 ` [PATCH v2 01/11] qcow2: make function update_refcount_discard() global Andrey Drobyshev
2024-05-13 6:31 ` [PATCH v2 02/11] qcow2: simplify L2 entries accounting for discard-no-unref Andrey Drobyshev
@ 2024-05-13 6:31 ` Andrey Drobyshev
2024-05-21 8:43 ` Alexander Ivanov
2024-05-13 6:31 ` [PATCH v2 04/11] block/file-posix: add trace event for fallocate() calls Andrey Drobyshev
` (8 subsequent siblings)
11 siblings, 1 reply; 34+ messages in thread
From: Andrey Drobyshev @ 2024-05-13 6:31 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis,
andrey.drobyshev, den
Normally discard requests are stored in the queue attached to BDRVQcow2State
to be processed later at once. Currently discard-no-unref option handling
causes these requests to be processed straight away. Let's fix that.
Note that when doing regular discards qcow2_free_any_cluster() would check
for the presence of external data files for us and redirect request to
underlying data_file. Here we want to do the same but avoid refcount updates,
thus we perform the same checks.
Suggested-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/qcow2-cluster.c | 39 +++++++++++++++++++++++++++++----------
1 file changed, 29 insertions(+), 10 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 5f057ba2fd..7dff0bd5a1 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1893,6 +1893,28 @@ again:
return 0;
}
+/*
+ * Helper for adding a discard request to the queue without any refcount
+ * modifications. If external data file is used redirects the request to
+ * the corresponding BdrvChild.
+ */
+static inline void
+discard_no_unref_any_file(BlockDriverState *bs, uint64_t offset,
+ uint64_t length, QCow2ClusterType ctype,
+ enum qcow2_discard_type dtype)
+{
+ BDRVQcow2State *s = bs->opaque;
+
+ if (s->discard_passthrough[dtype] &&
+ (ctype == QCOW2_CLUSTER_NORMAL || ctype == QCOW2_CLUSTER_ZERO_ALLOC)) {
+ if (has_data_file(bs)) {
+ bdrv_pdiscard(s->data_file, offset, length);
+ } else {
+ qcow2_queue_discard(bs, offset, length);
+ }
+ }
+}
+
/*
* This discards as many clusters of nb_clusters as possible at once (i.e.
* all clusters in the same L2 slice) and returns the number of discarded
@@ -1974,12 +1996,10 @@ discard_in_l2_slice(BlockDriverState *bs, uint64_t offset, uint64_t nb_clusters,
if (!keep_reference) {
/* Then decrease the refcount */
qcow2_free_any_cluster(bs, old_l2_entry, type);
- } else if (s->discard_passthrough[type] &&
- (cluster_type == QCOW2_CLUSTER_NORMAL ||
- cluster_type == QCOW2_CLUSTER_ZERO_ALLOC)) {
+ } else {
/* If we keep the reference, pass on the discard still */
- bdrv_pdiscard(s->data_file, old_l2_entry & L2E_OFFSET_MASK,
- s->cluster_size);
+ discard_no_unref_any_file(bs, old_l2_entry & L2E_OFFSET_MASK,
+ s->cluster_size, cluster_type, type);
}
}
@@ -2088,12 +2108,11 @@ zero_in_l2_slice(BlockDriverState *bs, uint64_t offset,
if (!keep_reference) {
/* Then decrease the refcount */
qcow2_free_any_cluster(bs, old_l2_entry, QCOW2_DISCARD_REQUEST);
- } else if (s->discard_passthrough[QCOW2_DISCARD_REQUEST] &&
- (type == QCOW2_CLUSTER_NORMAL ||
- type == QCOW2_CLUSTER_ZERO_ALLOC)) {
+ } else {
/* If we keep the reference, pass on the discard still */
- bdrv_pdiscard(s->data_file, old_l2_entry & L2E_OFFSET_MASK,
- s->cluster_size);
+ discard_no_unref_any_file(bs, old_l2_entry & L2E_OFFSET_MASK,
+ s->cluster_size, type,
+ QCOW2_DISCARD_REQUEST);
}
}
}
--
2.39.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v2 04/11] block/file-posix: add trace event for fallocate() calls
2024-05-13 6:31 [PATCH v2 00/11] qcow2: make subclusters discardable Andrey Drobyshev
` (2 preceding siblings ...)
2024-05-13 6:31 ` [PATCH v2 03/11] qcow2: put discard requests in the common queue when discard-no-unref enabled Andrey Drobyshev
@ 2024-05-13 6:31 ` Andrey Drobyshev
2024-05-21 8:54 ` Alexander Ivanov
2024-05-24 6:40 ` Alberto Garcia
2024-05-13 6:31 ` [PATCH v2 05/11] iotests/common.rc: add disk_usage function Andrey Drobyshev
` (7 subsequent siblings)
11 siblings, 2 replies; 34+ messages in thread
From: Andrey Drobyshev @ 2024-05-13 6:31 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis,
andrey.drobyshev, den
This would ease debugging of write zeroes and discard operations.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/file-posix.c | 1 +
block/trace-events | 1 +
2 files changed, 2 insertions(+)
diff --git a/block/file-posix.c b/block/file-posix.c
index 35684f7e21..45134f0eef 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1859,6 +1859,7 @@ static int translate_err(int err)
static int do_fallocate(int fd, int mode, off_t offset, off_t len)
{
do {
+ trace_file_do_fallocate(fd, mode, offset, len);
if (fallocate(fd, mode, offset, len) == 0) {
return 0;
}
diff --git a/block/trace-events b/block/trace-events
index 8e789e1f12..2f7ad28996 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -203,6 +203,7 @@ curl_setup_preadv(uint64_t bytes, uint64_t start, const char *range) "reading %"
curl_close(void) "close"
# file-posix.c
+file_do_fallocate(int fd, int mode, int64_t offset, int64_t len) "fd=%d mode=0x%02x offset=%" PRIi64 " len=%" PRIi64
file_copy_file_range(void *bs, int src, int64_t src_off, int dst, int64_t dst_off, int64_t bytes, int flags, int64_t ret) "bs %p src_fd %d offset %"PRIu64" dst_fd %d offset %"PRIu64" bytes %"PRIu64" flags %d ret %"PRId64
file_FindEjectableOpticalMedia(const char *media) "Matching using %s"
file_setup_cdrom(const char *partition) "Using %s as optical disc"
--
2.39.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v2 05/11] iotests/common.rc: add disk_usage function
2024-05-13 6:31 [PATCH v2 00/11] qcow2: make subclusters discardable Andrey Drobyshev
` (3 preceding siblings ...)
2024-05-13 6:31 ` [PATCH v2 04/11] block/file-posix: add trace event for fallocate() calls Andrey Drobyshev
@ 2024-05-13 6:31 ` Andrey Drobyshev
2024-05-21 8:56 ` Alexander Ivanov
2024-05-24 6:41 ` Alberto Garcia
2024-05-13 6:31 ` [PATCH v2 06/11] iotests/290: add test case to check 'discard-no-unref' option behavior Andrey Drobyshev
` (6 subsequent siblings)
11 siblings, 2 replies; 34+ messages in thread
From: Andrey Drobyshev @ 2024-05-13 6:31 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis,
andrey.drobyshev, den
Move the definition from iotests/250 to common.rc. This is used to
detect real disk usage of sparse files. In particular, we want to use
it for checking subclusters-based discards.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
tests/qemu-iotests/250 | 5 -----
tests/qemu-iotests/common.rc | 6 ++++++
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/tests/qemu-iotests/250 b/tests/qemu-iotests/250
index af48f83aba..c0a0dbc0ff 100755
--- a/tests/qemu-iotests/250
+++ b/tests/qemu-iotests/250
@@ -52,11 +52,6 @@ _unsupported_imgopts data_file
# bdrv_co_truncate(bs->file) call in qcow2_co_truncate(), which might succeed
# anyway.
-disk_usage()
-{
- du --block-size=1 $1 | awk '{print $1}'
-}
-
size=2100M
_make_test_img -o "cluster_size=1M,preallocation=metadata" $size
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 95c12577dd..237f746af8 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -140,6 +140,12 @@ _optstr_add()
fi
}
+# report real disk usage for sparse files
+disk_usage()
+{
+ du --block-size=1 "$1" | awk '{print $1}'
+}
+
# Set the variables to the empty string to turn Valgrind off
# for specific processes, e.g.
# $ VALGRIND_QEMU_IO= ./check -qcow2 -valgrind 015
--
2.39.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v2 06/11] iotests/290: add test case to check 'discard-no-unref' option behavior
2024-05-13 6:31 [PATCH v2 00/11] qcow2: make subclusters discardable Andrey Drobyshev
` (4 preceding siblings ...)
2024-05-13 6:31 ` [PATCH v2 05/11] iotests/common.rc: add disk_usage function Andrey Drobyshev
@ 2024-05-13 6:31 ` Andrey Drobyshev
2024-05-21 9:09 ` Alexander Ivanov
2024-05-24 6:42 ` Alberto Garcia
2024-05-13 6:31 ` [PATCH v2 07/11] qcow2: add get_sc_range_info() helper for working with subcluster ranges Andrey Drobyshev
` (5 subsequent siblings)
11 siblings, 2 replies; 34+ messages in thread
From: Andrey Drobyshev @ 2024-05-13 6:31 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis,
andrey.drobyshev, den
We basically fill 2 images with identical data and perform discard
operations with and without 'discard-no-unref' enabled. Then we check
that images still read identically, that their disk usage is the same
(i.e. fallocate(FALLOC_FL_PUNCH_HOLE|FALLOC_FL_KEEP_SIZE) is called for
both) and that with the option enabled cluster is still marked as
allocated in "qemu-img map" output. We also check that the option
doesn't work with qcow2 v2 images.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
tests/qemu-iotests/290 | 34 ++++++++++++++++++++++++++++++++++
tests/qemu-iotests/290.out | 28 ++++++++++++++++++++++++++++
2 files changed, 62 insertions(+)
diff --git a/tests/qemu-iotests/290 b/tests/qemu-iotests/290
index 776b59de1b..4eb929d15f 100755
--- a/tests/qemu-iotests/290
+++ b/tests/qemu-iotests/290
@@ -92,6 +92,40 @@ for qcow2_compat in 0.10 1.1; do
$QEMU_IMG map "$TEST_IMG" | _filter_testdir
done
+echo
+echo "### Test 'qemu-io -c discard' with 'discard-no-unref' option enabled"
+echo
+
+echo "# Check that qcow2 v2 images don't support 'discard-no-unref' option"
+NOUNREF_IMG="$TEST_IMG.nounref"
+TEST_IMG="$NOUNREF_IMG" _make_test_img -o "compat=0.10" 128k
+# This should immediately fail with an error
+$QEMU_IO -c 'reopen -o discard-no-unref=on' "$NOUNREF_IMG" | _filter_qemu_io
+
+echo "# Create two compat=1.1 images and fill them with identical data"
+_make_test_img -o "compat=1.1" 128k
+TEST_IMG="$NOUNREF_IMG" _make_test_img -o "compat=1.1" 128k
+$QEMU_IO -c 'write -P 0xaa 0 128k' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c 'write -P 0xaa 0 128k' "$NOUNREF_IMG" | _filter_qemu_io
+
+echo "# Enable 'discard-no-unref' in one of them, discard 2nd cluster in both"
+$QEMU_IO -c 'discard 64k 64k' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c 'reopen -o discard-no-unref=on' \
+ -c 'discard 64k 64k' "$NOUNREF_IMG" | _filter_qemu_io
+
+echo "# Compare disk usage of the 2 images"
+# Don't check the exact disk usage values but rather that they're equal
+echo "disk_usage(`basename $TEST_IMG`) - disk_usage(`basename $NOUNREF_IMG`)" \
+ "= $(( `disk_usage $TEST_IMG` - `disk_usage $NOUNREF_IMG`))"
+
+echo "# Check that images are still identical"
+$QEMU_IMG compare "$TEST_IMG" "$NOUNREF_IMG"
+
+echo "# Output of qemu-img map for the image with dropped reference"
+$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
+echo "# Output of qemu-img map for the image with kept reference"
+$QEMU_IMG map --output=json "$NOUNREF_IMG" | _filter_qemu_img_map
+
# success, all done
echo "*** done"
rm -f $seq.full
diff --git a/tests/qemu-iotests/290.out b/tests/qemu-iotests/290.out
index 22b476594f..f790feae81 100644
--- a/tests/qemu-iotests/290.out
+++ b/tests/qemu-iotests/290.out
@@ -58,4 +58,32 @@ read 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Output of qemu-img map
Offset Length Mapped to File
+
+### Test 'qemu-io -c discard' with 'discard-no-unref' option enabled
+
+# Check that qcow2 v2 images don't support 'discard-no-unref' option
+Formatting 'TEST_DIR/t.IMGFMT.nounref', fmt=IMGFMT size=131072
+qemu-io: discard-no-unref is only supported since qcow2 version 3
+# Create two compat=1.1 images and fill them with identical data
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072
+Formatting 'TEST_DIR/t.IMGFMT.nounref', fmt=IMGFMT size=131072
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+# Enable 'discard-no-unref' in one of them, discard 2nd cluster in both
+discard 65536/65536 bytes at offset 65536
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+discard 65536/65536 bytes at offset 65536
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+# Compare disk usage of the 2 images
+disk_usage(t.qcow2) - disk_usage(t.qcow2.nounref) = 0
+# Check that images are still identical
+Images are identical.
+# Output of qemu-img map for the image with dropped reference
+[{ "start": 0, "length": 65536, "depth": 0, "present": true, "zero": false, "data": true, "compressed": false, "offset": OFFSET},
+{ "start": 65536, "length": 65536, "depth": 0, "present": true, "zero": true, "data": false, "compressed": false}]
+# Output of qemu-img map for the image with kept reference
+[{ "start": 0, "length": 65536, "depth": 0, "present": true, "zero": false, "data": true, "compressed": false, "offset": OFFSET},
+{ "start": 65536, "length": 65536, "depth": 0, "present": true, "zero": true, "data": false, "compressed": false, "offset": OFFSET}]
*** done
--
2.39.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v2 07/11] qcow2: add get_sc_range_info() helper for working with subcluster ranges
2024-05-13 6:31 [PATCH v2 00/11] qcow2: make subclusters discardable Andrey Drobyshev
` (5 preceding siblings ...)
2024-05-13 6:31 ` [PATCH v2 06/11] iotests/290: add test case to check 'discard-no-unref' option behavior Andrey Drobyshev
@ 2024-05-13 6:31 ` Andrey Drobyshev
2024-05-21 9:17 ` Alexander Ivanov
2024-05-13 6:32 ` [PATCH v2 08/11] qcow2: zeroize the entire cluster when there're no non-zero subclusters Andrey Drobyshev
` (4 subsequent siblings)
11 siblings, 1 reply; 34+ messages in thread
From: Andrey Drobyshev @ 2024-05-13 6:31 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis,
andrey.drobyshev, den
This helper simply obtains the l2 table parameters of the cluster which
contains the given subclusters range. Right now this info is being
obtained and used by zero_l2_subclusters(). As we're about to introduce
the subclusters discard operation, this helper would let us avoid code
duplication.
Also introduce struct SubClusterRangeInfo, which would contain all the
needed params.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/qcow2-cluster.c | 140 ++++++++++++++++++++++++++++++++----------
1 file changed, 108 insertions(+), 32 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 7dff0bd5a1..475f167035 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1915,6 +1915,103 @@ discard_no_unref_any_file(BlockDriverState *bs, uint64_t offset,
}
}
+/*
+ * Structure containing info about the subclusters range within one cluster.
+ *
+ * Since @l2_slice is a strong reference to the l2 table slice containing
+ * the corresponding l2 entry, it must be explicitly released by
+ * qcow2_cache_put(). Thus the user must either declare it with g_auto()
+ * (in which case sc_range_info_cleanup() is called automatically) or do
+ * the cleanup themselves.
+ */
+typedef struct SubClusterRangeInfo {
+ uint64_t *l2_slice;
+ int l2_index;
+ uint64_t l2_entry;
+ uint64_t l2_bitmap;
+ QCow2ClusterType ctype;
+ Qcow2Cache *l2_table_cache;
+} SubClusterRangeInfo;
+
+static void sc_range_info_cleanup(SubClusterRangeInfo *scri)
+{
+ if (scri->l2_table_cache && scri->l2_slice) {
+ qcow2_cache_put(scri->l2_table_cache, (void **) &scri->l2_slice);
+ }
+}
+
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(SubClusterRangeInfo, sc_range_info_cleanup);
+
+/*
+ * For a given @offset and @nb_subclusters, fill out the SubClusterRangeInfo
+ * structure describing the subclusters range and referred to by @scri.
+ * Only the subclusters which can be independently discarded/zeroized
+ * (i.e. not compressed or invalid) are considered to be valid here.
+ *
+ * The subclusters range is denoted by @offset and @nb_subclusters and must
+ * not cross the cluster boundary. @offset must be aligned to the subcluster
+ * size.
+ *
+ * Return: 0 if the SubClusterRangeInfo is successfully filled out and the
+ * subclusters within the given range might be discarded/zeroized;
+ * -EINVAL if any of the subclusters within the range is invalid;
+ * -ENOTSUP if the range is contained within a compressed cluster.
+ */
+static int GRAPH_RDLOCK
+get_sc_range_info(BlockDriverState *bs, uint64_t offset,
+ unsigned nb_subclusters, SubClusterRangeInfo *scri)
+{
+ BDRVQcow2State *s = bs->opaque;
+ int ret, sc_cleared, sc_index = offset_to_sc_index(s, offset);
+ QCow2SubclusterType sctype;
+
+ /* Here we only work with the subclusters within single cluster. */
+ assert(nb_subclusters > 0 && nb_subclusters < s->subclusters_per_cluster);
+ assert(sc_index + nb_subclusters <= s->subclusters_per_cluster);
+ assert(offset_into_subcluster(s, offset) == 0);
+
+ scri->l2_table_cache = s->l2_table_cache;
+
+ ret = get_cluster_table(bs, offset, &scri->l2_slice, &scri->l2_index);
+ if (ret < 0) {
+ goto cleanup;
+ }
+
+ scri->l2_entry = get_l2_entry(s, scri->l2_slice, scri->l2_index);
+ scri->l2_bitmap = get_l2_bitmap(s, scri->l2_slice, scri->l2_index);
+ scri->ctype = qcow2_get_cluster_type(bs, scri->l2_entry);
+
+ sc_cleared = 0;
+ do {
+ ret = qcow2_get_subcluster_range_type(
+ bs, scri->l2_entry, scri->l2_bitmap, sc_index + sc_cleared,
+ &sctype);
+ if (ret < 0) {
+ goto cleanup;
+ }
+
+ switch (sctype) {
+ case QCOW2_SUBCLUSTER_COMPRESSED:
+ /* We cannot partially zeroize/discard compressed clusters. */
+ ret = -ENOTSUP;
+ goto cleanup;
+ case QCOW2_SUBCLUSTER_INVALID:
+ ret = -EINVAL;
+ goto cleanup;
+ default:
+ break;
+ }
+
+ sc_cleared += ret;
+ } while (sc_cleared < nb_subclusters);
+
+ return 0;
+
+cleanup:
+ sc_range_info_cleanup(scri);
+ return ret;
+}
+
/*
* This discards as many clusters of nb_clusters as possible at once (i.e.
* all clusters in the same L2 slice) and returns the number of discarded
@@ -2127,46 +2224,25 @@ zero_l2_subclusters(BlockDriverState *bs, uint64_t offset,
unsigned nb_subclusters)
{
BDRVQcow2State *s = bs->opaque;
- uint64_t *l2_slice;
- uint64_t old_l2_bitmap, l2_bitmap;
- int l2_index, ret, sc = offset_to_sc_index(s, offset);
-
- /* For full clusters use zero_in_l2_slice() instead */
- assert(nb_subclusters > 0 && nb_subclusters < s->subclusters_per_cluster);
- assert(sc + nb_subclusters <= s->subclusters_per_cluster);
- assert(offset_into_subcluster(s, offset) == 0);
+ uint64_t new_l2_bitmap;
+ int ret, sc = offset_to_sc_index(s, offset);
+ g_auto(SubClusterRangeInfo) scri = { 0 };
- ret = get_cluster_table(bs, offset, &l2_slice, &l2_index);
+ ret = get_sc_range_info(bs, offset, nb_subclusters, &scri);
if (ret < 0) {
return ret;
}
- switch (qcow2_get_cluster_type(bs, get_l2_entry(s, l2_slice, l2_index))) {
- case QCOW2_CLUSTER_COMPRESSED:
- ret = -ENOTSUP; /* We cannot partially zeroize compressed clusters */
- goto out;
- case QCOW2_CLUSTER_NORMAL:
- case QCOW2_CLUSTER_UNALLOCATED:
- break;
- default:
- g_assert_not_reached();
- }
-
- old_l2_bitmap = l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index);
-
- l2_bitmap |= QCOW_OFLAG_SUB_ZERO_RANGE(sc, sc + nb_subclusters);
- l2_bitmap &= ~QCOW_OFLAG_SUB_ALLOC_RANGE(sc, sc + nb_subclusters);
+ new_l2_bitmap = scri.l2_bitmap;
+ new_l2_bitmap |= QCOW_OFLAG_SUB_ZERO_RANGE(sc, sc + nb_subclusters);
+ new_l2_bitmap &= ~QCOW_OFLAG_SUB_ALLOC_RANGE(sc, sc + nb_subclusters);
- if (old_l2_bitmap != l2_bitmap) {
- set_l2_bitmap(s, l2_slice, l2_index, l2_bitmap);
- qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
+ if (new_l2_bitmap != scri.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);
}
- ret = 0;
-out:
- qcow2_cache_put(s->l2_table_cache, (void **) &l2_slice);
-
- return ret;
+ return 0;
}
int coroutine_fn qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset,
--
2.39.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v2 08/11] qcow2: zeroize the entire cluster when there're no non-zero subclusters
2024-05-13 6:31 [PATCH v2 00/11] qcow2: make subclusters discardable Andrey Drobyshev
` (6 preceding siblings ...)
2024-05-13 6:31 ` [PATCH v2 07/11] qcow2: add get_sc_range_info() helper for working with subcluster ranges Andrey Drobyshev
@ 2024-05-13 6:32 ` Andrey Drobyshev
2024-05-21 9:27 ` Alexander Ivanov
2024-05-13 6:32 ` [PATCH v2 09/11] qcow2: make subclusters discardable Andrey Drobyshev
` (3 subsequent siblings)
11 siblings, 1 reply; 34+ messages in thread
From: Andrey Drobyshev @ 2024-05-13 6:32 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis,
andrey.drobyshev, den
When zeroizing the last non-zero subclusters within single cluster, it
makes sense to go zeroize the entire cluster and go down zero_in_l2_slice()
path right away. That way we'd also update the corresponding refcount
table.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
---
block/qcow2-cluster.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 475f167035..8d39e2f960 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -2221,7 +2221,7 @@ zero_in_l2_slice(BlockDriverState *bs, uint64_t offset,
static int coroutine_fn GRAPH_RDLOCK
zero_l2_subclusters(BlockDriverState *bs, uint64_t offset,
- unsigned nb_subclusters)
+ unsigned nb_subclusters, int flags)
{
BDRVQcow2State *s = bs->opaque;
uint64_t new_l2_bitmap;
@@ -2237,6 +2237,16 @@ zero_l2_subclusters(BlockDriverState *bs, uint64_t offset,
new_l2_bitmap |= QCOW_OFLAG_SUB_ZERO_RANGE(sc, sc + nb_subclusters);
new_l2_bitmap &= ~QCOW_OFLAG_SUB_ALLOC_RANGE(sc, sc + nb_subclusters);
+ /*
+ * If there're no non-zero subclusters left, we might as well zeroize
+ * the entire cluster. That way we'd also update the refcount table.
+ */
+ if ((new_l2_bitmap & QCOW_L2_BITMAP_ALL_ZEROES) ==
+ QCOW_L2_BITMAP_ALL_ZEROES) {
+ return zero_in_l2_slice(bs, QEMU_ALIGN_DOWN(offset, s->cluster_size),
+ 1, flags);
+ }
+
if (new_l2_bitmap != scri.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);
@@ -2293,7 +2303,7 @@ int coroutine_fn qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset,
if (head) {
ret = zero_l2_subclusters(bs, offset - head,
- size_to_subclusters(s, head));
+ size_to_subclusters(s, head), flags);
if (ret < 0) {
goto fail;
}
@@ -2314,7 +2324,8 @@ int coroutine_fn qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset,
}
if (tail) {
- ret = zero_l2_subclusters(bs, end_offset, size_to_subclusters(s, tail));
+ ret = zero_l2_subclusters(bs, end_offset,
+ size_to_subclusters(s, tail), flags);
if (ret < 0) {
goto fail;
}
--
2.39.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v2 09/11] qcow2: make subclusters discardable
2024-05-13 6:31 [PATCH v2 00/11] qcow2: make subclusters discardable Andrey Drobyshev
` (7 preceding siblings ...)
2024-05-13 6:32 ` [PATCH v2 08/11] qcow2: zeroize the entire cluster when there're no non-zero subclusters Andrey Drobyshev
@ 2024-05-13 6:32 ` Andrey Drobyshev
2024-05-21 10:41 ` Alexander Ivanov
2024-05-13 6:32 ` [PATCH v2 10/11] qcow2: zero_l2_subclusters: fall through to discard operation when requested Andrey Drobyshev
` (2 subsequent siblings)
11 siblings, 1 reply; 34+ messages in thread
From: Andrey Drobyshev @ 2024-05-13 6:32 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis,
andrey.drobyshev, den
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.
Also rename qcow2_cluster_discard() -> qcow2_subcluster_discard() to
reflect the change.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/qcow2-cluster.c | 106 +++++++++++++++++++++++++++++++++++++----
block/qcow2-snapshot.c | 6 +--
block/qcow2.c | 25 +++++-----
block/qcow2.h | 4 +-
tests/qemu-iotests/271 | 2 +-
5 files changed, 117 insertions(+), 26 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 8d39e2f960..3c134a7e80 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -2105,25 +2105,106 @@ discard_in_l2_slice(BlockDriverState *bs, uint64_t offset, uint64_t nb_clusters,
return nb_clusters;
}
-int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, enum qcow2_discard_type type,
- bool full_discard)
+static int coroutine_fn GRAPH_RDLOCK
+discard_l2_subclusters(BlockDriverState *bs, uint64_t offset,
+ uint64_t nb_subclusters,
+ enum qcow2_discard_type type,
+ bool full_discard)
+{
+ BDRVQcow2State *s = bs->opaque;
+ uint64_t new_l2_bitmap, bitmap_alloc_mask, bitmap_zero_mask;
+ int ret, sc = offset_to_sc_index(s, offset);
+ g_auto(SubClusterRangeInfo) scri = { 0 };
+
+ ret = get_sc_range_info(bs, offset, nb_subclusters, &scri);
+ if (ret < 0) {
+ return ret;
+ }
+
+ new_l2_bitmap = scri.l2_bitmap;
+ bitmap_alloc_mask = QCOW_OFLAG_SUB_ALLOC_RANGE(sc, sc + nb_subclusters);
+ bitmap_zero_mask = QCOW_OFLAG_SUB_ZERO_RANGE(sc, sc + nb_subclusters);
+
+ new_l2_bitmap &= ~bitmap_alloc_mask;
+
+ /*
+ * Full discard means we fall through to the backing file, thus we need
+ * to mark the subclusters as deallocated and clear the corresponding
+ * zero bits.
+ *
+ * Non-full discard means subclusters should be explicitly marked as
+ * zeroes. In this case QCOW2 specification requires the corresponding
+ * allocation status bits to be unset as well. If the subclusters are
+ * deallocated in the first place and there's no backing, the operation
+ * can be skipped.
+ */
+ if (full_discard) {
+ new_l2_bitmap &= ~bitmap_zero_mask;
+ } else if (bs->backing || scri.l2_bitmap & bitmap_alloc_mask) {
+ new_l2_bitmap |= bitmap_zero_mask;
+ }
+
+ /*
+ * If after discarding this range there won't be any allocated subclusters
+ * left, and new bitmap becomes the same as it'd be after discarding the
+ * whole cluster, we better discard it entirely. That way we'd also
+ * update the refcount table.
+ */
+ if ((full_discard && new_l2_bitmap == 0) ||
+ (!full_discard && new_l2_bitmap == QCOW_L2_BITMAP_ALL_ZEROES)) {
+ return discard_in_l2_slice(
+ bs, QEMU_ALIGN_DOWN(offset, s->cluster_size),
+ 1, type, full_discard);
+ }
+
+ 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);
+ }
+
+ 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;
+}
+
+int qcow2_subcluster_discard(BlockDriverState *bs, uint64_t offset,
+ uint64_t bytes, enum qcow2_discard_type type,
+ bool full_discard)
{
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);
+ if (ret < 0) {
+ goto fail;
+ }
+ }
+
+ nb_clusters = size_to_clusters(s, end_offset - offset);
+
/* Each L2 slice is handled by its own loop iteration */
while (nb_clusters > 0) {
cleared = discard_in_l2_slice(bs, offset, nb_clusters, type,
@@ -2137,6 +2218,15 @@ int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
offset += (cleared * s->cluster_size);
}
+ if (tail) {
+ ret = discard_l2_subclusters(bs, end_offset,
+ size_to_subclusters(s, tail), type,
+ full_discard);
+ if (ret < 0) {
+ goto fail;
+ }
+ }
+
ret = 0;
fail:
s->cache_discards = false;
@@ -2286,8 +2376,8 @@ int coroutine_fn qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset,
*/
if (s->qcow_version < 3) {
if (!bs->backing) {
- return qcow2_cluster_discard(bs, offset, bytes,
- QCOW2_DISCARD_REQUEST, false);
+ return qcow2_subcluster_discard(bs, offset, bytes,
+ QCOW2_DISCARD_REQUEST, false);
}
return -ENOTSUP;
}
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 92e47978bf..4e39354d02 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -736,9 +736,9 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
/* The VM state isn't needed any more in the active L1 table; in fact, it
* hurts by causing expensive COW for the next snapshot. */
- qcow2_cluster_discard(bs, qcow2_vm_state_offset(s),
- ROUND_UP(sn->vm_state_size, s->cluster_size),
- QCOW2_DISCARD_NEVER, false);
+ qcow2_subcluster_discard(bs, qcow2_vm_state_offset(s),
+ ROUND_UP(sn->vm_state_size, s->cluster_size),
+ QCOW2_DISCARD_NEVER, false);
#ifdef DEBUG_ALLOC
{
diff --git a/block/qcow2.c b/block/qcow2.c
index 956128b409..792b106eb6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1967,7 +1967,7 @@ static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp)
bs->bl.request_alignment = qcrypto_block_get_sector_size(s->crypto);
}
bs->bl.pwrite_zeroes_alignment = s->subcluster_size;
- bs->bl.pdiscard_alignment = s->cluster_size;
+ bs->bl.pdiscard_alignment = s->subcluster_size;
}
static int GRAPH_UNLOCKED
@@ -4112,19 +4112,19 @@ qcow2_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
return -ENOTSUP;
}
- if (!QEMU_IS_ALIGNED(offset | bytes, s->cluster_size)) {
- assert(bytes < s->cluster_size);
+ if (!QEMU_IS_ALIGNED(offset | bytes, bs->bl.pdiscard_alignment)) {
+ assert(bytes < bs->bl.pdiscard_alignment);
/* Ignore partial clusters, except for the special case of the
* complete partial cluster at the end of an unaligned file */
- if (!QEMU_IS_ALIGNED(offset, s->cluster_size) ||
+ if (!QEMU_IS_ALIGNED(offset, bs->bl.pdiscard_alignment) ||
offset + bytes != bs->total_sectors * BDRV_SECTOR_SIZE) {
return -ENOTSUP;
}
}
qemu_co_mutex_lock(&s->lock);
- ret = qcow2_cluster_discard(bs, offset, bytes, QCOW2_DISCARD_REQUEST,
- false);
+ ret = qcow2_subcluster_discard(bs, offset, bytes, QCOW2_DISCARD_REQUEST,
+ false);
qemu_co_mutex_unlock(&s->lock);
return ret;
}
@@ -4335,10 +4335,10 @@ qcow2_co_truncate(BlockDriverState *bs, int64_t offset, bool exact,
goto fail;
}
- ret = qcow2_cluster_discard(bs, ROUND_UP(offset, s->cluster_size),
- old_length - ROUND_UP(offset,
- s->cluster_size),
- QCOW2_DISCARD_ALWAYS, true);
+ ret = qcow2_subcluster_discard(bs, ROUND_UP(offset, s->cluster_size),
+ old_length - ROUND_UP(offset,
+ s->cluster_size),
+ QCOW2_DISCARD_ALWAYS, true);
if (ret < 0) {
error_setg_errno(errp, -ret, "Failed to discard cropped clusters");
goto fail;
@@ -5032,8 +5032,9 @@ static int GRAPH_RDLOCK qcow2_make_empty(BlockDriverState *bs)
* default action for this kind of discard is to pass the discard,
* which will ideally result in an actually smaller image file, as
* is probably desired. */
- ret = qcow2_cluster_discard(bs, offset, MIN(step, end_offset - offset),
- QCOW2_DISCARD_SNAPSHOT, true);
+ ret = qcow2_subcluster_discard(bs, offset,
+ MIN(step, end_offset - offset),
+ QCOW2_DISCARD_SNAPSHOT, true);
if (ret < 0) {
break;
}
diff --git a/block/qcow2.h b/block/qcow2.h
index 197bdcdf53..a65c185b51 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -953,8 +953,8 @@ void coroutine_fn GRAPH_RDLOCK
qcow2_alloc_cluster_abort(BlockDriverState *bs, QCowL2Meta *m);
int GRAPH_RDLOCK
-qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- enum qcow2_discard_type type, bool full_discard);
+qcow2_subcluster_discard(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+ enum qcow2_discard_type type, bool full_discard);
int coroutine_fn GRAPH_RDLOCK
qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
diff --git a/tests/qemu-iotests/271 b/tests/qemu-iotests/271
index 59a6fafa2f..04c57813c2 100755
--- a/tests/qemu-iotests/271
+++ b/tests/qemu-iotests/271
@@ -518,7 +518,7 @@ done
############################################################
############################################################
-# Test qcow2_cluster_discard() with full and normal discards
+# Test qcow2_subcluster_discard() with full and normal discards
for use_backing_file in yes no; do
echo
echo "### Discarding clusters with non-zero bitmaps (backing file: $use_backing_file) ###"
--
2.39.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v2 10/11] qcow2: zero_l2_subclusters: fall through to discard operation when requested
2024-05-13 6:31 [PATCH v2 00/11] qcow2: make subclusters discardable Andrey Drobyshev
` (8 preceding siblings ...)
2024-05-13 6:32 ` [PATCH v2 09/11] qcow2: make subclusters discardable Andrey Drobyshev
@ 2024-05-13 6:32 ` Andrey Drobyshev
2024-05-21 10:46 ` Alexander Ivanov
2024-05-13 6:32 ` [PATCH v2 11/11] iotests/271: add test cases for subcluster-based discard/unmap Andrey Drobyshev
2024-06-03 9:19 ` [PATCH v2 00/11] qcow2: make subclusters discardable Andrey Drobyshev
11 siblings, 1 reply; 34+ messages in thread
From: Andrey Drobyshev @ 2024-05-13 6:32 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis,
andrey.drobyshev, den
When zeroizing subclusters within single cluster, detect usage of the
BDRV_REQ_MAY_UNMAP flag and fall through to the subcluster-based discard
operation, much like it's done with the cluster-based discards. That
way subcluster-aligned operations "qemu-io -c 'write -z -u ...'" will
lead to actual unmap.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/qcow2-cluster.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 3c134a7e80..53e04eff93 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -2107,15 +2107,16 @@ discard_in_l2_slice(BlockDriverState *bs, uint64_t offset, uint64_t nb_clusters,
static int coroutine_fn GRAPH_RDLOCK
discard_l2_subclusters(BlockDriverState *bs, uint64_t offset,
- uint64_t nb_subclusters,
- enum qcow2_discard_type type,
- bool full_discard)
+ uint64_t nb_subclusters, enum qcow2_discard_type type,
+ bool full_discard, bool uncond_zeroize)
{
BDRVQcow2State *s = bs->opaque;
uint64_t new_l2_bitmap, bitmap_alloc_mask, bitmap_zero_mask;
int ret, sc = offset_to_sc_index(s, offset);
g_auto(SubClusterRangeInfo) scri = { 0 };
+ assert(!(full_discard && uncond_zeroize));
+
ret = get_sc_range_info(bs, offset, nb_subclusters, &scri);
if (ret < 0) {
return ret;
@@ -2140,7 +2141,8 @@ discard_l2_subclusters(BlockDriverState *bs, uint64_t offset,
*/
if (full_discard) {
new_l2_bitmap &= ~bitmap_zero_mask;
- } else if (bs->backing || scri.l2_bitmap & bitmap_alloc_mask) {
+ } else if (uncond_zeroize || bs->backing ||
+ scri.l2_bitmap & bitmap_alloc_mask) {
new_l2_bitmap |= bitmap_zero_mask;
}
@@ -2197,7 +2199,7 @@ int qcow2_subcluster_discard(BlockDriverState *bs, uint64_t offset,
if (head) {
ret = discard_l2_subclusters(bs, offset - head,
size_to_subclusters(s, head), type,
- full_discard);
+ full_discard, false);
if (ret < 0) {
goto fail;
}
@@ -2221,7 +2223,7 @@ int qcow2_subcluster_discard(BlockDriverState *bs, uint64_t offset,
if (tail) {
ret = discard_l2_subclusters(bs, end_offset,
size_to_subclusters(s, tail), type,
- full_discard);
+ full_discard, false);
if (ret < 0) {
goto fail;
}
@@ -2318,6 +2320,18 @@ zero_l2_subclusters(BlockDriverState *bs, uint64_t offset,
int ret, sc = offset_to_sc_index(s, offset);
g_auto(SubClusterRangeInfo) scri = { 0 };
+ /*
+ * If the request allows discarding subclusters we go down the discard
+ * path regardless of their allocation status. After the discard
+ * operation with full_discard=false subclusters are going to be read as
+ * zeroes anyway. But we make sure that subclusters are explicitly
+ * zeroed anyway with uncond_zeroize=true.
+ */
+ if (flags & BDRV_REQ_MAY_UNMAP) {
+ return discard_l2_subclusters(bs, offset, nb_subclusters,
+ QCOW2_DISCARD_REQUEST, false, true);
+ }
+
ret = get_sc_range_info(bs, offset, nb_subclusters, &scri);
if (ret < 0) {
return ret;
--
2.39.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v2 11/11] iotests/271: add test cases for subcluster-based discard/unmap
2024-05-13 6:31 [PATCH v2 00/11] qcow2: make subclusters discardable Andrey Drobyshev
` (9 preceding siblings ...)
2024-05-13 6:32 ` [PATCH v2 10/11] qcow2: zero_l2_subclusters: fall through to discard operation when requested Andrey Drobyshev
@ 2024-05-13 6:32 ` Andrey Drobyshev
2024-05-21 10:50 ` Alexander Ivanov
2024-06-03 9:19 ` [PATCH v2 00/11] qcow2: make subclusters discardable Andrey Drobyshev
11 siblings, 1 reply; 34+ messages in thread
From: Andrey Drobyshev @ 2024-05-13 6:32 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis,
andrey.drobyshev, den
Add a bunch of test cases covering new subclusters behaviour: unmap of
last allocated subclusters; unmap of subclusters within unallocated
cluster; discard of unallocated subclusters within a cluster; regular discard
of subclusters within a cluster; discard of last allocated subclusters.
Also make all discard/unmap operations enable trace point 'file_do_fallocate'
so that actual fallocate() calls are visible.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
tests/qemu-iotests/271 | 70 +++++++++++++++++++++++++++++---------
tests/qemu-iotests/271.out | 69 ++++++++++++++++++++++++++++++++++---
2 files changed, 119 insertions(+), 20 deletions(-)
diff --git a/tests/qemu-iotests/271 b/tests/qemu-iotests/271
index 04c57813c2..8b80682cff 100755
--- a/tests/qemu-iotests/271
+++ b/tests/qemu-iotests/271
@@ -81,6 +81,12 @@ _verify_l2_bitmap()
fi
}
+# Filter out trace params which we don't control (file fd)
+_filter_trace_fallocate()
+{
+ sed -E 's/fd=[0-9]+/fd=N/g'
+}
+
# This should be called as _run_test c=XXX sc=XXX off=XXX len=XXX cmd=XXX
# c: cluster number (0 if unset)
# sc: subcluster number inside cluster @c (0 if unset)
@@ -94,12 +100,13 @@ _verify_l2_bitmap()
# discard -> discard
_run_test()
{
- unset c sc off len cmd
+ unset c sc off len cmd opt
for var in "$@"; do eval "$var"; done
case "${cmd:-write}" in
zero)
cmd="write -q -z";;
unmap)
+ opt="--trace enable=file_do_fallocate"
cmd="write -q -z -u";;
compress)
pat=$((${pat:-0} + 1))
@@ -108,6 +115,7 @@ _run_test()
pat=$((${pat:-0} + 1))
cmd="write -q -P ${pat}";;
discard)
+ opt="--trace enable=file_do_fallocate"
cmd="discard -q";;
*)
echo "Unknown option $cmd"
@@ -121,7 +129,7 @@ _run_test()
cmd="$cmd ${offset} ${len}"
raw_cmd=$(echo $cmd | sed s/-c//) # Raw images don't support -c
echo $cmd | sed 's/-P [0-9][0-9]\?/-P PATTERN/'
- $QEMU_IO -c "$cmd" "$TEST_IMG" | _filter_qemu_io
+ $QEMU_IO $opt -c "$cmd" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_trace_fallocate
$QEMU_IO -c "$raw_cmd" -f raw "$TEST_IMG.raw" | _filter_qemu_io
_verify_img
_verify_l2_bitmap "$c"
@@ -202,9 +210,10 @@ for use_backing_file in yes no; do
alloc="$(seq 16 31)"; zero="$(seq 0 15)"
_run_test sc=0 len=32k cmd=unmap
- ### Zero and unmap cluster #0
+ ### Zero and unmap second half of cluster #0 (this will unmap it and
+ ### discard l2 entry)
alloc=""; zero="$(seq 0 31)"
- _run_test sc=0 len=64k cmd=unmap
+ _run_test sc=16 len=32k cmd=unmap
### Write subcluster #1 (middle of subcluster)
alloc="1"; zero="0 $(seq 2 31)"
@@ -439,6 +448,12 @@ for use_backing_file in yes no; do
_verify_l2_bitmap 16
_verify_l2_bitmap 17
+ # Unmap subclusters #0-#3 of an unallocated cluster #8. Since
+ # 'write -z -u' doesn't lead to full discard, subclusters should become
+ # explicitly zeroized
+ alloc=""; zero="$(seq 0 3)"
+ _run_test c=8 sc=0 len=8k cmd=unmap
+
# Cluster-aligned request from clusters #9 to #11
alloc=""; zero="$(seq 0 31)"
_run_test c=9 sc=0 len=192k cmd=unmap
@@ -523,26 +538,45 @@ for use_backing_file in yes no; do
echo
echo "### Discarding clusters with non-zero bitmaps (backing file: $use_backing_file) ###"
echo
+
+ _reset_img 1M
+
+ # Write first half of cluster #0 and discard another half
+ alloc="$(seq 0 15)"; zero=""
+ _run_test sc=0 len=32k
+ # When discarding unallocated subclusters, they only have to be
+ # explicitly zeroized when the image has a backing file
if [ "$use_backing_file" = "yes" ]; then
- _make_test_img -o extended_l2=on -F raw -b "$TEST_IMG.base" 1M
+ alloc="$(seq 0 15)"; zero="$(seq 16 31)"
else
- _make_test_img -o extended_l2=on 1M
+ alloc="$(seq 0 15)"; zero=""
fi
- # Write clusters #0-#2 and then discard them
- $QEMU_IO -c 'write -q 0 128k' "$TEST_IMG"
- $QEMU_IO -c 'discard -q 0 128k' "$TEST_IMG"
+ _run_test sc=16 len=32k cmd=discard
+
+ # Write cluster #0 and discard its subclusters #0-#3
+ alloc="$(seq 0 31)"; zero=""
+ _run_test sc=0 len=64k
+ alloc="$(seq 4 31)"; zero="$(seq 0 3)"
+ _run_test sc=0 len=8k cmd=discard
+
+ # Discard remaining subclusters #4-#32. This should unmap the cluster
+ # and discard its l2 entry
+ alloc=""; zero="$(seq 0 31)"
+ _run_test sc=4 len=56k cmd=discard
+
+ # Write clusters #0-#1 and then discard them
+ alloc="$(seq 0 31)"; zero=""
+ _run_test sc=0 len=128k
# 'qemu-io discard' doesn't do a full discard, it zeroizes the
- # cluster, so both clusters have all zero bits set now
+ # cluster, so both clusters have all zero bits set after discard
alloc=""; zero="$(seq 0 31)"
- _verify_l2_bitmap 0
+ _run_test sc=0 len=128k cmd=discard
_verify_l2_bitmap 1
+
# Now mark the 2nd half of the subclusters from cluster #0 as unallocated
poke_file "$TEST_IMG" $(($l2_offset+8)) "\x00\x00"
+
# Discard cluster #0 again to see how the zero bits have changed
- $QEMU_IO -c 'discard -q 0 64k' "$TEST_IMG"
- # And do a full discard of cluster #1 by shrinking and growing the image
- $QEMU_IMG resize --shrink "$TEST_IMG" 64k
- $QEMU_IMG resize "$TEST_IMG" 1M
# A normal discard sets all 'zero' bits only if the image has a
# backing file, otherwise it won't touch them.
if [ "$use_backing_file" = "yes" ]; then
@@ -550,7 +584,11 @@ for use_backing_file in yes no; do
else
alloc=""; zero="$(seq 0 15)"
fi
- _verify_l2_bitmap 0
+ _run_test sc=0 len=64k cmd=discard
+
+ # And do a full discard of cluster #1 by shrinking and growing the image
+ $QEMU_IMG resize --shrink "$TEST_IMG" 64k
+ $QEMU_IMG resize "$TEST_IMG" 1M
# A full discard should clear the L2 entry completely. However
# when growing an image with a backing file the new clusters are
# zeroized to hide the stale data from the backing file
diff --git a/tests/qemu-iotests/271.out b/tests/qemu-iotests/271.out
index 0b24d50159..c03e4c9bc2 100644
--- a/tests/qemu-iotests/271.out
+++ b/tests/qemu-iotests/271.out
@@ -29,14 +29,17 @@ L2 entry #0: 0x8000000000050000 ffffffff00000000
write -q -P PATTERN 0 64k
L2 entry #0: 0x8000000000050000 00000000ffffffff
write -q -z -u 0 32k
+file_do_fallocate fd=N mode=0x03 offset=327680 len=32768
L2 entry #0: 0x8000000000050000 0000ffffffff0000
-write -q -z -u 0 64k
+write -q -z -u 32k 32k
+file_do_fallocate fd=N mode=0x03 offset=327680 len=65536
L2 entry #0: 0x0000000000000000 ffffffff00000000
write -q -P PATTERN 3k 512
L2 entry #0: 0x8000000000050000 fffffffd00000002
write -q -P PATTERN 0 64k
L2 entry #0: 0x8000000000050000 00000000ffffffff
discard -q 0 64k
+file_do_fallocate fd=N mode=0x03 offset=327680 len=65536
L2 entry #0: 0x0000000000000000 ffffffff00000000
write -q -c -P PATTERN 0 64k
L2 entry #0: 0x4000000000050000 0000000000000000
@@ -71,14 +74,17 @@ L2 entry #0: 0x8000000000050000 ffffffff00000000
write -q -P PATTERN 0 64k
L2 entry #0: 0x8000000000050000 00000000ffffffff
write -q -z -u 0 32k
+file_do_fallocate fd=N mode=0x03 offset=327680 len=32768
L2 entry #0: 0x8000000000050000 0000ffffffff0000
-write -q -z -u 0 64k
+write -q -z -u 32k 32k
+file_do_fallocate fd=N mode=0x03 offset=327680 len=65536
L2 entry #0: 0x0000000000000000 ffffffff00000000
write -q -P PATTERN 3k 512
L2 entry #0: 0x8000000000050000 fffffffd00000002
write -q -P PATTERN 0 64k
L2 entry #0: 0x8000000000050000 00000000ffffffff
discard -q 0 64k
+file_do_fallocate fd=N mode=0x03 offset=327680 len=65536
L2 entry #0: 0x0000000000000000 ffffffff00000000
write -q -c -P PATTERN 0 64k
L2 entry #0: 0x4000000000050000 0000000000000000
@@ -301,15 +307,20 @@ L2 entry #14: 0x80000000000a0000 00000000ffffffff
L2 entry #15: 0x80000000000b0000 00000000ffffffff
L2 entry #16: 0x80000000000c0000 00000000ffffffff
L2 entry #17: 0x80000000000d0000 00000000ffffffff
+write -q -z -u 512k 8k
+L2 entry #8: 0x0000000000000000 0000000f00000000
write -q -z -u 576k 192k
+file_do_fallocate fd=N mode=0x03 offset=327680 len=196608
L2 entry #9: 0x0000000000000000 ffffffff00000000
L2 entry #10: 0x0000000000000000 ffffffff00000000
L2 entry #11: 0x0000000000000000 ffffffff00000000
write -q -z -u 800k 128k
+file_do_fallocate fd=N mode=0x03 offset=557056 len=131072
L2 entry #12: 0x8000000000080000 ffff00000000ffff
L2 entry #13: 0x0000000000000000 ffffffff00000000
L2 entry #14: 0x80000000000a0000 0000ffffffff0000
write -q -z -u 991k 128k
+file_do_fallocate fd=N mode=0x03 offset=753664 len=129024
L2 entry #15: 0x80000000000b0000 ffff00000000ffff
L2 entry #16: 0x0000000000000000 ffffffff00000000
L2 entry #17: 0x80000000000d0000 00007fffffff8000
@@ -339,6 +350,7 @@ L2 entry #27: 0x4000000000120000 0000000000000000
write -q -c -P PATTERN 1792k 64k
L2 entry #28: 0x4000000000130000 0000000000000000
write -q -z -u 1152k 192k
+file_do_fallocate fd=N mode=0x03 offset=327680 len=196608
L2 entry #18: 0x0000000000000000 ffffffff00000000
L2 entry #19: 0x0000000000000000 ffffffff00000000
L2 entry #20: 0x0000000000000000 ffffffff00000000
@@ -351,6 +363,8 @@ L2 entry #24: 0x8000000000090000 00000000ffffffff
L2 entry #25: 0x80000000000e0000 00000000ffffffff
L2 entry #26: 0x80000000000f0000 00000000ffffffff
write -q -z -u 1759k 128k
+file_do_fallocate fd=N mode=0x03 offset=819200 len=32768
+file_do_fallocate fd=N mode=0x03 offset=1245184 len=65536
L2 entry #27: 0x80000000000c0000 ffff00000000ffff
L2 entry #28: 0x0000000000000000 ffffffff00000000
L2 entry #29: 0x8000000000100000 00007fff00008000
@@ -369,15 +383,20 @@ L2 entry #14: 0x80000000000a0000 00000000ffffffff
L2 entry #15: 0x80000000000b0000 00000000ffffffff
L2 entry #16: 0x80000000000c0000 00000000ffffffff
L2 entry #17: 0x80000000000d0000 00000000ffffffff
+write -q -z -u 512k 8k
+L2 entry #8: 0x0000000000000000 0000000f00000000
write -q -z -u 576k 192k
+file_do_fallocate fd=N mode=0x03 offset=327680 len=196608
L2 entry #9: 0x0000000000000000 ffffffff00000000
L2 entry #10: 0x0000000000000000 ffffffff00000000
L2 entry #11: 0x0000000000000000 ffffffff00000000
write -q -z -u 800k 128k
+file_do_fallocate fd=N mode=0x03 offset=557056 len=131072
L2 entry #12: 0x8000000000080000 ffff00000000ffff
L2 entry #13: 0x0000000000000000 ffffffff00000000
L2 entry #14: 0x80000000000a0000 0000ffffffff0000
write -q -z -u 991k 128k
+file_do_fallocate fd=N mode=0x03 offset=753664 len=129024
L2 entry #15: 0x80000000000b0000 ffff00000000ffff
L2 entry #16: 0x0000000000000000 ffffffff00000000
L2 entry #17: 0x80000000000d0000 00007fffffff8000
@@ -407,6 +426,7 @@ L2 entry #27: 0x4000000000120000 0000000000000000
write -q -c -P PATTERN 1792k 64k
L2 entry #28: 0x4000000000130000 0000000000000000
write -q -z -u 1152k 192k
+file_do_fallocate fd=N mode=0x03 offset=327680 len=196608
L2 entry #18: 0x0000000000000000 ffffffff00000000
L2 entry #19: 0x0000000000000000 ffffffff00000000
L2 entry #20: 0x0000000000000000 ffffffff00000000
@@ -419,28 +439,69 @@ L2 entry #24: 0x8000000000090000 00000000ffffffff
L2 entry #25: 0x80000000000e0000 00000000ffffffff
L2 entry #26: 0x80000000000f0000 00000000ffffffff
write -q -z -u 1759k 128k
+file_do_fallocate fd=N mode=0x03 offset=819200 len=32768
+file_do_fallocate fd=N mode=0x03 offset=1245184 len=65536
L2 entry #27: 0x80000000000c0000 ffff00000000ffff
L2 entry #28: 0x0000000000000000 ffffffff00000000
L2 entry #29: 0x0000000000000000 0000ffff00000000
### Discarding clusters with non-zero bitmaps (backing file: yes) ###
+Formatting 'TEST_DIR/t.IMGFMT.raw', fmt=raw size=1048576
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=raw size=1048576
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=raw
+write -q -P PATTERN 0 32k
+L2 entry #0: 0x8000000000050000 000000000000ffff
+discard -q 32k 32k
+file_do_fallocate fd=N mode=0x03 offset=360448 len=32768
+L2 entry #0: 0x8000000000050000 ffff00000000ffff
+write -q -P PATTERN 0 64k
+L2 entry #0: 0x8000000000050000 00000000ffffffff
+discard -q 0 8k
+file_do_fallocate fd=N mode=0x03 offset=327680 len=8192
+L2 entry #0: 0x8000000000050000 0000000ffffffff0
+discard -q 8k 56k
+file_do_fallocate fd=N mode=0x03 offset=327680 len=65536
+L2 entry #0: 0x0000000000000000 ffffffff00000000
+write -q -P PATTERN 0 128k
+L2 entry #0: 0x8000000000050000 00000000ffffffff
+discard -q 0 128k
+file_do_fallocate fd=N mode=0x03 offset=327680 len=131072
L2 entry #0: 0x0000000000000000 ffffffff00000000
L2 entry #1: 0x0000000000000000 ffffffff00000000
+discard -q 0 64k
+L2 entry #0: 0x0000000000000000 ffffffff00000000
Image resized.
Image resized.
-L2 entry #0: 0x0000000000000000 ffffffff00000000
L2 entry #1: 0x0000000000000000 ffffffff00000000
### Discarding clusters with non-zero bitmaps (backing file: no) ###
+Formatting 'TEST_DIR/t.IMGFMT.raw', fmt=raw size=1048576
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
+write -q -P PATTERN 0 32k
+L2 entry #0: 0x8000000000050000 000000000000ffff
+discard -q 32k 32k
+file_do_fallocate fd=N mode=0x03 offset=360448 len=32768
+L2 entry #0: 0x8000000000050000 000000000000ffff
+write -q -P PATTERN 0 64k
+L2 entry #0: 0x8000000000050000 00000000ffffffff
+discard -q 0 8k
+file_do_fallocate fd=N mode=0x03 offset=327680 len=8192
+L2 entry #0: 0x8000000000050000 0000000ffffffff0
+discard -q 8k 56k
+file_do_fallocate fd=N mode=0x03 offset=327680 len=65536
+L2 entry #0: 0x0000000000000000 ffffffff00000000
+write -q -P PATTERN 0 128k
+L2 entry #0: 0x8000000000050000 00000000ffffffff
+discard -q 0 128k
+file_do_fallocate fd=N mode=0x03 offset=327680 len=131072
L2 entry #0: 0x0000000000000000 ffffffff00000000
L2 entry #1: 0x0000000000000000 ffffffff00000000
+discard -q 0 64k
+L2 entry #0: 0x0000000000000000 0000ffff00000000
Image resized.
Image resized.
-L2 entry #0: 0x0000000000000000 0000ffff00000000
L2 entry #1: 0x0000000000000000 0000000000000000
### Corrupted L2 entries - read test (allocated) ###
--
2.39.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH v2 01/11] qcow2: make function update_refcount_discard() global
2024-05-13 6:31 ` [PATCH v2 01/11] qcow2: make function update_refcount_discard() global Andrey Drobyshev
@ 2024-05-15 22:26 ` Alberto Garcia
2024-05-21 8:31 ` Alexander Ivanov
1 sibling, 0 replies; 34+ messages in thread
From: Alberto Garcia @ 2024-05-15 22:26 UTC (permalink / raw)
To: Andrey Drobyshev, qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, jean-louis, andrey.drobyshev,
den
On Mon 13 May 2024 09:31:53 AM +03, Andrey Drobyshev wrote:
> We are going to need it for discarding separate subclusters. The
> function itself doesn't do anything with the refcount tables, it simply
> adds a discard request to the queue, so rename it to qcow2_queue_discard().
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Berto
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 02/11] qcow2: simplify L2 entries accounting for discard-no-unref
2024-05-13 6:31 ` [PATCH v2 02/11] qcow2: simplify L2 entries accounting for discard-no-unref Andrey Drobyshev
@ 2024-05-15 22:28 ` Alberto Garcia
2024-05-21 8:32 ` Alexander Ivanov
1 sibling, 0 replies; 34+ messages in thread
From: Alberto Garcia @ 2024-05-15 22:28 UTC (permalink / raw)
To: Andrey Drobyshev, qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, jean-louis, andrey.drobyshev,
den
On Mon 13 May 2024 09:31:54 AM +03, Andrey Drobyshev wrote:
> Commits 42a2890a and b2b10904 introduce handling of discard-no-unref
> option in discard_in_l2_slice() and zero_in_l2_slice(). They add even
> more if's when chosing the right l2 entry. What we really need for this
> option is the new entry simply to contain the same host cluster offset,
> no matter whether we unmap or zeroize the cluster. For that OR'ing with
> the old entry is enough.
>
> This patch doesn't change the logic and is pure refactoring.
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Berto
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 01/11] qcow2: make function update_refcount_discard() global
2024-05-13 6:31 ` [PATCH v2 01/11] qcow2: make function update_refcount_discard() global Andrey Drobyshev
2024-05-15 22:26 ` Alberto Garcia
@ 2024-05-21 8:31 ` Alexander Ivanov
1 sibling, 0 replies; 34+ messages in thread
From: Alexander Ivanov @ 2024-05-21 8:31 UTC (permalink / raw)
To: Andrey Drobyshev, qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis, den
On 5/13/24 08:31, Andrey Drobyshev wrote:
> We are going to need it for discarding separate subclusters. The
> function itself doesn't do anything with the refcount tables, it simply
> adds a discard request to the queue, so rename it to qcow2_queue_discard().
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
> ---
> block/qcow2-refcount.c | 8 ++++----
> block/qcow2.h | 2 ++
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 0266542cee..2026f5fa21 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -754,8 +754,8 @@ void qcow2_process_discards(BlockDriverState *bs, int ret)
> }
> }
>
> -static void update_refcount_discard(BlockDriverState *bs,
> - uint64_t offset, uint64_t length)
> +void qcow2_queue_discard(BlockDriverState *bs, uint64_t offset,
> + uint64_t length)
> {
> BDRVQcow2State *s = bs->opaque;
> Qcow2DiscardRegion *d, *p, *next;
> @@ -902,7 +902,7 @@ update_refcount(BlockDriverState *bs, int64_t offset, int64_t length,
> }
>
> if (s->discard_passthrough[type]) {
> - update_refcount_discard(bs, cluster_offset, s->cluster_size);
> + qcow2_queue_discard(bs, cluster_offset, s->cluster_size);
> }
> }
> }
> @@ -3619,7 +3619,7 @@ qcow2_discard_refcount_block(BlockDriverState *bs, uint64_t discard_block_offs)
> /* discard refblock from the cache if refblock is cached */
> qcow2_cache_discard(s->refcount_block_cache, refblock);
> }
> - update_refcount_discard(bs, discard_block_offs, s->cluster_size);
> + qcow2_queue_discard(bs, discard_block_offs, s->cluster_size);
>
> return 0;
> }
> diff --git a/block/qcow2.h b/block/qcow2.h
> index a9e3481c6e..197bdcdf53 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -891,6 +891,8 @@ int coroutine_fn qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *re
> BdrvCheckMode fix);
>
> void GRAPH_RDLOCK qcow2_process_discards(BlockDriverState *bs, int ret);
> +void qcow2_queue_discard(BlockDriverState *bs, uint64_t offset,
> + uint64_t length);
>
> int GRAPH_RDLOCK
> qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset,
Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
--
Best regards,
Alexander Ivanov
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 02/11] qcow2: simplify L2 entries accounting for discard-no-unref
2024-05-13 6:31 ` [PATCH v2 02/11] qcow2: simplify L2 entries accounting for discard-no-unref Andrey Drobyshev
2024-05-15 22:28 ` Alberto Garcia
@ 2024-05-21 8:32 ` Alexander Ivanov
1 sibling, 0 replies; 34+ messages in thread
From: Alexander Ivanov @ 2024-05-21 8:32 UTC (permalink / raw)
To: Andrey Drobyshev, qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis, den
On 5/13/24 08:31, Andrey Drobyshev wrote:
> Commits 42a2890a and b2b10904 introduce handling of discard-no-unref
> option in discard_in_l2_slice() and zero_in_l2_slice(). They add even
> more if's when chosing the right l2 entry. What we really need for this
> option is the new entry simply to contain the same host cluster offset,
> no matter whether we unmap or zeroize the cluster. For that OR'ing with
> the old entry is enough.
>
> This patch doesn't change the logic and is pure refactoring.
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> block/qcow2-cluster.c | 34 +++++++++++++++-------------------
> 1 file changed, 15 insertions(+), 19 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index ce8c0076b3..5f057ba2fd 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1946,25 +1946,21 @@ discard_in_l2_slice(BlockDriverState *bs, uint64_t offset, uint64_t nb_clusters,
> new_l2_entry = new_l2_bitmap = 0;
> } else if (bs->backing || qcow2_cluster_is_allocated(cluster_type)) {
> if (has_subclusters(s)) {
> - if (keep_reference) {
> - new_l2_entry = old_l2_entry;
> - } else {
> - new_l2_entry = 0;
> - }
> + new_l2_entry = 0;
> new_l2_bitmap = QCOW_L2_BITMAP_ALL_ZEROES;
> } else {
> - if (s->qcow_version >= 3) {
> - if (keep_reference) {
> - new_l2_entry |= QCOW_OFLAG_ZERO;
> - } else {
> - new_l2_entry = QCOW_OFLAG_ZERO;
> - }
> - } else {
> - new_l2_entry = 0;
> - }
> + new_l2_entry = s->qcow_version >= 3 ? QCOW_OFLAG_ZERO : 0;
> }
> }
>
> + /*
> + * No need to check for the QCOW version since discard-no-unref is
> + * only allowed since version 3.
> + */
> + if (keep_reference) {
> + new_l2_entry |= old_l2_entry;
> + }
> +
> if (old_l2_entry == new_l2_entry && old_l2_bitmap == new_l2_bitmap) {
> continue;
> }
> @@ -2064,19 +2060,19 @@ zero_in_l2_slice(BlockDriverState *bs, uint64_t offset,
> ((flags & BDRV_REQ_MAY_UNMAP) && qcow2_cluster_is_allocated(type));
> bool keep_reference =
> (s->discard_no_unref && type != QCOW2_CLUSTER_COMPRESSED);
> - uint64_t new_l2_entry = old_l2_entry;
> + uint64_t new_l2_entry = unmap ? 0 : old_l2_entry;
> uint64_t new_l2_bitmap = old_l2_bitmap;
>
> - if (unmap && !keep_reference) {
> - new_l2_entry = 0;
> - }
> -
> if (has_subclusters(s)) {
> new_l2_bitmap = QCOW_L2_BITMAP_ALL_ZEROES;
> } else {
> new_l2_entry |= QCOW_OFLAG_ZERO;
> }
>
> + if (keep_reference) {
> + new_l2_entry |= old_l2_entry;
> + }
> +
> if (old_l2_entry == new_l2_entry && old_l2_bitmap == new_l2_bitmap) {
> continue;
> }
Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
--
Best regards,
Alexander Ivanov
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 03/11] qcow2: put discard requests in the common queue when discard-no-unref enabled
2024-05-13 6:31 ` [PATCH v2 03/11] qcow2: put discard requests in the common queue when discard-no-unref enabled Andrey Drobyshev
@ 2024-05-21 8:43 ` Alexander Ivanov
0 siblings, 0 replies; 34+ messages in thread
From: Alexander Ivanov @ 2024-05-21 8:43 UTC (permalink / raw)
To: Andrey Drobyshev, qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis, den
On 5/13/24 08:31, Andrey Drobyshev wrote:
> Normally discard requests are stored in the queue attached to BDRVQcow2State
> to be processed later at once. Currently discard-no-unref option handling
> causes these requests to be processed straight away. Let's fix that.
>
> Note that when doing regular discards qcow2_free_any_cluster() would check
> for the presence of external data files for us and redirect request to
> underlying data_file. Here we want to do the same but avoid refcount updates,
> thus we perform the same checks.
>
> Suggested-by: Hanna Czenczek <hreitz@redhat.com>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> block/qcow2-cluster.c | 39 +++++++++++++++++++++++++++++----------
> 1 file changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 5f057ba2fd..7dff0bd5a1 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1893,6 +1893,28 @@ again:
> return 0;
> }
>
> +/*
> + * Helper for adding a discard request to the queue without any refcount
> + * modifications. If external data file is used redirects the request to
> + * the corresponding BdrvChild.
> + */
> +static inline void
> +discard_no_unref_any_file(BlockDriverState *bs, uint64_t offset,
> + uint64_t length, QCow2ClusterType ctype,
> + enum qcow2_discard_type dtype)
> +{
> + BDRVQcow2State *s = bs->opaque;
> +
> + if (s->discard_passthrough[dtype] &&
> + (ctype == QCOW2_CLUSTER_NORMAL || ctype == QCOW2_CLUSTER_ZERO_ALLOC)) {
> + if (has_data_file(bs)) {
> + bdrv_pdiscard(s->data_file, offset, length);
> + } else {
> + qcow2_queue_discard(bs, offset, length);
> + }
> + }
> +}
> +
> /*
> * This discards as many clusters of nb_clusters as possible at once (i.e.
> * all clusters in the same L2 slice) and returns the number of discarded
> @@ -1974,12 +1996,10 @@ discard_in_l2_slice(BlockDriverState *bs, uint64_t offset, uint64_t nb_clusters,
> if (!keep_reference) {
> /* Then decrease the refcount */
> qcow2_free_any_cluster(bs, old_l2_entry, type);
> - } else if (s->discard_passthrough[type] &&
> - (cluster_type == QCOW2_CLUSTER_NORMAL ||
> - cluster_type == QCOW2_CLUSTER_ZERO_ALLOC)) {
> + } else {
> /* If we keep the reference, pass on the discard still */
> - bdrv_pdiscard(s->data_file, old_l2_entry & L2E_OFFSET_MASK,
> - s->cluster_size);
> + discard_no_unref_any_file(bs, old_l2_entry & L2E_OFFSET_MASK,
> + s->cluster_size, cluster_type, type);
> }
> }
>
> @@ -2088,12 +2108,11 @@ zero_in_l2_slice(BlockDriverState *bs, uint64_t offset,
> if (!keep_reference) {
> /* Then decrease the refcount */
> qcow2_free_any_cluster(bs, old_l2_entry, QCOW2_DISCARD_REQUEST);
> - } else if (s->discard_passthrough[QCOW2_DISCARD_REQUEST] &&
> - (type == QCOW2_CLUSTER_NORMAL ||
> - type == QCOW2_CLUSTER_ZERO_ALLOC)) {
> + } else {
> /* If we keep the reference, pass on the discard still */
> - bdrv_pdiscard(s->data_file, old_l2_entry & L2E_OFFSET_MASK,
> - s->cluster_size);
> + discard_no_unref_any_file(bs, old_l2_entry & L2E_OFFSET_MASK,
> + s->cluster_size, type,
> + QCOW2_DISCARD_REQUEST);
> }
> }
> }
Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
--
Best regards,
Alexander Ivanov
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 04/11] block/file-posix: add trace event for fallocate() calls
2024-05-13 6:31 ` [PATCH v2 04/11] block/file-posix: add trace event for fallocate() calls Andrey Drobyshev
@ 2024-05-21 8:54 ` Alexander Ivanov
2024-05-24 6:40 ` Alberto Garcia
1 sibling, 0 replies; 34+ messages in thread
From: Alexander Ivanov @ 2024-05-21 8:54 UTC (permalink / raw)
To: Andrey Drobyshev, qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis, den
On 5/13/24 08:31, Andrey Drobyshev wrote:
> This would ease debugging of write zeroes and discard operations.
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> block/file-posix.c | 1 +
> block/trace-events | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 35684f7e21..45134f0eef 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -1859,6 +1859,7 @@ static int translate_err(int err)
> static int do_fallocate(int fd, int mode, off_t offset, off_t len)
> {
> do {
> + trace_file_do_fallocate(fd, mode, offset, len);
> if (fallocate(fd, mode, offset, len) == 0) {
> return 0;
> }
> diff --git a/block/trace-events b/block/trace-events
> index 8e789e1f12..2f7ad28996 100644
> --- a/block/trace-events
> +++ b/block/trace-events
> @@ -203,6 +203,7 @@ curl_setup_preadv(uint64_t bytes, uint64_t start, const char *range) "reading %"
> curl_close(void) "close"
>
> # file-posix.c
> +file_do_fallocate(int fd, int mode, int64_t offset, int64_t len) "fd=%d mode=0x%02x offset=%" PRIi64 " len=%" PRIi64
> file_copy_file_range(void *bs, int src, int64_t src_off, int dst, int64_t dst_off, int64_t bytes, int flags, int64_t ret) "bs %p src_fd %d offset %"PRIu64" dst_fd %d offset %"PRIu64" bytes %"PRIu64" flags %d ret %"PRId64
> file_FindEjectableOpticalMedia(const char *media) "Matching using %s"
> file_setup_cdrom(const char *partition) "Using %s as optical disc"
Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
--
Best regards,
Alexander Ivanov
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 05/11] iotests/common.rc: add disk_usage function
2024-05-13 6:31 ` [PATCH v2 05/11] iotests/common.rc: add disk_usage function Andrey Drobyshev
@ 2024-05-21 8:56 ` Alexander Ivanov
2024-05-24 6:41 ` Alberto Garcia
1 sibling, 0 replies; 34+ messages in thread
From: Alexander Ivanov @ 2024-05-21 8:56 UTC (permalink / raw)
To: Andrey Drobyshev, qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis, den
On 5/13/24 08:31, Andrey Drobyshev wrote:
> Move the definition from iotests/250 to common.rc. This is used to
> detect real disk usage of sparse files. In particular, we want to use
> it for checking subclusters-based discards.
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> tests/qemu-iotests/250 | 5 -----
> tests/qemu-iotests/common.rc | 6 ++++++
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/tests/qemu-iotests/250 b/tests/qemu-iotests/250
> index af48f83aba..c0a0dbc0ff 100755
> --- a/tests/qemu-iotests/250
> +++ b/tests/qemu-iotests/250
> @@ -52,11 +52,6 @@ _unsupported_imgopts data_file
> # bdrv_co_truncate(bs->file) call in qcow2_co_truncate(), which might succeed
> # anyway.
>
> -disk_usage()
> -{
> - du --block-size=1 $1 | awk '{print $1}'
> -}
> -
> size=2100M
>
> _make_test_img -o "cluster_size=1M,preallocation=metadata" $size
> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> index 95c12577dd..237f746af8 100644
> --- a/tests/qemu-iotests/common.rc
> +++ b/tests/qemu-iotests/common.rc
> @@ -140,6 +140,12 @@ _optstr_add()
> fi
> }
>
> +# report real disk usage for sparse files
> +disk_usage()
> +{
> + du --block-size=1 "$1" | awk '{print $1}'
> +}
> +
> # Set the variables to the empty string to turn Valgrind off
> # for specific processes, e.g.
> # $ VALGRIND_QEMU_IO= ./check -qcow2 -valgrind 015
Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
--
Best regards,
Alexander Ivanov
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 06/11] iotests/290: add test case to check 'discard-no-unref' option behavior
2024-05-13 6:31 ` [PATCH v2 06/11] iotests/290: add test case to check 'discard-no-unref' option behavior Andrey Drobyshev
@ 2024-05-21 9:09 ` Alexander Ivanov
2024-05-24 6:42 ` Alberto Garcia
1 sibling, 0 replies; 34+ messages in thread
From: Alexander Ivanov @ 2024-05-21 9:09 UTC (permalink / raw)
To: Andrey Drobyshev, qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis, den
On 5/13/24 08:31, Andrey Drobyshev wrote:
> We basically fill 2 images with identical data and perform discard
> operations with and without 'discard-no-unref' enabled. Then we check
> that images still read identically, that their disk usage is the same
> (i.e. fallocate(FALLOC_FL_PUNCH_HOLE|FALLOC_FL_KEEP_SIZE) is called for
> both) and that with the option enabled cluster is still marked as
> allocated in "qemu-img map" output. We also check that the option
> doesn't work with qcow2 v2 images.
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> tests/qemu-iotests/290 | 34 ++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/290.out | 28 ++++++++++++++++++++++++++++
> 2 files changed, 62 insertions(+)
>
> diff --git a/tests/qemu-iotests/290 b/tests/qemu-iotests/290
> index 776b59de1b..4eb929d15f 100755
> --- a/tests/qemu-iotests/290
> +++ b/tests/qemu-iotests/290
> @@ -92,6 +92,40 @@ for qcow2_compat in 0.10 1.1; do
> $QEMU_IMG map "$TEST_IMG" | _filter_testdir
> done
>
> +echo
> +echo "### Test 'qemu-io -c discard' with 'discard-no-unref' option enabled"
> +echo
> +
> +echo "# Check that qcow2 v2 images don't support 'discard-no-unref' option"
> +NOUNREF_IMG="$TEST_IMG.nounref"
> +TEST_IMG="$NOUNREF_IMG" _make_test_img -o "compat=0.10" 128k
> +# This should immediately fail with an error
> +$QEMU_IO -c 'reopen -o discard-no-unref=on' "$NOUNREF_IMG" | _filter_qemu_io
> +
> +echo "# Create two compat=1.1 images and fill them with identical data"
> +_make_test_img -o "compat=1.1" 128k
> +TEST_IMG="$NOUNREF_IMG" _make_test_img -o "compat=1.1" 128k
> +$QEMU_IO -c 'write -P 0xaa 0 128k' "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c 'write -P 0xaa 0 128k' "$NOUNREF_IMG" | _filter_qemu_io
> +
> +echo "# Enable 'discard-no-unref' in one of them, discard 2nd cluster in both"
> +$QEMU_IO -c 'discard 64k 64k' "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c 'reopen -o discard-no-unref=on' \
> + -c 'discard 64k 64k' "$NOUNREF_IMG" | _filter_qemu_io
> +
> +echo "# Compare disk usage of the 2 images"
> +# Don't check the exact disk usage values but rather that they're equal
> +echo "disk_usage(`basename $TEST_IMG`) - disk_usage(`basename $NOUNREF_IMG`)" \
> + "= $(( `disk_usage $TEST_IMG` - `disk_usage $NOUNREF_IMG`))"
> +
> +echo "# Check that images are still identical"
> +$QEMU_IMG compare "$TEST_IMG" "$NOUNREF_IMG"
> +
> +echo "# Output of qemu-img map for the image with dropped reference"
> +$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
> +echo "# Output of qemu-img map for the image with kept reference"
> +$QEMU_IMG map --output=json "$NOUNREF_IMG" | _filter_qemu_img_map
> +
> # success, all done
> echo "*** done"
> rm -f $seq.full
> diff --git a/tests/qemu-iotests/290.out b/tests/qemu-iotests/290.out
> index 22b476594f..f790feae81 100644
> --- a/tests/qemu-iotests/290.out
> +++ b/tests/qemu-iotests/290.out
> @@ -58,4 +58,32 @@ read 131072/131072 bytes at offset 0
> 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> # Output of qemu-img map
> Offset Length Mapped to File
> +
> +### Test 'qemu-io -c discard' with 'discard-no-unref' option enabled
> +
> +# Check that qcow2 v2 images don't support 'discard-no-unref' option
> +Formatting 'TEST_DIR/t.IMGFMT.nounref', fmt=IMGFMT size=131072
> +qemu-io: discard-no-unref is only supported since qcow2 version 3
> +# Create two compat=1.1 images and fill them with identical data
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072
> +Formatting 'TEST_DIR/t.IMGFMT.nounref', fmt=IMGFMT size=131072
> +wrote 131072/131072 bytes at offset 0
> +128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +wrote 131072/131072 bytes at offset 0
> +128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +# Enable 'discard-no-unref' in one of them, discard 2nd cluster in both
> +discard 65536/65536 bytes at offset 65536
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +discard 65536/65536 bytes at offset 65536
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +# Compare disk usage of the 2 images
> +disk_usage(t.qcow2) - disk_usage(t.qcow2.nounref) = 0
> +# Check that images are still identical
> +Images are identical.
> +# Output of qemu-img map for the image with dropped reference
> +[{ "start": 0, "length": 65536, "depth": 0, "present": true, "zero": false, "data": true, "compressed": false, "offset": OFFSET},
> +{ "start": 65536, "length": 65536, "depth": 0, "present": true, "zero": true, "data": false, "compressed": false}]
> +# Output of qemu-img map for the image with kept reference
> +[{ "start": 0, "length": 65536, "depth": 0, "present": true, "zero": false, "data": true, "compressed": false, "offset": OFFSET},
> +{ "start": 65536, "length": 65536, "depth": 0, "present": true, "zero": true, "data": false, "compressed": false, "offset": OFFSET}]
> *** done
Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
--
Best regards,
Alexander Ivanov
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 07/11] qcow2: add get_sc_range_info() helper for working with subcluster ranges
2024-05-13 6:31 ` [PATCH v2 07/11] qcow2: add get_sc_range_info() helper for working with subcluster ranges Andrey Drobyshev
@ 2024-05-21 9:17 ` Alexander Ivanov
0 siblings, 0 replies; 34+ messages in thread
From: Alexander Ivanov @ 2024-05-21 9:17 UTC (permalink / raw)
To: Andrey Drobyshev, qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis, den
On 5/13/24 08:31, Andrey Drobyshev wrote:
> This helper simply obtains the l2 table parameters of the cluster which
> contains the given subclusters range. Right now this info is being
> obtained and used by zero_l2_subclusters(). As we're about to introduce
> the subclusters discard operation, this helper would let us avoid code
> duplication.
>
> Also introduce struct SubClusterRangeInfo, which would contain all the
> needed params.
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> block/qcow2-cluster.c | 140 ++++++++++++++++++++++++++++++++----------
> 1 file changed, 108 insertions(+), 32 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 7dff0bd5a1..475f167035 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1915,6 +1915,103 @@ discard_no_unref_any_file(BlockDriverState *bs, uint64_t offset,
> }
> }
>
> +/*
> + * Structure containing info about the subclusters range within one cluster.
> + *
> + * Since @l2_slice is a strong reference to the l2 table slice containing
> + * the corresponding l2 entry, it must be explicitly released by
> + * qcow2_cache_put(). Thus the user must either declare it with g_auto()
> + * (in which case sc_range_info_cleanup() is called automatically) or do
> + * the cleanup themselves.
> + */
> +typedef struct SubClusterRangeInfo {
> + uint64_t *l2_slice;
> + int l2_index;
> + uint64_t l2_entry;
> + uint64_t l2_bitmap;
> + QCow2ClusterType ctype;
> + Qcow2Cache *l2_table_cache;
> +} SubClusterRangeInfo;
> +
> +static void sc_range_info_cleanup(SubClusterRangeInfo *scri)
> +{
> + if (scri->l2_table_cache && scri->l2_slice) {
> + qcow2_cache_put(scri->l2_table_cache, (void **) &scri->l2_slice);
> + }
> +}
> +
> +G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(SubClusterRangeInfo, sc_range_info_cleanup);
> +
> +/*
> + * For a given @offset and @nb_subclusters, fill out the SubClusterRangeInfo
> + * structure describing the subclusters range and referred to by @scri.
> + * Only the subclusters which can be independently discarded/zeroized
> + * (i.e. not compressed or invalid) are considered to be valid here.
> + *
> + * The subclusters range is denoted by @offset and @nb_subclusters and must
> + * not cross the cluster boundary. @offset must be aligned to the subcluster
> + * size.
> + *
> + * Return: 0 if the SubClusterRangeInfo is successfully filled out and the
> + * subclusters within the given range might be discarded/zeroized;
> + * -EINVAL if any of the subclusters within the range is invalid;
> + * -ENOTSUP if the range is contained within a compressed cluster.
> + */
> +static int GRAPH_RDLOCK
> +get_sc_range_info(BlockDriverState *bs, uint64_t offset,
> + unsigned nb_subclusters, SubClusterRangeInfo *scri)
> +{
> + BDRVQcow2State *s = bs->opaque;
> + int ret, sc_cleared, sc_index = offset_to_sc_index(s, offset);
> + QCow2SubclusterType sctype;
> +
> + /* Here we only work with the subclusters within single cluster. */
> + assert(nb_subclusters > 0 && nb_subclusters < s->subclusters_per_cluster);
> + assert(sc_index + nb_subclusters <= s->subclusters_per_cluster);
> + assert(offset_into_subcluster(s, offset) == 0);
> +
> + scri->l2_table_cache = s->l2_table_cache;
> +
> + ret = get_cluster_table(bs, offset, &scri->l2_slice, &scri->l2_index);
> + if (ret < 0) {
> + goto cleanup;
> + }
> +
> + scri->l2_entry = get_l2_entry(s, scri->l2_slice, scri->l2_index);
> + scri->l2_bitmap = get_l2_bitmap(s, scri->l2_slice, scri->l2_index);
> + scri->ctype = qcow2_get_cluster_type(bs, scri->l2_entry);
> +
> + sc_cleared = 0;
> + do {
> + ret = qcow2_get_subcluster_range_type(
> + bs, scri->l2_entry, scri->l2_bitmap, sc_index + sc_cleared,
> + &sctype);
> + if (ret < 0) {
> + goto cleanup;
> + }
> +
> + switch (sctype) {
> + case QCOW2_SUBCLUSTER_COMPRESSED:
> + /* We cannot partially zeroize/discard compressed clusters. */
> + ret = -ENOTSUP;
> + goto cleanup;
> + case QCOW2_SUBCLUSTER_INVALID:
> + ret = -EINVAL;
> + goto cleanup;
> + default:
> + break;
> + }
> +
> + sc_cleared += ret;
> + } while (sc_cleared < nb_subclusters);
> +
> + return 0;
> +
> +cleanup:
> + sc_range_info_cleanup(scri);
> + return ret;
> +}
> +
> /*
> * This discards as many clusters of nb_clusters as possible at once (i.e.
> * all clusters in the same L2 slice) and returns the number of discarded
> @@ -2127,46 +2224,25 @@ zero_l2_subclusters(BlockDriverState *bs, uint64_t offset,
> unsigned nb_subclusters)
> {
> BDRVQcow2State *s = bs->opaque;
> - uint64_t *l2_slice;
> - uint64_t old_l2_bitmap, l2_bitmap;
> - int l2_index, ret, sc = offset_to_sc_index(s, offset);
> -
> - /* For full clusters use zero_in_l2_slice() instead */
> - assert(nb_subclusters > 0 && nb_subclusters < s->subclusters_per_cluster);
> - assert(sc + nb_subclusters <= s->subclusters_per_cluster);
> - assert(offset_into_subcluster(s, offset) == 0);
> + uint64_t new_l2_bitmap;
> + int ret, sc = offset_to_sc_index(s, offset);
> + g_auto(SubClusterRangeInfo) scri = { 0 };
>
> - ret = get_cluster_table(bs, offset, &l2_slice, &l2_index);
> + ret = get_sc_range_info(bs, offset, nb_subclusters, &scri);
> if (ret < 0) {
> return ret;
> }
>
> - switch (qcow2_get_cluster_type(bs, get_l2_entry(s, l2_slice, l2_index))) {
> - case QCOW2_CLUSTER_COMPRESSED:
> - ret = -ENOTSUP; /* We cannot partially zeroize compressed clusters */
> - goto out;
> - case QCOW2_CLUSTER_NORMAL:
> - case QCOW2_CLUSTER_UNALLOCATED:
> - break;
> - default:
> - g_assert_not_reached();
> - }
> -
> - old_l2_bitmap = l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index);
> -
> - l2_bitmap |= QCOW_OFLAG_SUB_ZERO_RANGE(sc, sc + nb_subclusters);
> - l2_bitmap &= ~QCOW_OFLAG_SUB_ALLOC_RANGE(sc, sc + nb_subclusters);
> + new_l2_bitmap = scri.l2_bitmap;
> + new_l2_bitmap |= QCOW_OFLAG_SUB_ZERO_RANGE(sc, sc + nb_subclusters);
> + new_l2_bitmap &= ~QCOW_OFLAG_SUB_ALLOC_RANGE(sc, sc + nb_subclusters);
>
> - if (old_l2_bitmap != l2_bitmap) {
> - set_l2_bitmap(s, l2_slice, l2_index, l2_bitmap);
> - qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
> + if (new_l2_bitmap != scri.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);
> }
>
> - ret = 0;
> -out:
> - qcow2_cache_put(s->l2_table_cache, (void **) &l2_slice);
> -
> - return ret;
> + return 0;
> }
>
> int coroutine_fn qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset,
Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
--
Best regards,
Alexander Ivanov
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 08/11] qcow2: zeroize the entire cluster when there're no non-zero subclusters
2024-05-13 6:32 ` [PATCH v2 08/11] qcow2: zeroize the entire cluster when there're no non-zero subclusters Andrey Drobyshev
@ 2024-05-21 9:27 ` Alexander Ivanov
0 siblings, 0 replies; 34+ messages in thread
From: Alexander Ivanov @ 2024-05-21 9:27 UTC (permalink / raw)
To: Andrey Drobyshev, qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis, den
On 5/13/24 08:32, Andrey Drobyshev wrote:
> When zeroizing the last non-zero subclusters within single cluster, it
> makes sense to go zeroize the entire cluster and go down zero_in_l2_slice()
> path right away. That way we'd also update the corresponding refcount
> table.
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
> ---
> block/qcow2-cluster.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 475f167035..8d39e2f960 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -2221,7 +2221,7 @@ zero_in_l2_slice(BlockDriverState *bs, uint64_t offset,
>
> static int coroutine_fn GRAPH_RDLOCK
> zero_l2_subclusters(BlockDriverState *bs, uint64_t offset,
> - unsigned nb_subclusters)
> + unsigned nb_subclusters, int flags)
> {
> BDRVQcow2State *s = bs->opaque;
> uint64_t new_l2_bitmap;
> @@ -2237,6 +2237,16 @@ zero_l2_subclusters(BlockDriverState *bs, uint64_t offset,
> new_l2_bitmap |= QCOW_OFLAG_SUB_ZERO_RANGE(sc, sc + nb_subclusters);
> new_l2_bitmap &= ~QCOW_OFLAG_SUB_ALLOC_RANGE(sc, sc + nb_subclusters);
>
> + /*
> + * If there're no non-zero subclusters left, we might as well zeroize
> + * the entire cluster. That way we'd also update the refcount table.
> + */
> + if ((new_l2_bitmap & QCOW_L2_BITMAP_ALL_ZEROES) ==
> + QCOW_L2_BITMAP_ALL_ZEROES) {
> + return zero_in_l2_slice(bs, QEMU_ALIGN_DOWN(offset, s->cluster_size),
> + 1, flags);
> + }
> +
> if (new_l2_bitmap != scri.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);
> @@ -2293,7 +2303,7 @@ int coroutine_fn qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset,
>
> if (head) {
> ret = zero_l2_subclusters(bs, offset - head,
> - size_to_subclusters(s, head));
> + size_to_subclusters(s, head), flags);
> if (ret < 0) {
> goto fail;
> }
> @@ -2314,7 +2324,8 @@ int coroutine_fn qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset,
> }
>
> if (tail) {
> - ret = zero_l2_subclusters(bs, end_offset, size_to_subclusters(s, tail));
> + ret = zero_l2_subclusters(bs, end_offset,
> + size_to_subclusters(s, tail), flags);
> if (ret < 0) {
> goto fail;
> }
Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
--
Best regards,
Alexander Ivanov
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 09/11] qcow2: make subclusters discardable
2024-05-13 6:32 ` [PATCH v2 09/11] qcow2: make subclusters discardable Andrey Drobyshev
@ 2024-05-21 10:41 ` Alexander Ivanov
0 siblings, 0 replies; 34+ messages in thread
From: Alexander Ivanov @ 2024-05-21 10:41 UTC (permalink / raw)
To: Andrey Drobyshev, qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis, den
On 5/13/24 08:32, 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.
>
> Also rename qcow2_cluster_discard() -> qcow2_subcluster_discard() to
> reflect the change.
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> block/qcow2-cluster.c | 106 +++++++++++++++++++++++++++++++++++++----
> block/qcow2-snapshot.c | 6 +--
> block/qcow2.c | 25 +++++-----
> block/qcow2.h | 4 +-
> tests/qemu-iotests/271 | 2 +-
> 5 files changed, 117 insertions(+), 26 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 8d39e2f960..3c134a7e80 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -2105,25 +2105,106 @@ discard_in_l2_slice(BlockDriverState *bs, uint64_t offset, uint64_t nb_clusters,
> return nb_clusters;
> }
>
> -int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
> - uint64_t bytes, enum qcow2_discard_type type,
> - bool full_discard)
> +static int coroutine_fn GRAPH_RDLOCK
> +discard_l2_subclusters(BlockDriverState *bs, uint64_t offset,
> + uint64_t nb_subclusters,
> + enum qcow2_discard_type type,
> + bool full_discard)
> +{
> + BDRVQcow2State *s = bs->opaque;
> + uint64_t new_l2_bitmap, bitmap_alloc_mask, bitmap_zero_mask;
> + int ret, sc = offset_to_sc_index(s, offset);
> + g_auto(SubClusterRangeInfo) scri = { 0 };
> +
> + ret = get_sc_range_info(bs, offset, nb_subclusters, &scri);
> + if (ret < 0) {
> + return ret;
> + }
> +
> + new_l2_bitmap = scri.l2_bitmap;
> + bitmap_alloc_mask = QCOW_OFLAG_SUB_ALLOC_RANGE(sc, sc + nb_subclusters);
> + bitmap_zero_mask = QCOW_OFLAG_SUB_ZERO_RANGE(sc, sc + nb_subclusters);
> +
> + new_l2_bitmap &= ~bitmap_alloc_mask;
> +
> + /*
> + * Full discard means we fall through to the backing file, thus we need
> + * to mark the subclusters as deallocated and clear the corresponding
> + * zero bits.
> + *
> + * Non-full discard means subclusters should be explicitly marked as
> + * zeroes. In this case QCOW2 specification requires the corresponding
> + * allocation status bits to be unset as well. If the subclusters are
> + * deallocated in the first place and there's no backing, the operation
> + * can be skipped.
> + */
> + if (full_discard) {
> + new_l2_bitmap &= ~bitmap_zero_mask;
> + } else if (bs->backing || scri.l2_bitmap & bitmap_alloc_mask) {
> + new_l2_bitmap |= bitmap_zero_mask;
> + }
> +
> + /*
> + * If after discarding this range there won't be any allocated subclusters
> + * left, and new bitmap becomes the same as it'd be after discarding the
> + * whole cluster, we better discard it entirely. That way we'd also
> + * update the refcount table.
> + */
> + if ((full_discard && new_l2_bitmap == 0) ||
> + (!full_discard && new_l2_bitmap == QCOW_L2_BITMAP_ALL_ZEROES)) {
> + return discard_in_l2_slice(
> + bs, QEMU_ALIGN_DOWN(offset, s->cluster_size),
> + 1, type, full_discard);
> + }
> +
> + 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);
> + }
> +
> + 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;
> +}
> +
> +int qcow2_subcluster_discard(BlockDriverState *bs, uint64_t offset,
> + uint64_t bytes, enum qcow2_discard_type type,
> + bool full_discard)
> {
> 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);
> + if (ret < 0) {
> + goto fail;
> + }
> + }
> +
> + nb_clusters = size_to_clusters(s, end_offset - offset);
> +
> /* Each L2 slice is handled by its own loop iteration */
> while (nb_clusters > 0) {
> cleared = discard_in_l2_slice(bs, offset, nb_clusters, type,
> @@ -2137,6 +2218,15 @@ int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
> offset += (cleared * s->cluster_size);
> }
>
> + if (tail) {
> + ret = discard_l2_subclusters(bs, end_offset,
> + size_to_subclusters(s, tail), type,
> + full_discard);
> + if (ret < 0) {
> + goto fail;
> + }
> + }
> +
> ret = 0;
> fail:
> s->cache_discards = false;
> @@ -2286,8 +2376,8 @@ int coroutine_fn qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset,
> */
> if (s->qcow_version < 3) {
> if (!bs->backing) {
> - return qcow2_cluster_discard(bs, offset, bytes,
> - QCOW2_DISCARD_REQUEST, false);
> + return qcow2_subcluster_discard(bs, offset, bytes,
> + QCOW2_DISCARD_REQUEST, false);
> }
> return -ENOTSUP;
> }
> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
> index 92e47978bf..4e39354d02 100644
> --- a/block/qcow2-snapshot.c
> +++ b/block/qcow2-snapshot.c
> @@ -736,9 +736,9 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
>
> /* The VM state isn't needed any more in the active L1 table; in fact, it
> * hurts by causing expensive COW for the next snapshot. */
> - qcow2_cluster_discard(bs, qcow2_vm_state_offset(s),
> - ROUND_UP(sn->vm_state_size, s->cluster_size),
> - QCOW2_DISCARD_NEVER, false);
> + qcow2_subcluster_discard(bs, qcow2_vm_state_offset(s),
> + ROUND_UP(sn->vm_state_size, s->cluster_size),
> + QCOW2_DISCARD_NEVER, false);
>
> #ifdef DEBUG_ALLOC
> {
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 956128b409..792b106eb6 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1967,7 +1967,7 @@ static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp)
> bs->bl.request_alignment = qcrypto_block_get_sector_size(s->crypto);
> }
> bs->bl.pwrite_zeroes_alignment = s->subcluster_size;
> - bs->bl.pdiscard_alignment = s->cluster_size;
> + bs->bl.pdiscard_alignment = s->subcluster_size;
> }
>
> static int GRAPH_UNLOCKED
> @@ -4112,19 +4112,19 @@ qcow2_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
> return -ENOTSUP;
> }
>
> - if (!QEMU_IS_ALIGNED(offset | bytes, s->cluster_size)) {
> - assert(bytes < s->cluster_size);
> + if (!QEMU_IS_ALIGNED(offset | bytes, bs->bl.pdiscard_alignment)) {
> + assert(bytes < bs->bl.pdiscard_alignment);
> /* Ignore partial clusters, except for the special case of the
> * complete partial cluster at the end of an unaligned file */
> - if (!QEMU_IS_ALIGNED(offset, s->cluster_size) ||
> + if (!QEMU_IS_ALIGNED(offset, bs->bl.pdiscard_alignment) ||
> offset + bytes != bs->total_sectors * BDRV_SECTOR_SIZE) {
> return -ENOTSUP;
> }
> }
>
> qemu_co_mutex_lock(&s->lock);
> - ret = qcow2_cluster_discard(bs, offset, bytes, QCOW2_DISCARD_REQUEST,
> - false);
> + ret = qcow2_subcluster_discard(bs, offset, bytes, QCOW2_DISCARD_REQUEST,
> + false);
> qemu_co_mutex_unlock(&s->lock);
> return ret;
> }
> @@ -4335,10 +4335,10 @@ qcow2_co_truncate(BlockDriverState *bs, int64_t offset, bool exact,
> goto fail;
> }
>
> - ret = qcow2_cluster_discard(bs, ROUND_UP(offset, s->cluster_size),
> - old_length - ROUND_UP(offset,
> - s->cluster_size),
> - QCOW2_DISCARD_ALWAYS, true);
> + ret = qcow2_subcluster_discard(bs, ROUND_UP(offset, s->cluster_size),
> + old_length - ROUND_UP(offset,
> + s->cluster_size),
> + QCOW2_DISCARD_ALWAYS, true);
> if (ret < 0) {
> error_setg_errno(errp, -ret, "Failed to discard cropped clusters");
> goto fail;
> @@ -5032,8 +5032,9 @@ static int GRAPH_RDLOCK qcow2_make_empty(BlockDriverState *bs)
> * default action for this kind of discard is to pass the discard,
> * which will ideally result in an actually smaller image file, as
> * is probably desired. */
> - ret = qcow2_cluster_discard(bs, offset, MIN(step, end_offset - offset),
> - QCOW2_DISCARD_SNAPSHOT, true);
> + ret = qcow2_subcluster_discard(bs, offset,
> + MIN(step, end_offset - offset),
> + QCOW2_DISCARD_SNAPSHOT, true);
> if (ret < 0) {
> break;
> }
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 197bdcdf53..a65c185b51 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -953,8 +953,8 @@ void coroutine_fn GRAPH_RDLOCK
> qcow2_alloc_cluster_abort(BlockDriverState *bs, QCowL2Meta *m);
>
> int GRAPH_RDLOCK
> -qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
> - enum qcow2_discard_type type, bool full_discard);
> +qcow2_subcluster_discard(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
> + enum qcow2_discard_type type, bool full_discard);
>
> int coroutine_fn GRAPH_RDLOCK
> qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
> diff --git a/tests/qemu-iotests/271 b/tests/qemu-iotests/271
> index 59a6fafa2f..04c57813c2 100755
> --- a/tests/qemu-iotests/271
> +++ b/tests/qemu-iotests/271
> @@ -518,7 +518,7 @@ done
> ############################################################
> ############################################################
>
> -# Test qcow2_cluster_discard() with full and normal discards
> +# Test qcow2_subcluster_discard() with full and normal discards
> for use_backing_file in yes no; do
> echo
> echo "### Discarding clusters with non-zero bitmaps (backing file: $use_backing_file) ###"
Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
--
Best regards,
Alexander Ivanov
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 10/11] qcow2: zero_l2_subclusters: fall through to discard operation when requested
2024-05-13 6:32 ` [PATCH v2 10/11] qcow2: zero_l2_subclusters: fall through to discard operation when requested Andrey Drobyshev
@ 2024-05-21 10:46 ` Alexander Ivanov
0 siblings, 0 replies; 34+ messages in thread
From: Alexander Ivanov @ 2024-05-21 10:46 UTC (permalink / raw)
To: Andrey Drobyshev, qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis, den
On 5/13/24 08:32, Andrey Drobyshev wrote:
> When zeroizing subclusters within single cluster, detect usage of the
> BDRV_REQ_MAY_UNMAP flag and fall through to the subcluster-based discard
> operation, much like it's done with the cluster-based discards. That
> way subcluster-aligned operations "qemu-io -c 'write -z -u ...'" will
> lead to actual unmap.
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> block/qcow2-cluster.c | 26 ++++++++++++++++++++------
> 1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 3c134a7e80..53e04eff93 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -2107,15 +2107,16 @@ discard_in_l2_slice(BlockDriverState *bs, uint64_t offset, uint64_t nb_clusters,
>
> static int coroutine_fn GRAPH_RDLOCK
> discard_l2_subclusters(BlockDriverState *bs, uint64_t offset,
> - uint64_t nb_subclusters,
> - enum qcow2_discard_type type,
> - bool full_discard)
> + uint64_t nb_subclusters, enum qcow2_discard_type type,
> + bool full_discard, bool uncond_zeroize)
> {
> BDRVQcow2State *s = bs->opaque;
> uint64_t new_l2_bitmap, bitmap_alloc_mask, bitmap_zero_mask;
> int ret, sc = offset_to_sc_index(s, offset);
> g_auto(SubClusterRangeInfo) scri = { 0 };
>
> + assert(!(full_discard && uncond_zeroize));
> +
> ret = get_sc_range_info(bs, offset, nb_subclusters, &scri);
> if (ret < 0) {
> return ret;
> @@ -2140,7 +2141,8 @@ discard_l2_subclusters(BlockDriverState *bs, uint64_t offset,
> */
> if (full_discard) {
> new_l2_bitmap &= ~bitmap_zero_mask;
> - } else if (bs->backing || scri.l2_bitmap & bitmap_alloc_mask) {
> + } else if (uncond_zeroize || bs->backing ||
> + scri.l2_bitmap & bitmap_alloc_mask) {
> new_l2_bitmap |= bitmap_zero_mask;
> }
>
> @@ -2197,7 +2199,7 @@ int qcow2_subcluster_discard(BlockDriverState *bs, uint64_t offset,
> if (head) {
> ret = discard_l2_subclusters(bs, offset - head,
> size_to_subclusters(s, head), type,
> - full_discard);
> + full_discard, false);
> if (ret < 0) {
> goto fail;
> }
> @@ -2221,7 +2223,7 @@ int qcow2_subcluster_discard(BlockDriverState *bs, uint64_t offset,
> if (tail) {
> ret = discard_l2_subclusters(bs, end_offset,
> size_to_subclusters(s, tail), type,
> - full_discard);
> + full_discard, false);
> if (ret < 0) {
> goto fail;
> }
> @@ -2318,6 +2320,18 @@ zero_l2_subclusters(BlockDriverState *bs, uint64_t offset,
> int ret, sc = offset_to_sc_index(s, offset);
> g_auto(SubClusterRangeInfo) scri = { 0 };
>
> + /*
> + * If the request allows discarding subclusters we go down the discard
> + * path regardless of their allocation status. After the discard
> + * operation with full_discard=false subclusters are going to be read as
> + * zeroes anyway. But we make sure that subclusters are explicitly
> + * zeroed anyway with uncond_zeroize=true.
> + */
> + if (flags & BDRV_REQ_MAY_UNMAP) {
> + return discard_l2_subclusters(bs, offset, nb_subclusters,
> + QCOW2_DISCARD_REQUEST, false, true);
> + }
> +
> ret = get_sc_range_info(bs, offset, nb_subclusters, &scri);
> if (ret < 0) {
> return ret;
Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
--
Best regards,
Alexander Ivanov
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 11/11] iotests/271: add test cases for subcluster-based discard/unmap
2024-05-13 6:32 ` [PATCH v2 11/11] iotests/271: add test cases for subcluster-based discard/unmap Andrey Drobyshev
@ 2024-05-21 10:50 ` Alexander Ivanov
0 siblings, 0 replies; 34+ messages in thread
From: Alexander Ivanov @ 2024-05-21 10:50 UTC (permalink / raw)
To: Andrey Drobyshev, qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis, den
On 5/13/24 08:32, Andrey Drobyshev wrote:
> Add a bunch of test cases covering new subclusters behaviour: unmap of
> last allocated subclusters; unmap of subclusters within unallocated
> cluster; discard of unallocated subclusters within a cluster; regular discard
> of subclusters within a cluster; discard of last allocated subclusters.
>
> Also make all discard/unmap operations enable trace point 'file_do_fallocate'
> so that actual fallocate() calls are visible.
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> tests/qemu-iotests/271 | 70 +++++++++++++++++++++++++++++---------
> tests/qemu-iotests/271.out | 69 ++++++++++++++++++++++++++++++++++---
> 2 files changed, 119 insertions(+), 20 deletions(-)
>
> diff --git a/tests/qemu-iotests/271 b/tests/qemu-iotests/271
> index 04c57813c2..8b80682cff 100755
> --- a/tests/qemu-iotests/271
> +++ b/tests/qemu-iotests/271
> @@ -81,6 +81,12 @@ _verify_l2_bitmap()
> fi
> }
>
> +# Filter out trace params which we don't control (file fd)
> +_filter_trace_fallocate()
> +{
> + sed -E 's/fd=[0-9]+/fd=N/g'
> +}
> +
> # This should be called as _run_test c=XXX sc=XXX off=XXX len=XXX cmd=XXX
> # c: cluster number (0 if unset)
> # sc: subcluster number inside cluster @c (0 if unset)
> @@ -94,12 +100,13 @@ _verify_l2_bitmap()
> # discard -> discard
> _run_test()
> {
> - unset c sc off len cmd
> + unset c sc off len cmd opt
> for var in "$@"; do eval "$var"; done
> case "${cmd:-write}" in
> zero)
> cmd="write -q -z";;
> unmap)
> + opt="--trace enable=file_do_fallocate"
> cmd="write -q -z -u";;
> compress)
> pat=$((${pat:-0} + 1))
> @@ -108,6 +115,7 @@ _run_test()
> pat=$((${pat:-0} + 1))
> cmd="write -q -P ${pat}";;
> discard)
> + opt="--trace enable=file_do_fallocate"
> cmd="discard -q";;
> *)
> echo "Unknown option $cmd"
> @@ -121,7 +129,7 @@ _run_test()
> cmd="$cmd ${offset} ${len}"
> raw_cmd=$(echo $cmd | sed s/-c//) # Raw images don't support -c
> echo $cmd | sed 's/-P [0-9][0-9]\?/-P PATTERN/'
> - $QEMU_IO -c "$cmd" "$TEST_IMG" | _filter_qemu_io
> + $QEMU_IO $opt -c "$cmd" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_trace_fallocate
> $QEMU_IO -c "$raw_cmd" -f raw "$TEST_IMG.raw" | _filter_qemu_io
> _verify_img
> _verify_l2_bitmap "$c"
> @@ -202,9 +210,10 @@ for use_backing_file in yes no; do
> alloc="$(seq 16 31)"; zero="$(seq 0 15)"
> _run_test sc=0 len=32k cmd=unmap
>
> - ### Zero and unmap cluster #0
> + ### Zero and unmap second half of cluster #0 (this will unmap it and
> + ### discard l2 entry)
> alloc=""; zero="$(seq 0 31)"
> - _run_test sc=0 len=64k cmd=unmap
> + _run_test sc=16 len=32k cmd=unmap
>
> ### Write subcluster #1 (middle of subcluster)
> alloc="1"; zero="0 $(seq 2 31)"
> @@ -439,6 +448,12 @@ for use_backing_file in yes no; do
> _verify_l2_bitmap 16
> _verify_l2_bitmap 17
>
> + # Unmap subclusters #0-#3 of an unallocated cluster #8. Since
> + # 'write -z -u' doesn't lead to full discard, subclusters should become
> + # explicitly zeroized
> + alloc=""; zero="$(seq 0 3)"
> + _run_test c=8 sc=0 len=8k cmd=unmap
> +
> # Cluster-aligned request from clusters #9 to #11
> alloc=""; zero="$(seq 0 31)"
> _run_test c=9 sc=0 len=192k cmd=unmap
> @@ -523,26 +538,45 @@ for use_backing_file in yes no; do
> echo
> echo "### Discarding clusters with non-zero bitmaps (backing file: $use_backing_file) ###"
> echo
> +
> + _reset_img 1M
> +
> + # Write first half of cluster #0 and discard another half
> + alloc="$(seq 0 15)"; zero=""
> + _run_test sc=0 len=32k
> + # When discarding unallocated subclusters, they only have to be
> + # explicitly zeroized when the image has a backing file
> if [ "$use_backing_file" = "yes" ]; then
> - _make_test_img -o extended_l2=on -F raw -b "$TEST_IMG.base" 1M
> + alloc="$(seq 0 15)"; zero="$(seq 16 31)"
> else
> - _make_test_img -o extended_l2=on 1M
> + alloc="$(seq 0 15)"; zero=""
> fi
> - # Write clusters #0-#2 and then discard them
> - $QEMU_IO -c 'write -q 0 128k' "$TEST_IMG"
> - $QEMU_IO -c 'discard -q 0 128k' "$TEST_IMG"
> + _run_test sc=16 len=32k cmd=discard
> +
> + # Write cluster #0 and discard its subclusters #0-#3
> + alloc="$(seq 0 31)"; zero=""
> + _run_test sc=0 len=64k
> + alloc="$(seq 4 31)"; zero="$(seq 0 3)"
> + _run_test sc=0 len=8k cmd=discard
> +
> + # Discard remaining subclusters #4-#32. This should unmap the cluster
> + # and discard its l2 entry
> + alloc=""; zero="$(seq 0 31)"
> + _run_test sc=4 len=56k cmd=discard
> +
> + # Write clusters #0-#1 and then discard them
> + alloc="$(seq 0 31)"; zero=""
> + _run_test sc=0 len=128k
> # 'qemu-io discard' doesn't do a full discard, it zeroizes the
> - # cluster, so both clusters have all zero bits set now
> + # cluster, so both clusters have all zero bits set after discard
> alloc=""; zero="$(seq 0 31)"
> - _verify_l2_bitmap 0
> + _run_test sc=0 len=128k cmd=discard
> _verify_l2_bitmap 1
> +
> # Now mark the 2nd half of the subclusters from cluster #0 as unallocated
> poke_file "$TEST_IMG" $(($l2_offset+8)) "\x00\x00"
> +
> # Discard cluster #0 again to see how the zero bits have changed
> - $QEMU_IO -c 'discard -q 0 64k' "$TEST_IMG"
> - # And do a full discard of cluster #1 by shrinking and growing the image
> - $QEMU_IMG resize --shrink "$TEST_IMG" 64k
> - $QEMU_IMG resize "$TEST_IMG" 1M
> # A normal discard sets all 'zero' bits only if the image has a
> # backing file, otherwise it won't touch them.
> if [ "$use_backing_file" = "yes" ]; then
> @@ -550,7 +584,11 @@ for use_backing_file in yes no; do
> else
> alloc=""; zero="$(seq 0 15)"
> fi
> - _verify_l2_bitmap 0
> + _run_test sc=0 len=64k cmd=discard
> +
> + # And do a full discard of cluster #1 by shrinking and growing the image
> + $QEMU_IMG resize --shrink "$TEST_IMG" 64k
> + $QEMU_IMG resize "$TEST_IMG" 1M
> # A full discard should clear the L2 entry completely. However
> # when growing an image with a backing file the new clusters are
> # zeroized to hide the stale data from the backing file
> diff --git a/tests/qemu-iotests/271.out b/tests/qemu-iotests/271.out
> index 0b24d50159..c03e4c9bc2 100644
> --- a/tests/qemu-iotests/271.out
> +++ b/tests/qemu-iotests/271.out
> @@ -29,14 +29,17 @@ L2 entry #0: 0x8000000000050000 ffffffff00000000
> write -q -P PATTERN 0 64k
> L2 entry #0: 0x8000000000050000 00000000ffffffff
> write -q -z -u 0 32k
> +file_do_fallocate fd=N mode=0x03 offset=327680 len=32768
> L2 entry #0: 0x8000000000050000 0000ffffffff0000
> -write -q -z -u 0 64k
> +write -q -z -u 32k 32k
> +file_do_fallocate fd=N mode=0x03 offset=327680 len=65536
> L2 entry #0: 0x0000000000000000 ffffffff00000000
> write -q -P PATTERN 3k 512
> L2 entry #0: 0x8000000000050000 fffffffd00000002
> write -q -P PATTERN 0 64k
> L2 entry #0: 0x8000000000050000 00000000ffffffff
> discard -q 0 64k
> +file_do_fallocate fd=N mode=0x03 offset=327680 len=65536
> L2 entry #0: 0x0000000000000000 ffffffff00000000
> write -q -c -P PATTERN 0 64k
> L2 entry #0: 0x4000000000050000 0000000000000000
> @@ -71,14 +74,17 @@ L2 entry #0: 0x8000000000050000 ffffffff00000000
> write -q -P PATTERN 0 64k
> L2 entry #0: 0x8000000000050000 00000000ffffffff
> write -q -z -u 0 32k
> +file_do_fallocate fd=N mode=0x03 offset=327680 len=32768
> L2 entry #0: 0x8000000000050000 0000ffffffff0000
> -write -q -z -u 0 64k
> +write -q -z -u 32k 32k
> +file_do_fallocate fd=N mode=0x03 offset=327680 len=65536
> L2 entry #0: 0x0000000000000000 ffffffff00000000
> write -q -P PATTERN 3k 512
> L2 entry #0: 0x8000000000050000 fffffffd00000002
> write -q -P PATTERN 0 64k
> L2 entry #0: 0x8000000000050000 00000000ffffffff
> discard -q 0 64k
> +file_do_fallocate fd=N mode=0x03 offset=327680 len=65536
> L2 entry #0: 0x0000000000000000 ffffffff00000000
> write -q -c -P PATTERN 0 64k
> L2 entry #0: 0x4000000000050000 0000000000000000
> @@ -301,15 +307,20 @@ L2 entry #14: 0x80000000000a0000 00000000ffffffff
> L2 entry #15: 0x80000000000b0000 00000000ffffffff
> L2 entry #16: 0x80000000000c0000 00000000ffffffff
> L2 entry #17: 0x80000000000d0000 00000000ffffffff
> +write -q -z -u 512k 8k
> +L2 entry #8: 0x0000000000000000 0000000f00000000
> write -q -z -u 576k 192k
> +file_do_fallocate fd=N mode=0x03 offset=327680 len=196608
> L2 entry #9: 0x0000000000000000 ffffffff00000000
> L2 entry #10: 0x0000000000000000 ffffffff00000000
> L2 entry #11: 0x0000000000000000 ffffffff00000000
> write -q -z -u 800k 128k
> +file_do_fallocate fd=N mode=0x03 offset=557056 len=131072
> L2 entry #12: 0x8000000000080000 ffff00000000ffff
> L2 entry #13: 0x0000000000000000 ffffffff00000000
> L2 entry #14: 0x80000000000a0000 0000ffffffff0000
> write -q -z -u 991k 128k
> +file_do_fallocate fd=N mode=0x03 offset=753664 len=129024
> L2 entry #15: 0x80000000000b0000 ffff00000000ffff
> L2 entry #16: 0x0000000000000000 ffffffff00000000
> L2 entry #17: 0x80000000000d0000 00007fffffff8000
> @@ -339,6 +350,7 @@ L2 entry #27: 0x4000000000120000 0000000000000000
> write -q -c -P PATTERN 1792k 64k
> L2 entry #28: 0x4000000000130000 0000000000000000
> write -q -z -u 1152k 192k
> +file_do_fallocate fd=N mode=0x03 offset=327680 len=196608
> L2 entry #18: 0x0000000000000000 ffffffff00000000
> L2 entry #19: 0x0000000000000000 ffffffff00000000
> L2 entry #20: 0x0000000000000000 ffffffff00000000
> @@ -351,6 +363,8 @@ L2 entry #24: 0x8000000000090000 00000000ffffffff
> L2 entry #25: 0x80000000000e0000 00000000ffffffff
> L2 entry #26: 0x80000000000f0000 00000000ffffffff
> write -q -z -u 1759k 128k
> +file_do_fallocate fd=N mode=0x03 offset=819200 len=32768
> +file_do_fallocate fd=N mode=0x03 offset=1245184 len=65536
> L2 entry #27: 0x80000000000c0000 ffff00000000ffff
> L2 entry #28: 0x0000000000000000 ffffffff00000000
> L2 entry #29: 0x8000000000100000 00007fff00008000
> @@ -369,15 +383,20 @@ L2 entry #14: 0x80000000000a0000 00000000ffffffff
> L2 entry #15: 0x80000000000b0000 00000000ffffffff
> L2 entry #16: 0x80000000000c0000 00000000ffffffff
> L2 entry #17: 0x80000000000d0000 00000000ffffffff
> +write -q -z -u 512k 8k
> +L2 entry #8: 0x0000000000000000 0000000f00000000
> write -q -z -u 576k 192k
> +file_do_fallocate fd=N mode=0x03 offset=327680 len=196608
> L2 entry #9: 0x0000000000000000 ffffffff00000000
> L2 entry #10: 0x0000000000000000 ffffffff00000000
> L2 entry #11: 0x0000000000000000 ffffffff00000000
> write -q -z -u 800k 128k
> +file_do_fallocate fd=N mode=0x03 offset=557056 len=131072
> L2 entry #12: 0x8000000000080000 ffff00000000ffff
> L2 entry #13: 0x0000000000000000 ffffffff00000000
> L2 entry #14: 0x80000000000a0000 0000ffffffff0000
> write -q -z -u 991k 128k
> +file_do_fallocate fd=N mode=0x03 offset=753664 len=129024
> L2 entry #15: 0x80000000000b0000 ffff00000000ffff
> L2 entry #16: 0x0000000000000000 ffffffff00000000
> L2 entry #17: 0x80000000000d0000 00007fffffff8000
> @@ -407,6 +426,7 @@ L2 entry #27: 0x4000000000120000 0000000000000000
> write -q -c -P PATTERN 1792k 64k
> L2 entry #28: 0x4000000000130000 0000000000000000
> write -q -z -u 1152k 192k
> +file_do_fallocate fd=N mode=0x03 offset=327680 len=196608
> L2 entry #18: 0x0000000000000000 ffffffff00000000
> L2 entry #19: 0x0000000000000000 ffffffff00000000
> L2 entry #20: 0x0000000000000000 ffffffff00000000
> @@ -419,28 +439,69 @@ L2 entry #24: 0x8000000000090000 00000000ffffffff
> L2 entry #25: 0x80000000000e0000 00000000ffffffff
> L2 entry #26: 0x80000000000f0000 00000000ffffffff
> write -q -z -u 1759k 128k
> +file_do_fallocate fd=N mode=0x03 offset=819200 len=32768
> +file_do_fallocate fd=N mode=0x03 offset=1245184 len=65536
> L2 entry #27: 0x80000000000c0000 ffff00000000ffff
> L2 entry #28: 0x0000000000000000 ffffffff00000000
> L2 entry #29: 0x0000000000000000 0000ffff00000000
>
> ### Discarding clusters with non-zero bitmaps (backing file: yes) ###
>
> +Formatting 'TEST_DIR/t.IMGFMT.raw', fmt=raw size=1048576
> +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=raw size=1048576
> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=raw
> +write -q -P PATTERN 0 32k
> +L2 entry #0: 0x8000000000050000 000000000000ffff
> +discard -q 32k 32k
> +file_do_fallocate fd=N mode=0x03 offset=360448 len=32768
> +L2 entry #0: 0x8000000000050000 ffff00000000ffff
> +write -q -P PATTERN 0 64k
> +L2 entry #0: 0x8000000000050000 00000000ffffffff
> +discard -q 0 8k
> +file_do_fallocate fd=N mode=0x03 offset=327680 len=8192
> +L2 entry #0: 0x8000000000050000 0000000ffffffff0
> +discard -q 8k 56k
> +file_do_fallocate fd=N mode=0x03 offset=327680 len=65536
> +L2 entry #0: 0x0000000000000000 ffffffff00000000
> +write -q -P PATTERN 0 128k
> +L2 entry #0: 0x8000000000050000 00000000ffffffff
> +discard -q 0 128k
> +file_do_fallocate fd=N mode=0x03 offset=327680 len=131072
> L2 entry #0: 0x0000000000000000 ffffffff00000000
> L2 entry #1: 0x0000000000000000 ffffffff00000000
> +discard -q 0 64k
> +L2 entry #0: 0x0000000000000000 ffffffff00000000
> Image resized.
> Image resized.
> -L2 entry #0: 0x0000000000000000 ffffffff00000000
> L2 entry #1: 0x0000000000000000 ffffffff00000000
>
> ### Discarding clusters with non-zero bitmaps (backing file: no) ###
>
> +Formatting 'TEST_DIR/t.IMGFMT.raw', fmt=raw size=1048576
> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
> +write -q -P PATTERN 0 32k
> +L2 entry #0: 0x8000000000050000 000000000000ffff
> +discard -q 32k 32k
> +file_do_fallocate fd=N mode=0x03 offset=360448 len=32768
> +L2 entry #0: 0x8000000000050000 000000000000ffff
> +write -q -P PATTERN 0 64k
> +L2 entry #0: 0x8000000000050000 00000000ffffffff
> +discard -q 0 8k
> +file_do_fallocate fd=N mode=0x03 offset=327680 len=8192
> +L2 entry #0: 0x8000000000050000 0000000ffffffff0
> +discard -q 8k 56k
> +file_do_fallocate fd=N mode=0x03 offset=327680 len=65536
> +L2 entry #0: 0x0000000000000000 ffffffff00000000
> +write -q -P PATTERN 0 128k
> +L2 entry #0: 0x8000000000050000 00000000ffffffff
> +discard -q 0 128k
> +file_do_fallocate fd=N mode=0x03 offset=327680 len=131072
> L2 entry #0: 0x0000000000000000 ffffffff00000000
> L2 entry #1: 0x0000000000000000 ffffffff00000000
> +discard -q 0 64k
> +L2 entry #0: 0x0000000000000000 0000ffff00000000
> Image resized.
> Image resized.
> -L2 entry #0: 0x0000000000000000 0000ffff00000000
> L2 entry #1: 0x0000000000000000 0000000000000000
>
> ### Corrupted L2 entries - read test (allocated) ###
Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
--
Best regards,
Alexander Ivanov
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 04/11] block/file-posix: add trace event for fallocate() calls
2024-05-13 6:31 ` [PATCH v2 04/11] block/file-posix: add trace event for fallocate() calls Andrey Drobyshev
2024-05-21 8:54 ` Alexander Ivanov
@ 2024-05-24 6:40 ` Alberto Garcia
1 sibling, 0 replies; 34+ messages in thread
From: Alberto Garcia @ 2024-05-24 6:40 UTC (permalink / raw)
To: Andrey Drobyshev, qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, jean-louis, andrey.drobyshev,
den
On Mon 13 May 2024 09:31:56 AM +03, Andrey Drobyshev wrote:
> This would ease debugging of write zeroes and discard operations.
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Berto
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 05/11] iotests/common.rc: add disk_usage function
2024-05-13 6:31 ` [PATCH v2 05/11] iotests/common.rc: add disk_usage function Andrey Drobyshev
2024-05-21 8:56 ` Alexander Ivanov
@ 2024-05-24 6:41 ` Alberto Garcia
1 sibling, 0 replies; 34+ messages in thread
From: Alberto Garcia @ 2024-05-24 6:41 UTC (permalink / raw)
To: Andrey Drobyshev, qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, jean-louis, andrey.drobyshev,
den
On Mon 13 May 2024 09:31:57 AM +03, Andrey Drobyshev wrote:
> Move the definition from iotests/250 to common.rc. This is used to
> detect real disk usage of sparse files. In particular, we want to use
> it for checking subclusters-based discards.
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Berto
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 06/11] iotests/290: add test case to check 'discard-no-unref' option behavior
2024-05-13 6:31 ` [PATCH v2 06/11] iotests/290: add test case to check 'discard-no-unref' option behavior Andrey Drobyshev
2024-05-21 9:09 ` Alexander Ivanov
@ 2024-05-24 6:42 ` Alberto Garcia
1 sibling, 0 replies; 34+ messages in thread
From: Alberto Garcia @ 2024-05-24 6:42 UTC (permalink / raw)
To: Andrey Drobyshev, qemu-block
Cc: qemu-devel, hreitz, kwolf, eblake, jean-louis, andrey.drobyshev,
den
On Mon 13 May 2024 09:31:58 AM +03, Andrey Drobyshev wrote:
> We basically fill 2 images with identical data and perform discard
> operations with and without 'discard-no-unref' enabled. Then we check
> that images still read identically, that their disk usage is the same
> (i.e. fallocate(FALLOC_FL_PUNCH_HOLE|FALLOC_FL_KEEP_SIZE) is called for
> both) and that with the option enabled cluster is still marked as
> allocated in "qemu-img map" output. We also check that the option
> doesn't work with qcow2 v2 images.
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Berto
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 00/11] qcow2: make subclusters discardable
2024-05-13 6:31 [PATCH v2 00/11] qcow2: make subclusters discardable Andrey Drobyshev
` (10 preceding siblings ...)
2024-05-13 6:32 ` [PATCH v2 11/11] iotests/271: add test cases for subcluster-based discard/unmap Andrey Drobyshev
@ 2024-06-03 9:19 ` Andrey Drobyshev
2024-06-10 9:53 ` Andrey Drobyshev
11 siblings, 1 reply; 34+ messages in thread
From: Andrey Drobyshev @ 2024-06-03 9:19 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis, den
On 5/13/24 9:31 AM, Andrey Drobyshev wrote:
> v1: https://lists.nongnu.org/archive/html/qemu-devel/2023-10/msg07223.html
>
> Andrey Drobyshev (11):
> qcow2: make function update_refcount_discard() global
> qcow2: simplify L2 entries accounting for discard-no-unref
> qcow2: put discard requests in the common queue when discard-no-unref
> enabled
> block/file-posix: add trace event for fallocate() calls
> iotests/common.rc: add disk_usage function
> iotests/290: add test case to check 'discard-no-unref' option behavior
> qcow2: add get_sc_range_info() helper for working with subcluster
> ranges
> qcow2: zeroize the entire cluster when there're no non-zero
> subclusters
> qcow2: make subclusters discardable
> qcow2: zero_l2_subclusters: fall through to discard operation when
> requested
> iotests/271: add test cases for subcluster-based discard/unmap
>
> block/file-posix.c | 1 +
> block/qcow2-cluster.c | 346 ++++++++++++++++++++++++++++-------
> block/qcow2-refcount.c | 8 +-
> block/qcow2-snapshot.c | 6 +-
> block/qcow2.c | 25 +--
> block/qcow2.h | 6 +-
> block/trace-events | 1 +
> tests/qemu-iotests/250 | 5 -
> tests/qemu-iotests/271 | 72 ++++++--
> tests/qemu-iotests/271.out | 69 ++++++-
> tests/qemu-iotests/290 | 34 ++++
> tests/qemu-iotests/290.out | 28 +++
> tests/qemu-iotests/common.rc | 6 +
> 13 files changed, 490 insertions(+), 117 deletions(-)
>
Friendly ping
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 00/11] qcow2: make subclusters discardable
2024-06-03 9:19 ` [PATCH v2 00/11] qcow2: make subclusters discardable Andrey Drobyshev
@ 2024-06-10 9:53 ` Andrey Drobyshev
2024-06-17 7:39 ` Andrey Drobyshev
0 siblings, 1 reply; 34+ messages in thread
From: Andrey Drobyshev @ 2024-06-10 9:53 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis, den
On 6/3/24 12:19 PM, Andrey Drobyshev wrote:
> On 5/13/24 9:31 AM, Andrey Drobyshev wrote:
>> v1: https://lists.nongnu.org/archive/html/qemu-devel/2023-10/msg07223.html
>>
>> Andrey Drobyshev (11):
>> qcow2: make function update_refcount_discard() global
>> qcow2: simplify L2 entries accounting for discard-no-unref
>> qcow2: put discard requests in the common queue when discard-no-unref
>> enabled
>> block/file-posix: add trace event for fallocate() calls
>> iotests/common.rc: add disk_usage function
>> iotests/290: add test case to check 'discard-no-unref' option behavior
>> qcow2: add get_sc_range_info() helper for working with subcluster
>> ranges
>> qcow2: zeroize the entire cluster when there're no non-zero
>> subclusters
>> qcow2: make subclusters discardable
>> qcow2: zero_l2_subclusters: fall through to discard operation when
>> requested
>> iotests/271: add test cases for subcluster-based discard/unmap
>>
>> block/file-posix.c | 1 +
>> block/qcow2-cluster.c | 346 ++++++++++++++++++++++++++++-------
>> block/qcow2-refcount.c | 8 +-
>> block/qcow2-snapshot.c | 6 +-
>> block/qcow2.c | 25 +--
>> block/qcow2.h | 6 +-
>> block/trace-events | 1 +
>> tests/qemu-iotests/250 | 5 -
>> tests/qemu-iotests/271 | 72 ++++++--
>> tests/qemu-iotests/271.out | 69 ++++++-
>> tests/qemu-iotests/290 | 34 ++++
>> tests/qemu-iotests/290.out | 28 +++
>> tests/qemu-iotests/common.rc | 6 +
>> 13 files changed, 490 insertions(+), 117 deletions(-)
>>
>
> Friendly ping
Ping
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 00/11] qcow2: make subclusters discardable
2024-06-10 9:53 ` Andrey Drobyshev
@ 2024-06-17 7:39 ` Andrey Drobyshev
2024-06-24 7:43 ` Andrey Drobyshev
0 siblings, 1 reply; 34+ messages in thread
From: Andrey Drobyshev @ 2024-06-17 7:39 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis, den
On 6/10/24 11:53 AM, Andrey Drobyshev wrote:
> On 6/3/24 12:19 PM, Andrey Drobyshev wrote:
>> On 5/13/24 9:31 AM, Andrey Drobyshev wrote:
>>> v1: https://lists.nongnu.org/archive/html/qemu-devel/2023-10/msg07223.html
>>>
>>> Andrey Drobyshev (11):
>>> qcow2: make function update_refcount_discard() global
>>> qcow2: simplify L2 entries accounting for discard-no-unref
>>> qcow2: put discard requests in the common queue when discard-no-unref
>>> enabled
>>> block/file-posix: add trace event for fallocate() calls
>>> iotests/common.rc: add disk_usage function
>>> iotests/290: add test case to check 'discard-no-unref' option behavior
>>> qcow2: add get_sc_range_info() helper for working with subcluster
>>> ranges
>>> qcow2: zeroize the entire cluster when there're no non-zero
>>> subclusters
>>> qcow2: make subclusters discardable
>>> qcow2: zero_l2_subclusters: fall through to discard operation when
>>> requested
>>> iotests/271: add test cases for subcluster-based discard/unmap
>>>
>>> block/file-posix.c | 1 +
>>> block/qcow2-cluster.c | 346 ++++++++++++++++++++++++++++-------
>>> block/qcow2-refcount.c | 8 +-
>>> block/qcow2-snapshot.c | 6 +-
>>> block/qcow2.c | 25 +--
>>> block/qcow2.h | 6 +-
>>> block/trace-events | 1 +
>>> tests/qemu-iotests/250 | 5 -
>>> tests/qemu-iotests/271 | 72 ++++++--
>>> tests/qemu-iotests/271.out | 69 ++++++-
>>> tests/qemu-iotests/290 | 34 ++++
>>> tests/qemu-iotests/290.out | 28 +++
>>> tests/qemu-iotests/common.rc | 6 +
>>> 13 files changed, 490 insertions(+), 117 deletions(-)
>>>
>>
>> Friendly ping
>
> Ping
Friendly ping
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 00/11] qcow2: make subclusters discardable
2024-06-17 7:39 ` Andrey Drobyshev
@ 2024-06-24 7:43 ` Andrey Drobyshev
2024-07-08 7:06 ` Andrey Drobyshev
0 siblings, 1 reply; 34+ messages in thread
From: Andrey Drobyshev @ 2024-06-24 7:43 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis, den
On 6/17/24 9:39 AM, Andrey Drobyshev wrote:
> On 6/10/24 11:53 AM, Andrey Drobyshev wrote:
>> On 6/3/24 12:19 PM, Andrey Drobyshev wrote:
>>> On 5/13/24 9:31 AM, Andrey Drobyshev wrote:
>>>> v1: https://lists.nongnu.org/archive/html/qemu-devel/2023-10/msg07223.html
>>>>
>>>> Andrey Drobyshev (11):
>>>> qcow2: make function update_refcount_discard() global
>>>> qcow2: simplify L2 entries accounting for discard-no-unref
>>>> qcow2: put discard requests in the common queue when discard-no-unref
>>>> enabled
>>>> block/file-posix: add trace event for fallocate() calls
>>>> iotests/common.rc: add disk_usage function
>>>> iotests/290: add test case to check 'discard-no-unref' option behavior
>>>> qcow2: add get_sc_range_info() helper for working with subcluster
>>>> ranges
>>>> qcow2: zeroize the entire cluster when there're no non-zero
>>>> subclusters
>>>> qcow2: make subclusters discardable
>>>> qcow2: zero_l2_subclusters: fall through to discard operation when
>>>> requested
>>>> iotests/271: add test cases for subcluster-based discard/unmap
>>>>
>>>> block/file-posix.c | 1 +
>>>> block/qcow2-cluster.c | 346 ++++++++++++++++++++++++++++-------
>>>> block/qcow2-refcount.c | 8 +-
>>>> block/qcow2-snapshot.c | 6 +-
>>>> block/qcow2.c | 25 +--
>>>> block/qcow2.h | 6 +-
>>>> block/trace-events | 1 +
>>>> tests/qemu-iotests/250 | 5 -
>>>> tests/qemu-iotests/271 | 72 ++++++--
>>>> tests/qemu-iotests/271.out | 69 ++++++-
>>>> tests/qemu-iotests/290 | 34 ++++
>>>> tests/qemu-iotests/290.out | 28 +++
>>>> tests/qemu-iotests/common.rc | 6 +
>>>> 13 files changed, 490 insertions(+), 117 deletions(-)
>>>>
>>>
>>> Friendly ping
>>
>> Ping
>
> Friendly ping
Yet another one
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 00/11] qcow2: make subclusters discardable
2024-06-24 7:43 ` Andrey Drobyshev
@ 2024-07-08 7:06 ` Andrey Drobyshev
2024-07-15 7:44 ` Andrey Drobyshev
0 siblings, 1 reply; 34+ messages in thread
From: Andrey Drobyshev @ 2024-07-08 7:06 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, hreitz, kwolf, eblake, berto, jean-louis, den
On 6/24/24 10:43 AM, Andrey Drobyshev wrote:
> On 6/17/24 9:39 AM, Andrey Drobyshev wrote:
>> On 6/10/24 11:53 AM, Andrey Drobyshev wrote:
>>> On 6/3/24 12:19 PM, Andrey Drobyshev wrote:
>>>> On 5/13/24 9:31 AM, Andrey Drobyshev wrote:
>>>>> v1: https://lists.nongnu.org/archive/html/qemu-devel/2023-10/msg07223.html
>>>>>
>>>>> Andrey Drobyshev (11):
>>>>> qcow2: make function update_refcount_discard() global
>>>>> qcow2: simplify L2 entries accounting for discard-no-unref
>>>>> qcow2: put discard requests in the common queue when discard-no-unref
>>>>> enabled
>>>>> block/file-posix: add trace event for fallocate() calls
>>>>> iotests/common.rc: add disk_usage function
>>>>> iotests/290: add test case to check 'discard-no-unref' option behavior
>>>>> qcow2: add get_sc_range_info() helper for working with subcluster
>>>>> ranges
>>>>> qcow2: zeroize the entire cluster when there're no non-zero
>>>>> subclusters
>>>>> qcow2: make subclusters discardable
>>>>> qcow2: zero_l2_subclusters: fall through to discard operation when
>>>>> requested
>>>>> iotests/271: add test cases for subcluster-based discard/unmap
>>>>>
>>>>> block/file-posix.c | 1 +
>>>>> block/qcow2-cluster.c | 346 ++++++++++++++++++++++++++++-------
>>>>> block/qcow2-refcount.c | 8 +-
>>>>> block/qcow2-snapshot.c | 6 +-
>>>>> block/qcow2.c | 25 +--
>>>>> block/qcow2.h | 6 +-
>>>>> block/trace-events | 1 +
>>>>> tests/qemu-iotests/250 | 5 -
>>>>> tests/qemu-iotests/271 | 72 ++++++--
>>>>> tests/qemu-iotests/271.out | 69 ++++++-
>>>>> tests/qemu-iotests/290 | 34 ++++
>>>>> tests/qemu-iotests/290.out | 28 +++
>>>>> tests/qemu-iotests/common.rc | 6 +
>>>>> 13 files changed, 490 insertions(+), 117 deletions(-)
>>>>>
>>>>
>>>> Friendly ping
>>>
>>> Ping
>>
>> Friendly ping
>
> Yet another one
Ping
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 00/11] qcow2: make subclusters discardable
2024-07-08 7:06 ` Andrey Drobyshev
@ 2024-07-15 7:44 ` Andrey Drobyshev
0 siblings, 0 replies; 34+ messages in thread
From: Andrey Drobyshev @ 2024-07-15 7:44 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, hreitz, kwolf, eblake, berto, den, stefanha
On 7/8/24 10:06 AM, Andrey Drobyshev wrote:
> On 6/24/24 10:43 AM, Andrey Drobyshev wrote:
>> On 6/17/24 9:39 AM, Andrey Drobyshev wrote:
>>> On 6/10/24 11:53 AM, Andrey Drobyshev wrote:
>>>> On 6/3/24 12:19 PM, Andrey Drobyshev wrote:
>>>>> On 5/13/24 9:31 AM, Andrey Drobyshev wrote:
>>>>>> v1: https://lists.nongnu.org/archive/html/qemu-devel/2023-10/msg07223.html
>>>>>>
>>>>>> Andrey Drobyshev (11):
>>>>>> qcow2: make function update_refcount_discard() global
>>>>>> qcow2: simplify L2 entries accounting for discard-no-unref
>>>>>> qcow2: put discard requests in the common queue when discard-no-unref
>>>>>> enabled
>>>>>> block/file-posix: add trace event for fallocate() calls
>>>>>> iotests/common.rc: add disk_usage function
>>>>>> iotests/290: add test case to check 'discard-no-unref' option behavior
>>>>>> qcow2: add get_sc_range_info() helper for working with subcluster
>>>>>> ranges
>>>>>> qcow2: zeroize the entire cluster when there're no non-zero
>>>>>> subclusters
>>>>>> qcow2: make subclusters discardable
>>>>>> qcow2: zero_l2_subclusters: fall through to discard operation when
>>>>>> requested
>>>>>> iotests/271: add test cases for subcluster-based discard/unmap
>>>>>>
>>>>>> block/file-posix.c | 1 +
>>>>>> block/qcow2-cluster.c | 346 ++++++++++++++++++++++++++++-------
>>>>>> block/qcow2-refcount.c | 8 +-
>>>>>> block/qcow2-snapshot.c | 6 +-
>>>>>> block/qcow2.c | 25 +--
>>>>>> block/qcow2.h | 6 +-
>>>>>> block/trace-events | 1 +
>>>>>> tests/qemu-iotests/250 | 5 -
>>>>>> tests/qemu-iotests/271 | 72 ++++++--
>>>>>> tests/qemu-iotests/271.out | 69 ++++++-
>>>>>> tests/qemu-iotests/290 | 34 ++++
>>>>>> tests/qemu-iotests/290.out | 28 +++
>>>>>> tests/qemu-iotests/common.rc | 6 +
>>>>>> 13 files changed, 490 insertions(+), 117 deletions(-)
>>>>>>
>>>>>
>>>>> Friendly ping
>>>>
>>>> Ping
>>>
>>> Friendly ping
>>
>> Yet another one
>
> Ping
Another ping
^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2024-07-15 7:46 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-13 6:31 [PATCH v2 00/11] qcow2: make subclusters discardable Andrey Drobyshev
2024-05-13 6:31 ` [PATCH v2 01/11] qcow2: make function update_refcount_discard() global Andrey Drobyshev
2024-05-15 22:26 ` Alberto Garcia
2024-05-21 8:31 ` Alexander Ivanov
2024-05-13 6:31 ` [PATCH v2 02/11] qcow2: simplify L2 entries accounting for discard-no-unref Andrey Drobyshev
2024-05-15 22:28 ` Alberto Garcia
2024-05-21 8:32 ` Alexander Ivanov
2024-05-13 6:31 ` [PATCH v2 03/11] qcow2: put discard requests in the common queue when discard-no-unref enabled Andrey Drobyshev
2024-05-21 8:43 ` Alexander Ivanov
2024-05-13 6:31 ` [PATCH v2 04/11] block/file-posix: add trace event for fallocate() calls Andrey Drobyshev
2024-05-21 8:54 ` Alexander Ivanov
2024-05-24 6:40 ` Alberto Garcia
2024-05-13 6:31 ` [PATCH v2 05/11] iotests/common.rc: add disk_usage function Andrey Drobyshev
2024-05-21 8:56 ` Alexander Ivanov
2024-05-24 6:41 ` Alberto Garcia
2024-05-13 6:31 ` [PATCH v2 06/11] iotests/290: add test case to check 'discard-no-unref' option behavior Andrey Drobyshev
2024-05-21 9:09 ` Alexander Ivanov
2024-05-24 6:42 ` Alberto Garcia
2024-05-13 6:31 ` [PATCH v2 07/11] qcow2: add get_sc_range_info() helper for working with subcluster ranges Andrey Drobyshev
2024-05-21 9:17 ` Alexander Ivanov
2024-05-13 6:32 ` [PATCH v2 08/11] qcow2: zeroize the entire cluster when there're no non-zero subclusters Andrey Drobyshev
2024-05-21 9:27 ` Alexander Ivanov
2024-05-13 6:32 ` [PATCH v2 09/11] qcow2: make subclusters discardable Andrey Drobyshev
2024-05-21 10:41 ` Alexander Ivanov
2024-05-13 6:32 ` [PATCH v2 10/11] qcow2: zero_l2_subclusters: fall through to discard operation when requested Andrey Drobyshev
2024-05-21 10:46 ` Alexander Ivanov
2024-05-13 6:32 ` [PATCH v2 11/11] iotests/271: add test cases for subcluster-based discard/unmap Andrey Drobyshev
2024-05-21 10:50 ` Alexander Ivanov
2024-06-03 9:19 ` [PATCH v2 00/11] qcow2: make subclusters discardable Andrey Drobyshev
2024-06-10 9:53 ` Andrey Drobyshev
2024-06-17 7:39 ` Andrey Drobyshev
2024-06-24 7:43 ` Andrey Drobyshev
2024-07-08 7:06 ` Andrey Drobyshev
2024-07-15 7:44 ` Andrey Drobyshev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).