[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qcow2: Set zero flag for discarded clusters
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH] qcow2: Set zero flag for discarded clusters |
Date: |
Sat, 8 Feb 2014 17:32:55 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 08.02.2014 um 17:17 hat Max Reitz geschrieben:
> 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 <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..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.
Indeed. Makes we wonder what source it was that I tested... :-/
Kevin