Skip to contentSkip to conversation

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

PATCH New Jonathan Brassow 3 participants 3 messages Aug 25, 2010 – Aug 26, 2010

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Do not scan snapshot origin for labels while 'ignore_suspended_devices()' is set

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.

lib/activate/dev_manager.c +15 -0
Index: 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? */
#1 lore raw

PATCH 1/1 Do not scan snapshot origin for labels while 'ignore_suspended_devices()' is set

lib/activate/dev_manager.c int device_is_usable(struct device *dev) · line 191

Quoted 12 lines · Jonathan Brassow
lib/activate/dev_manager.c +12 -0
+		 * 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

#2 lore raw

PATCH 1/1 Do not scan snapshot origin for labels while 'ignore_suspended_devices()' is set

Quoted 18 lines · Milan Broz

lib/activate/dev_manager.c int device_is_usable(struct device *dev) · line 191

Quoted 12 lines · Jonathan Brassow
lib/activate/dev_manager.c +12 -0
+		 * 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.

Quoted 1 line

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).

Yours,
Petr.