Re: [Qemu-devel] [RFC V6 27/33] qcow2: Adapt checking of QCOW_OFLAG_COPIED for dedup.

[Kevin Wolf]

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.

Kevin