On Fri, Mar 01, 2013 at 09:59:33AM +0100, Kevin Wolf wrote:
> Am 28.02.2013 um 17:14 hat Benoît Canet geschrieben:
> > Le Thursday 28 Feb 2013 à 11:14:34 (+0100), Kevin Wolf a écrit :
> > > Am 28.02.2013 um 10:41 hat Stefan Hajnoczi geschrieben:
> > > > On Wed, Feb 27, 2013 at 04:00:28PM +0100, Benoît Canet wrote:
> > > > > > > - if ((refcount == 1) != ((l2_entry &
> > > > > > > QCOW_OFLAG_COPIED) != 0)) {
> > > > > > > + if (!s->has_dedup &&
> > > > > > > + (refcount == 1) != ((l2_entry &
> > > > > > > QCOW_OFLAG_COPIED) != 0)) {
> > > > > > > + fprintf(stderr, "ERROR OFLAG_COPIED:
> > > > > > > offset=%"
> > > > > > > + PRIx64 " refcount=%d\n", l2_entry,
> > > > > > > refcount);
> > > > > > > + res->corruptions++;
> > > > > > > + }
> > > > > >
> > > > > > Why is this warning suppressed when dedup is enabled? The meaning
> > > > > > of
> > > > > > QCOW_OFLAG_COPIED is that refcount == 1. If this invariant is
> > > > > > violated
> > > > > > then something is wrong.
> > > > >
> > > > > When deduplication is done refcount will be bigger than one and
> > > > > QCOW_OFLAG_COPIED will be cleared.
> > > > >
> > > > > Then if enough logical clustere pointing to the same physical cluster
> > > > > are
> > > > > rewritten with something else the refcount will goes down back to one.
> > > > >
> > > > > But this time QCOW_OFLAG_COPIED can be set back so this equality
> > > > > won't be true.
> > > >
> > > > When the refcount decreases to 1 again we need to set QCOW_OFLAG_COPIED
> > > > again. qcow2-snapshot.c:qcow2_snapshot_delete() does this with:
> > > >
> > > > /* must update the copied flag on the current cluster offsets */
> > > > ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset,
> > > > s->l1_size, 0);
> > > >
> > > > Is dedup not restoring QCOW_OFLAG_COPIED?
> > >
> > > 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 with
> > > 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.
> >
> > Also when two logical clusters point to the same physical cluster and one
> > of the
> > logical cluster get overwritten the deduplication code has no way to know
> > 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.
Okay, I agree that qcow2_update_snapshot_refcount() is too expensive.
Please add a comment explaining that QCOW_OFLAG_COPIED is not guaranteed
when dedup is enabled since it would be too expensive to do this
everything sharing breaks (refcount is decremented to 1).
Stefan
