[dm-devel] Re: A BUG in snapshot merging

[Mike Snitzer]

On Tue, Sep 22 2009 at  1:00pm -0400,
Mike Snitzer <snitzer at redhat.com> wrote:

> On Tue, Sep 22 2009 at 12:37pm -0400,
> Mikulas Patocka <mpatocka at redhat.com> wrote:
> 
> > I got this BUG when attempting to use merged patchset of Mike's and Jon's 
> > patches (from 
> > http://people.redhat.com/msnitzer/patches/snapshot-merge/kernel_unified/2.6.31/)
> > 
> > I think we shouldn't join these two patchsets together. I mean, before 
> > clustered patches, merging was stable (I reviewed and tested it and except 
> > for one userspace bug (already fixed) there were no flaws) ... now it 
> > doesn't work.
> > 
> > I would recommend to leave merging as it was (i.e. stable, apply only 
> > little patches on it) and develop Jon's clustering on the top of merging 
> > and not interleave it with merging, so that the clustering patches could 
> > be rolled back if problems were found. When clustering will be stable and 
> > reviewed, it could be added to the kernel --- but it may happen later than 
> > merging, so don't mix it.
> 
> 
> When did you pull in the patches on my people page?
> 
> As of yesterday evening (Boston) I uploaded patches that are broken
> (based on Jon's reworked handover).  I'm now combining your handover
> with Jon's handover (in hopes of avoiding refactoring associations).
> 
> The patches are in flux... I'm working to resolve the issues that are
> rooted at the handover mechanism.
> 
> As for patch ordering.  I'm not opposed to what you suggested (merge
> first then clusterized).  But that is a secondary concern right now.  We
> have enough time between now and the next merge window to get them both
> working.

Mikulas,

I've fixed the handover mechanism.  It now reflects the combination of
both your handover and Jon's (whereby avoiding refactoring
associations in dm_exception_store and dm_snapshot).

I've uploaded the updated quilt series to the usual place:
http://people.redhat.com/msnitzer/patches/snapshot-merge/kernel_unified/2.6.31/

I not sure which BUG() you hit in dm-snap-persistent.c (because my line
numbers have changed).. but given that it was in merge_callback() I'd
imagine it is the BUG_ON() that Jon added to clear_exception().

That BUG_ON() is actually useful.  If you can reproduce it with these
updated patches it bears further investigation.

In general, I think snapshot-merge is stronger for having combined with
Jon's clusterized patches.  I actually prefer the final result more so
than if the merge patches were to try to stand on their own (Jon's
refactoring of the exception-store et al has had a positive side-effect
on snapshot-merge).

Mike