On Fri, Nov 06 2009 at 1:46pm -0500, Mike Snitzer <snitzer@xxxxxxxxxx> wrote: > Alasdair, > > Here is the incremental patch that illustrates the relevant changes that > I layered ontop of the current patch you have in your tree. > > . changed snapshot_ctr() to avoid read_metadata entirely for > snapshot-merge (unnecessary if handover will occur) > - adjusted a few other things in snapshot_ctr to accomodate early > return in the case of handover > > . new snapshot's ti->split_io is now reset in handover_exceptions() to > match the chunk_size of the store that was just handed over > > . register_snapshot() for snapshot-merge snapshot is now done after > handover_exceptions rather than in snapshot_ctr() > - not done in handover_exceptions() to avoid lock order issues between > s->lock and _origins_lock > > . remove support for old->resume handover > - now that snapshot_ctr no longer does read_metadata() this is no > longer needed to support 'lvchange --refresh' OK, actually in practice 'lvchange --refresh' is: old->suspend new->ctr old->resume - device-mapper: snapshots: Unable to handover exceptions to another snapshot on resume. new->resume - snapshot_resume: handing over exceptions So it looks like we really do need to allow snapshot_resume to trigger handover if the old is resumed before the new. (snapshot-merge is always activated last by lvm because it acts as the origin). I added this before, and mistakenly thought it wasn't now: http://patchwork.kernel.org/patch/57787/ Without it the old snapshot would be active with the exceptions that get handed over to the snapshot-merge. Quite bad. Here is the incremental patch to add it back: diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c index e5acff4..1ba1861 100644 --- a/drivers/md/dm-snap.c +++ b/drivers/md/dm-snap.c @@ -82,9 +82,10 @@ struct dm_snapshot { * * 'is_handover_destination' is set in snapshot_ctr if an existing * snapshot has the same cow device. The handover is performed, - * and 'is_handover_destination' is cleared, when either of the + * and 'is_handover_destination' is cleared, when one of the * following occurs: * - src (old) snapshot, that is handing over, is destructed + * - src (old) snapshot, that is handing over, is resumed * - dest (new) snapshot, that is accepting the handover, is resumed */ int is_handover_destination; @@ -1379,23 +1380,28 @@ static void snapshot_presuspend(struct dm_target *ti) static void snapshot_resume(struct dm_target *ti) { struct dm_snapshot *s = ti->private; - struct dm_snapshot *old_snap, *new_snap = NULL; + struct dm_snapshot *lock_snap, *old_snap, *new_snap = NULL; down_write(&s->lock); if (s->handover_snap) { - if (!s->is_handover_destination) { - DMERR("Unable to handover exceptions to " - "another snapshot on resume."); - goto out; - } - /* Get exception store from another snapshot */ + /* + * Initially assumes this snapshot will get + * exception store from another snapshot + */ old_snap = s->handover_snap; new_snap = s; + lock_snap = old_snap; - down_write_nested(&old_snap->lock, + if (!s->is_handover_destination) { + /* Handover exceptions to another snapshot */ + old_snap = s; + new_snap = s->handover_snap; + lock_snap = new_snap; + } + down_write_nested(&lock_snap->lock, SINGLE_DEPTH_NESTING); handover_exceptions(old_snap, new_snap); - up_write(&old_snap->lock); + up_write(&lock_snap->lock); } /* An incomplete exception handover is not allowed */ BUG_ON(s->handover_snap); I'll post v2 of the overall handover patch; it will include this patch. Mike -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
