[PATCH] Do not scan snapshot origin for labels while 'ignore_suspended_devices()' is set

[PATCH] Do not scan snapshot origin for labels while 'ignore_suspended_devices()' is set

[Jonathan Brassow]

This patch fixes a potential for I/O to hang and LVM commands
to block when a mirror under a snapshot suffers a failure.

The problem has to do with label scanning.  When a mirror suffers
a failure, the kernel blocks I/O to prevent corruption.  When
LVM attempts to repair the mirror, it scans the devices on the
system for LVM labels.  While mirrors are skipped during this
scanning process, snapshot-origins are not.  When the origin is
scanned, it kicks up I/O to the mirror (which is blocked)
underneath - causing the label scan (an thus the repair operation)
to hang.

This patch simply bypasses snapshot-origin devices when doing
labels scans (while ignore_suspended_devices() is set).  This
fixes the issue.

Signed-off-by: Jonathan Brassow <jbrassow@redhat.com>

Index: LVM2/lib/activate/dev_manager.c
===================================================================
--- LVM2.orig/lib/activate/dev_manager.c
+++ LVM2/lib/activate/dev_manager.c
@@ -175,6 +175,21 @@ int device_is_usable(struct device *dev)
 			log_debug("%s: Mirror device %s not usable.", dev_name(dev), name);
 			goto out;
 		}
+
+		/*
+		 * Snapshot origin could be sitting on top of a mirror which
+		 * could be blocking I/O.  Skip snapshot origins entirely for
+		 * now.
+		 *
+		 * FIXME: rather than skipping origin, check if mirror is
+		 * underneath and if the mirror is blocking I/O.
+		 */
+		if (target_type && !strcmp(target_type, "snapshot-origin") &&
+		    ignore_suspended_devices()) {
+			log_debug("%s: Snapshot-origin device %s not usable.",
+				  dev_name(dev), name);
+			goto out;
+		}
 	} while (next);
 
 	/* FIXME Also check dependencies? */

[PATCH] Do not scan snapshot origin for labels while 'ignore_suspended_devices()' is set

[Milan Broz]

On 08/25/2010 04:38 PM, Jonathan Brassow wrote:

> +		 * Snapshot origin could be sitting on top of a mirror which
> +		 * could be blocking I/O.  Skip snapshot origins entirely for
> +		 * now.
> +		 *
> +		 * FIXME: rather than skipping origin, check if mirror is
> +		 * underneath and if the mirror is blocking I/O.
> +		 */
> +		if (target_type && !strcmp(target_type, "snapshot-origin") &&
> +		    ignore_suspended_devices()) {
> +			log_debug("%s: Snapshot-origin device %s not usable.",
> +				  dev_name(dev), name);
> +			goto out;

Well, if no one can find situation where this breaks some existing config,
let's try that - we need something working in this release.

The code need rewrite here anyway... ack from me for now.

Milan

[PATCH] Do not scan snapshot origin for labels while 'ignore_suspended_devices()' is set

[Petr Rockai]

Milan Broz <mbroz@redhat.com> writes:

> On 08/25/2010 04:38 PM, Jonathan Brassow wrote:
>
>> +		 * Snapshot origin could be sitting on top of a mirror which
>> +		 * could be blocking I/O.  Skip snapshot origins entirely for
>> +		 * now.
>> +		 *
>> +		 * FIXME: rather than skipping origin, check if mirror is
>> +		 * underneath and if the mirror is blocking I/O.
>> +		 */
>> +		if (target_type && !strcmp(target_type, "snapshot-origin") &&
>> +		    ignore_suspended_devices()) {
>> +			log_debug("%s: Snapshot-origin device %s not usable.",
>> +				  dev_name(dev), name);
>> +			goto out;
>
> Well, if no one can find situation where this breaks some existing config,
> let's try that - we need something working in this release.
>
> The code need rewrite here anyway... ack from me for now.

Well, stacked PVs will break in some (rare) corner cases with the above
change. Nevertheless, we can probably get away with this for now (since
stacked PVs don't work very well anyway).

Reviewed-By: Petr Rockai <prockai@redhat.com>

Yours,
   Petr.