All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2] qcow2: Set zero flag for discarded clusters
@ 2014-02-17 14:45 Kevin Wolf
  2014-02-17 17:12 ` Eric Blake
  0 siblings, 1 reply; 7+ messages in thread
From: Kevin Wolf @ 2014-02-17 14:45 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, stefanha, mreitz

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 <kwolf@redhat.com>
---
 block/qcow2-cluster.c  | 22 ++++++++++++++++++++--
 tests/qemu-iotests/046 | 18 ++++++++++++++----
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index cd2dcfa..f404ef1 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1332,13 +1332,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);
diff --git a/tests/qemu-iotests/046 b/tests/qemu-iotests/046
index 2d44bbb..e0be46c 100755
--- a/tests/qemu-iotests/046
+++ b/tests/qemu-iotests/046
@@ -193,6 +193,16 @@ echo "== Verify image content =="
 
 function verify_io()
 {
+    if ($QEMU_IMG info -f "$IMGFMT" "$TEST_IMG" | grep "compat: 0.10" > /dev/null); then
+        # For v2 images, discarded clusters are read from the backing file
+        # Keep the variable empty so that the backing file value can be used as
+        # the default below
+        discarded=
+    else
+        # Discarded clusters are zeroed for v3 or later
+        discarded=0
+    fi
+
     echo read -P 0 0 0x10000
 
     echo read -P 1  0x10000 0x2000
@@ -221,16 +231,16 @@ function verify_io()
     echo read -P 70 0x78000 0x6000
     echo read -P 7  0x7e000 0x2000
 
-    echo read -P 8  0x80000 0x6000
+    echo read -P ${discarded:-8} 0x80000 0x6000
     echo read -P 80 0x86000 0x2000
-    echo read -P 8  0x88000 0x2000
+    echo read -P ${discarded:-8} 0x88000 0x2000
     echo read -P 81 0x8a000 0xe000
     echo read -P 90 0x98000 0x6000
     echo read -P 9  0x9e000 0x2000
 
-    echo read -P 10  0xa0000 0x6000
+    echo read -P ${discarded:-10} 0xa0000 0x6000
     echo read -P 100 0xa6000 0x2000
-    echo read -P 10  0xa8000 0x2000
+    echo read -P ${discarded:-10} 0xa8000 0x2000
     echo read -P 101 0xaa000 0xe000
     echo read -P 110 0xb8000 0x8000
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] qcow2: Set zero flag for discarded clusters
@ 2014-02-08 16:17 Max Reitz
  2014-02-08 16:50 ` [Qemu-devel] [PATCH v2] " Kevin Wolf
  0 siblings, 1 reply; 7+ messages in thread
From: Max Reitz @ 2014-02-08 16:17 UTC (permalink / raw)
  To: Kevin Wolf, qemu-devel; +Cc: pbonzini, stefanha

On 08.02.2014 16:28, 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 <kwolf@redhat.com>
> ---
>   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..008bc04 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->version >= 3) {

Oh, I revoke my reviewed-by from earlier. This should probably be 
"s->qcow_version"; otherwise, it won't compile.

Max

> +            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);

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-02-18 11:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-17 14:45 [Qemu-devel] [PATCH v2] qcow2: Set zero flag for discarded clusters Kevin Wolf
2014-02-17 17:12 ` Eric Blake
2014-02-18 11:29   ` Kevin Wolf
  -- strict thread matches above, loose matches on Subject: below --
2014-02-08 16:17 [Qemu-devel] [PATCH] " Max Reitz
2014-02-08 16:50 ` [Qemu-devel] [PATCH v2] " Kevin Wolf
2014-02-14 14:34   ` Stefan Hajnoczi
2014-02-14 17:05   ` Stefan Hajnoczi
2014-02-14 18:11     ` Kevin Wolf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.