From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43245) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UBLod-0002lX-0d for qemu-devel@nongnu.org; Fri, 01 Mar 2013 03:59:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UBLob-0002I7-Qw for qemu-devel@nongnu.org; Fri, 01 Mar 2013 03:59:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50622) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UBLob-0002I3-Iu for qemu-devel@nongnu.org; Fri, 01 Mar 2013 03:59:41 -0500 Date: Fri, 1 Mar 2013 09:59:33 +0100 From: Kevin Wolf Message-ID: <20130301085933.GA2461@dhcp-200-207.str.redhat.com> References: <1360153926-9492-1-git-send-email-benoit@irqsave.net> <1360153926-9492-28-git-send-email-benoit@irqsave.net> <20130208105746.GD7835@stefanha-thinkpad.redhat.com> <20130227150028.GC4010@irqsave.net> <20130228094139.GB12748@stefanha-thinkpad.redhat.com> <20130228101434.GD2429@dhcp-200-207.str.redhat.com> <20130228161418.GA4214@irqsave.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20130228161418.GA4214@irqsave.net> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC V6 27/33] qcow2: Adapt checking of QCOW_OFLAG_COPIED for dedup. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?iso-8859-1?Q?Beno=EEt?= Canet Cc: qemu-devel@nongnu.org, Stefan Hajnoczi Am 28.02.2013 um 17:14 hat Beno=EEt Canet geschrieben: > Le Thursday 28 Feb 2013 =E0 11:14:34 (+0100), Kevin Wolf a =E9crit : > > Am 28.02.2013 um 10:41 hat Stefan Hajnoczi geschrieben: > > > On Wed, Feb 27, 2013 at 04:00:28PM +0100, Beno=EEt Canet wrote: > > > > > > - if ((refcount =3D=3D 1) !=3D ((l2_entry & QC= OW_OFLAG_COPIED) !=3D 0)) { > > > > > > + if (!s->has_dedup && > > > > > > + (refcount =3D=3D 1) !=3D ((l2_entry & QC= OW_OFLAG_COPIED) !=3D 0)) { > > > > > > + fprintf(stderr, "ERROR OFLAG_COPIED: off= set=3D%" > > > > > > + PRIx64 " refcount=3D%d\n", l2_entry,= refcount); > > > > > > + res->corruptions++; > > > > > > + } > > > > >=20 > > > > > Why is this warning suppressed when dedup is enabled? The mean= ing of > > > > > QCOW_OFLAG_COPIED is that refcount =3D=3D 1. If this invariant= is violated > > > > > then something is wrong. > > > >=20 > > > > When deduplication is done refcount will be bigger than one and > > > > QCOW_OFLAG_COPIED will be cleared. > > > >=20 > > > > Then if enough logical clustere pointing to the same physical clu= ster are > > > > rewritten with something else the refcount will goes down back to= one. > > > >=20 > > > > But this time QCOW_OFLAG_COPIED can be set back so this equality = won't be true. > > >=20 > > > When the refcount decreases to 1 again we need to set QCOW_OFLAG_CO= PIED > > > again. qcow2-snapshot.c:qcow2_snapshot_delete() does this with: > > >=20 > > > /* must update the copied flag on the current cluster offsets *= / > > > ret =3D qcow2_update_snapshot_refcount(bs, s->l1_table_offset, = s->l1_size, 0); > > >=20 > > > Is dedup not restoring QCOW_OFLAG_COPIED? > >=20 > > This is a very expensive operation. I don't think that you can do it = for > > each deduplicated cluster that is overwritten. Not doing it comes wit= h > > the cost of doing more COW than is actually needed. And we need to > > mention in the spec that QCOW_OFLAG_COPIED can be missing on clusters > > with deduplication enabled. >=20 > Also when two logical clusters point to the same physical cluster and o= ne of the > logical cluster get overwritten the deduplication code has no way to kn= ow the > index of the last logical cluster entry. Well, strictly speaking you can. The qcow2_update_snapshot_refcount() call that Stefan mention does exactly that. It's just insanely expensive because it has to look at the refcounts for all clusters. Kevin