From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44602) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WENEm-00046Z-O4 for qemu-devel@nongnu.org; Fri, 14 Feb 2014 13:11:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WENEg-0000ui-P0 for qemu-devel@nongnu.org; Fri, 14 Feb 2014 13:11:44 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33274) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WENEg-0000uT-HQ for qemu-devel@nongnu.org; Fri, 14 Feb 2014 13:11:38 -0500 Date: Fri, 14 Feb 2014 19:11:30 +0100 From: Kevin Wolf Message-ID: <20140214181130.GL32343@dhcp-200-207.str.redhat.com> References: <52F65880.9000307@redhat.com> <1391878202-27440-1-git-send-email-kwolf@redhat.com> <20140214170512.GA7660@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140214170512.GA7660@stefanha-thinkpad.redhat.com> Subject: Re: [Qemu-devel] [PATCH v2] qcow2: Set zero flag for discarded clusters List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, mreitz@redhat.com Am 14.02.2014 um 18:05 hat Stefan Hajnoczi geschrieben: > 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 > > Reviewed-by: Max Reitz > > --- > > 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. Okay. This is kind of nasty, because we now have different expected results for v2 and v3 images. I'll have to see how the test case is fixed in the best way. Perhaps it's best not to check discarded areas at all, because strictly speaking they are undefined. Kevin