[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] qcow2: Set zero flag for discarded clusters
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v2] qcow2: Set zero flag for discarded clusters |
Date: |
Fri, 14 Feb 2014 18:05:13 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Sat, Feb 08, 2014 at 05:50:02PM +0100, Kevin Wolf wrote:
> Instead of making the backing file contents visible again after a discard
> request, set the zero flag if possible (i.e. on version >= 3).
>
> Signed-off-by: Kevin Wolf <address@hidden>
> Reviewed-by: Max Reitz <address@hidden>
> ---
> block/qcow2-cluster.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 25d45d1..9461969 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1333,13 +1333,31 @@ static int discard_single_l2(BlockDriverState *bs,
> uint64_t offset,
> uint64_t old_offset;
>
> old_offset = be64_to_cpu(l2_table[l2_index + i]);
> - if ((old_offset & L2E_OFFSET_MASK) == 0) {
> +
> + /*
> + * Make sure that a discarded area reads back as zeroes for v3 images
> + * (we cannot do it for v2 without actually writing a zero-filled
> + * buffer). We can skip the operation if the cluster is already
> marked
> + * as zero, or if it's unallocated and we don't have a backing file.
> + *
> + * TODO We might want to use bdrv_get_block_status(bs) here, but
> we're
> + * holding s->lock, so that doesn't work today.
> + */
> + if (!!(old_offset & QCOW_OFLAG_ZERO)) {
> + continue;
> + }
> +
> + if ((old_offset & L2E_OFFSET_MASK) == 0 && !bs->backing_hd) {
> continue;
> }
>
> /* First remove L2 entries */
> qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
> - l2_table[l2_index + i] = cpu_to_be64(0);
> + if (s->qcow_version >= 3) {
> + l2_table[l2_index + i] = cpu_to_be64(QCOW_OFLAG_ZERO);
> + } else {
> + l2_table[l2_index + i] = cpu_to_be64(0);
> + }
>
> /* Then decrease the refcount */
> qcow2_free_any_clusters(bs, old_offset, 1, type);
Oops, this breaks qemu-iotests 046. I have dropped it from the pull request.
046 1s ... - output mismatch (see 046.out.bad)
--- 046.out 2014-02-03 16:06:23.917196159 +0100
+++ 046.out.bad 2014-02-14 18:02:01.577193856 +0100
@@ -187,10 +187,12 @@
24 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 8192/8192 bytes at offset 516096
8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Pattern verification failed at offset 524288, 24576 bytes
read 24576/24576 bytes at offset 524288
24 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 8192/8192 bytes at offset 548864
8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Pattern verification failed at offset 557056, 8192 bytes
read 8192/8192 bytes at offset 557056
8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 57344/57344 bytes at offset 565248
@@ -199,10 +201,12 @@
24 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 8192/8192 bytes at offset 647168
8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Pattern verification failed at offset 655360, 24576 bytes
read 24576/24576 bytes at offset 655360
24 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 8192/8192 bytes at offset 679936
8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Pattern verification failed at offset 688128, 8192 bytes
read 8192/8192 bytes at offset 688128
8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 57344/57344 bytes at offset 696320
Failures: 046
Failed 1 of 1 tests