[dm-devel] Unnecessary snapshot validation

[Mikulas Patocka]

On Fri, 30 Jan 2015, Alasdair G Kergon wrote:

> I've had a complaint tonight at FOSDEM that we invalidate old-style
> snapshots in a particular case where there is no need to!
> 
> If you take a snapshot of an origin, then keep writing to the *snapshot*
> and fill it up we should just return errors on any further such writes
> *without* invalidating the snapshot - so reads can still be performed
> successfully.
> 
> (check for -ENOSPC from prepare_exception in __find_pending_exception?)
> 
> Alasdair

Hi

Here I'm sending a patch that keeps data when the snapshot fills up.

The user can extend the overflowed snapshot, deactivate and activate it 
again, run fsck (if journaling filesystem is not used) mount it and 
recover the data.

Mikulas



From: Mikulas Patocka <mpatocka at redhat.com>

dm snapshot: don't invalidate on-disk image on snapshot write overflow

When the snapshot overflows because of a write to the origin, the on-disk
image has to be invalidated. However, when the snapshot overflows because
of a write to the snapshot, the on-disk image doesn't have to be
invalidated. This patch changes the behavior, so that the on-disk image is
not invalidated in this case.

When the snapshot overflows, the variable snapshot_overflowed is set. All
writes to the snapshot are disallowed to minimize filesystem corruption -
this condition is cleared when the snapshot is deactivated and activated.

Signed-off-by: Mikulas Patocka <mpatocka at redhat.com>

---
 drivers/md/dm-snap.c |   18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

Index: linux-4.1-rc8/drivers/md/dm-snap.c
===================================================================
--- linux-4.1-rc8.orig/drivers/md/dm-snap.c	2015-06-19 20:57:04.000000000 +0200
+++ linux-4.1-rc8/drivers/md/dm-snap.c	2015-06-21 17:11:10.000000000 +0200
@@ -63,6 +63,13 @@ struct dm_snapshot {
 	 */
 	int valid;
 
+	/*
+	 * The snasphot overflowed because of write to the snapshot device.
+	 * We don't have to invalidate the snapshot in this case, but we need
+	 * to prevent further writes.
+	 */
+	int snapshot_overflowed;
+
 	/* Origin writes don't trigger exceptions until this is set */
 	int active;
 
@@ -1152,6 +1159,7 @@ static int snapshot_ctr(struct dm_target
 
 	s->ti = ti;
 	s->valid = 1;
+	s->snapshot_overflowed = 0;
 	s->active = 0;
 	atomic_set(&s->pending_exceptions_count, 0);
 	s->exception_start_sequence = 0;
@@ -1301,6 +1309,7 @@ static void __handover_exceptions(struct
 
 	snap_dest->ti->max_io_len = snap_dest->store->chunk_size;
 	snap_dest->valid = snap_src->valid;
+	snap_dest->snapshot_overflowed = snap_src->snapshot_overflowed;
 
 	/*
 	 * Set source invalid to ensure it receives no further I/O.
@@ -1692,7 +1701,7 @@ static int snapshot_map(struct dm_target
 	 * to copy an exception */
 	down_write(&s->lock);
 
-	if (!s->valid) {
+	if (!s->valid || (unlikely(s->snapshot_overflowed) && bio_rw(bio) == WRITE)) {
 		r = -EIO;
 		goto out_unlock;
 	}
@@ -1716,7 +1725,7 @@ static int snapshot_map(struct dm_target
 			pe = alloc_pending_exception(s);
 			down_write(&s->lock);
 
-			if (!s->valid) {
+			if (!s->valid || s->snapshot_overflowed) {
 				free_pending_exception(pe);
 				r = -EIO;
 				goto out_unlock;
@@ -1731,7 +1740,8 @@ static int snapshot_map(struct dm_target
 
 			pe = __find_pending_exception(s, pe, chunk);
 			if (!pe) {
-				__invalidate_snapshot(s, -ENOMEM);
+				s->snapshot_overflowed = 1;
+				DMERR("Invalidating snapshot: Unable to allocate exception.");
 				r = -EIO;
 				goto out_unlock;
 			}
@@ -1987,7 +1997,7 @@ static void snapshot_status(struct dm_ta
 
 		down_write(&snap->lock);
 
-		if (!snap->valid)
+		if (!snap->valid || snap->snapshot_overflowed)
 			DMEMIT("Invalid");
 		else if (snap->merge_failed)
 			DMEMIT("Merge failed");