Better zfs_get_enclosure_sysfs_path() enclosure support

A multpathed disk will have several 'underlying' paths to the disk.  For
example, multipath disk 'dm-0' may be made up of paths:
/dev/{sda,sdb,sdc,sdd}.  On many enclosures those underlying sysfs
paths will have a symlink back to their enclosure device entry
(like 'enclosure_device0/slot1').  This is used by the
statechange-led.sh script to set/clear the fault LED for a disk, and
by 'zpool status -c'.

However, on some enclosures, those underlying paths may not all have
symlinks back to the enclosure device.  Maybe only two out of four
of them might.

This patch updates zfs_get_enclosure_sysfs_path() to favor returning
paths that have symlinks back to their enclosure devices, rather
than just returning the first path.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tony Hutter <[email protected]>
Closes #11617 

1 files changed, 142 insertions, 110 deletions

diff --git a/lib/libzutil/os/linux/zutil_device_path_os.c b/lib/libzutil/os/linux/zutil_device_path_os.c
index 36331fd72..1f767bb7a 100644
--- a/lib/libzutil/os/linux/zutil_device_path_os.c
+++ b/lib/libzutil/os/linux/zutil_device_path_os.c
@@ -155,14 +155,123 @@ zfs_strip_path(char *path)
 }
 
 /*
+ * Given a dev name like "sda", return the full enclosure sysfs path to
+ * the disk.  You can also pass in the name with "/dev" prepended
+ * to it (like /dev/sda).
+ *
+ * For example, disk "sda" in enclosure slot 1:
+ *     dev:            "sda"
+ *     returns:        "/sys/class/enclosure/1:0:3:0/Slot 1"
+ *
+ * 'dev' must be a non-devicemapper device.
+ *
+ * Returned string must be freed.
+ */
+char *
+zfs_get_enclosure_sysfs_path(const char *dev_name)
+{
+	DIR *dp = NULL;
+	struct dirent *ep;
+	char buf[MAXPATHLEN];
+	char *tmp1 = NULL;
+	char *tmp2 = NULL;
+	char *tmp3 = NULL;
+	char *path = NULL;
+	size_t size;
+	int tmpsize;
+
+	if (dev_name == NULL)
+		return (NULL);
+
+	/* If they preface 'dev' with a path (like "/dev") then strip it off */
+	tmp1 = strrchr(dev_name, '/');
+	if (tmp1 != NULL)
+		dev_name = tmp1 + 1;    /* +1 since we want the chr after '/' */
+
+	tmpsize = asprintf(&tmp1, "/sys/block/%s/device", dev_name);
+	if (tmpsize == -1 || tmp1 == NULL) {
+		tmp1 = NULL;
+		goto end;
+	}
+
+	dp = opendir(tmp1);
+	if (dp == NULL) {
+		tmp1 = NULL;	/* To make free() at the end a NOP */
+		goto end;
+	}
+
+	/*
+	 * Look though all sysfs entries in /sys/block/<dev>/device for
+	 * the enclosure symlink.
+	 */
+	while ((ep = readdir(dp))) {
+		/* Ignore everything that's not our enclosure_device link */
+		if (strstr(ep->d_name, "enclosure_device") == NULL)
+			continue;
+
+		if (asprintf(&tmp2, "%s/%s", tmp1, ep->d_name) == -1 ||
+		    tmp2 == NULL)
+			break;
+
+		size = readlink(tmp2, buf, sizeof (buf));
+
+		/* Did readlink fail or crop the link name? */
+		if (size == -1 || size >= sizeof (buf)) {
+			free(tmp2);
+			tmp2 = NULL;	/* To make free() at the end a NOP */
+			break;
+		}
+
+		/*
+		 * We got a valid link.  readlink() doesn't terminate strings
+		 * so we have to do it.
+		 */
+		buf[size] = '\0';
+
+		/*
+		 * Our link will look like:
+		 *
+		 * "../../../../port-11:1:2/..STUFF../enclosure/1:0:3:0/SLOT 1"
+		 *
+		 * We want to grab the "enclosure/1:0:3:0/SLOT 1" part
+		 */
+		tmp3 = strstr(buf, "enclosure");
+		if (tmp3 == NULL)
+			break;
+
+		if (asprintf(&path, "/sys/class/%s", tmp3) == -1) {
+			/* If asprintf() fails, 'path' is undefined */
+			path = NULL;
+			break;
+		}
+
+		if (path == NULL)
+			break;
+	}
+
+end:
+	free(tmp2);
+	free(tmp1);
+
+	if (dp != NULL)
+		closedir(dp);
+
+	return (path);
+}
+
+/*
  * Allocate and return the underlying device name for a device mapper device.
- * If a device mapper device maps to multiple devices, return the first device.
  *
  * For example, dm_name = "/dev/dm-0" could return "/dev/sda". Symlinks to a
  * DM device (like /dev/disk/by-vdev/A0) are also allowed.
  *
- * Returns device name, or NULL on error or no match.  If dm_name is not a DM
- * device then return NULL.
+ * If the DM device has multiple underlying devices (like with multipath
+ * DM devices), then favor underlying devices that have a symlink back to their
+ * back to their enclosure device in sysfs.  This will be useful for the
+ * zedlet scripts that toggle the fault LED.
+ *
+ * Returns an underlying device name, or NULL on error or no match.  If dm_name
+ * is not a DM device then return NULL.
  *
  * NOTE: The returned name string must be *freed*.
  */
@@ -176,6 +285,8 @@ dm_get_underlying_path(const char *dm_name)
 	char *path = NULL;
 	char *dev_str;
 	int size;
+	char *first_path = NULL;
+	char *enclosure_path;
 
 	if (dm_name == NULL)
 		return (NULL);
@@ -204,13 +315,27 @@ dm_get_underlying_path(const char *dm_name)
 		goto end;
 
 	/*
-	 * Return first entry (that isn't itself a directory) in the
-	 * directory containing device-mapper dependent (underlying)
-	 * devices.
+	 * A device-mapper device can have multiple paths to it (multipath).
+	 * Favor paths that have a symlink back to their enclosure device.
+	 * We have to do this since some enclosures may only provide a symlink
+	 * back for one underlying path to a disk and not the other.
+	 *
+	 * If no paths have links back to their enclosure, then just return the
+	 * first path.
 	 */
 	while ((ep = readdir(dp))) {
 		if (ep->d_type != DT_DIR) {	/* skip "." and ".." dirs */
+			if (!first_path)
+				first_path = strdup(ep->d_name);
+
+			enclosure_path =
+			    zfs_get_enclosure_sysfs_path(ep->d_name);
+
+			if (!enclosure_path)
+				continue;
+
 			size = asprintf(&path, "/dev/%s", ep->d_name);
+			free(enclosure_path);
 			break;
 		}
 	}
@@ -220,6 +345,17 @@ end:
 		closedir(dp);
 	free(tmp);
 	free(realp);
+
+	if (!path) {
+		/*
+		 * None of the underlying paths had a link back to their
+		 * enclosure devices.  Throw up out hands and return the first
+		 * underlying path.
+		 */
+		size = asprintf(&path, "/dev/%s", first_path);
+	}
+
+	free(first_path);
 	return (path);
 }
 
@@ -331,110 +467,6 @@ zfs_get_underlying_path(const char *dev_name)
 	return (name);
 }
 
-/*
- * Given a dev name like "sda", return the full enclosure sysfs path to
- * the disk.  You can also pass in the name with "/dev" prepended
- * to it (like /dev/sda).
- *
- * For example, disk "sda" in enclosure slot 1:
- *     dev:            "sda"
- *     returns:        "/sys/class/enclosure/1:0:3:0/Slot 1"
- *
- * 'dev' must be a non-devicemapper device.
- *
- * Returned string must be freed.
- */
-char *
-zfs_get_enclosure_sysfs_path(const char *dev_name)
-{
-	DIR *dp = NULL;
-	struct dirent *ep;
-	char buf[MAXPATHLEN];
-	char *tmp1 = NULL;
-	char *tmp2 = NULL;
-	char *tmp3 = NULL;
-	char *path = NULL;
-	size_t size;
-	int tmpsize;
-
-	if (dev_name == NULL)
-		return (NULL);
-
-	/* If they preface 'dev' with a path (like "/dev") then strip it off */
-	tmp1 = strrchr(dev_name, '/');
-	if (tmp1 != NULL)
-		dev_name = tmp1 + 1;    /* +1 since we want the chr after '/' */
-
-	tmpsize = asprintf(&tmp1, "/sys/block/%s/device", dev_name);
-	if (tmpsize == -1 || tmp1 == NULL) {
-		tmp1 = NULL;
-		goto end;
-	}
-
-	dp = opendir(tmp1);
-	if (dp == NULL) {
-		tmp1 = NULL;	/* To make free() at the end a NOP */
-		goto end;
-	}
-
-	/*
-	 * Look though all sysfs entries in /sys/block/<dev>/device for
-	 * the enclosure symlink.
-	 */
-	while ((ep = readdir(dp))) {
-		/* Ignore everything that's not our enclosure_device link */
-		if (strstr(ep->d_name, "enclosure_device") == NULL)
-			continue;
-
-		if (asprintf(&tmp2, "%s/%s", tmp1, ep->d_name) == -1 ||
-		    tmp2 == NULL)
-			break;
-
-		size = readlink(tmp2, buf, sizeof (buf));
-
-		/* Did readlink fail or crop the link name? */
-		if (size == -1 || size >= sizeof (buf)) {
-			free(tmp2);
-			tmp2 = NULL;	/* To make free() at the end a NOP */
-			break;
-		}
-
-		/*
-		 * We got a valid link.  readlink() doesn't terminate strings
-		 * so we have to do it.
-		 */
-		buf[size] = '\0';
-
-		/*
-		 * Our link will look like:
-		 *
-		 * "../../../../port-11:1:2/..STUFF../enclosure/1:0:3:0/SLOT 1"
-		 *
-		 * We want to grab the "enclosure/1:0:3:0/SLOT 1" part
-		 */
-		tmp3 = strstr(buf, "enclosure");
-		if (tmp3 == NULL)
-			break;
-
-		if (asprintf(&path, "/sys/class/%s", tmp3) == -1) {
-			/* If asprintf() fails, 'path' is undefined */
-			path = NULL;
-			break;
-		}
-
-		if (path == NULL)
-			break;
-	}
-
-end:
-	free(tmp2);
-	free(tmp1);
-
-	if (dp != NULL)
-		closedir(dp);
-
-	return (path);
-}
 
 #ifdef HAVE_LIBUDEV