Rescan enclosure sysfs path on import

When you create a pool, zfs writes vd->vdev_enc_sysfs_path with the
enclosure sysfs path to the fault LEDs, like:

    vdev_enc_sysfs_path = /sys/class/enclosure/0:0:1:0/SLOT8

However, this enclosure path doesn't get updated on successive imports
even if enclosure path to the disk changes.  This patch fixes the issue.

Signed-off-by: Tony Hutter <hutter2@llnl.gov>

cmd/zpool/Makefile.am

@@ -26,7 +26,8 @@ zpool_LDADD = \
 	$(abs_top_builddir)/lib/libzfs/libzfs.la \
 	$(abs_top_builddir)/lib/libzfs_core/libzfs_core.la \
 	$(abs_top_builddir)/lib/libnvpair/libnvpair.la \
-	$(abs_top_builddir)/lib/libuutil/libuutil.la
+	$(abs_top_builddir)/lib/libuutil/libuutil.la \
+	$(abs_top_builddir)/lib/libzutil/libzutil.la
 
 zpool_LDADD += $(LTLIBINTL)
 

cmd/zpool/zpool_iter.c

@@ -264,51 +264,6 @@ for_each_pool(int argc, char **argv, boolean_t unavail,
 	return (ret);
 }
 
-static int
-for_each_vdev_cb(zpool_handle_t *zhp, nvlist_t *nv, pool_vdev_iter_f func,
-    void *data)
-{
-	nvlist_t **child;
-	uint_t c, children;
-	int ret = 0;
-	int i;
-	char *type;
-
-	const char *list[] = {
-	    ZPOOL_CONFIG_SPARES,
-	    ZPOOL_CONFIG_L2CACHE,
-	    ZPOOL_CONFIG_CHILDREN
-	};
-
-	for (i = 0; i < ARRAY_SIZE(list); i++) {
-		if (nvlist_lookup_nvlist_array(nv, list[i], &child,
-		    &children) == 0) {
-			for (c = 0; c < children; c++) {
-				uint64_t ishole = 0;
-
-				(void) nvlist_lookup_uint64(child[c],
-				    ZPOOL_CONFIG_IS_HOLE, &ishole);
-
-				if (ishole)
-					continue;
-
-				ret |= for_each_vdev_cb(zhp, child[c], func,
-				    data);
-			}
-		}
-	}
-
-	if (nvlist_lookup_string(nv, ZPOOL_CONFIG_TYPE, &type) != 0)
-		return (ret);
-
-	/* Don't run our function on root vdevs */
-	if (strcmp(type, VDEV_TYPE_ROOT) != 0) {
-		ret |= func(zhp, nv, data);
-	}
-
-	return (ret);
-}
-
 /*
  * This is the equivalent of for_each_pool() for vdevs.  It iterates thorough
  * all vdevs in the pool, ignoring root vdevs and holes, calling func() on
@@ -327,7 +282,7 @@ for_each_vdev(zpool_handle_t *zhp, pool_vdev_iter_f func, void *data)
 		verify(nvlist_lookup_nvlist(config, ZPOOL_CONFIG_VDEV_TREE,
 		    &nvroot) == 0);
 	}
-	return (for_each_vdev_cb(zhp, nvroot, func, data));
+	return (for_each_vdev_cb((void *) zhp, nvroot, func, data));
 }
 
 /*
@@ -603,14 +558,15 @@ vdev_run_cmd_thread(void *cb_cmd_data)
 
 /* For each vdev in the pool run a command */
 static int
-for_each_vdev_run_cb(zpool_handle_t *zhp, nvlist_t *nv, void *cb_vcdl)
+for_each_vdev_run_cb(void *zhp_data, nvlist_t *nv, void *cb_vcdl)
 {
 	vdev_cmd_data_list_t *vcdl = cb_vcdl;
 	vdev_cmd_data_t *data;
 	char *path = NULL;
 	char *vname = NULL;
 	char *vdev_enc_sysfs_path = NULL;
 	int i, match = 0;
+	zpool_handle_t *zhp = zhp_data;
 
 	if (nvlist_lookup_string(nv, ZPOOL_CONFIG_PATH, &path) != 0)
 		return (1);

cmd/zpool/zpool_main.c

@@ -5156,11 +5156,12 @@ get_stat_flags(zpool_list_t *list)
  * Return 1 if cb_data->cb_vdev_names[0] is this vdev's name, 0 otherwise.
  */
 static int
-is_vdev_cb(zpool_handle_t *zhp, nvlist_t *nv, void *cb_data)
+is_vdev_cb(void *zhp_data, nvlist_t *nv, void *cb_data)
 {
 	iostat_cbdata_t *cb = cb_data;
 	char *name = NULL;
 	int ret = 0;
+	zpool_handle_t *zhp = zhp_data;
 
 	name = zpool_vdev_name(g_zfs, zhp, nv, cb->cb_name_flags);
 

cmd/zpool/zpool_util.h

@@ -27,6 +27,7 @@
 
 #include <libnvpair.h>
 #include <libzfs.h>
+#include <libzutil.h>
 
 #ifdef	__cplusplus
 extern "C" {
@@ -68,7 +69,6 @@ int for_each_pool(int, char **, boolean_t unavail, zprop_list_t **,
     boolean_t, zpool_iter_f, void *);
 
 /* Vdev list functions */
-typedef int (*pool_vdev_iter_f)(zpool_handle_t *, nvlist_t *, void *);
 int for_each_vdev(zpool_handle_t *zhp, pool_vdev_iter_f func, void *data);
 
 typedef struct zpool_list zpool_list_t;

include/libzutil.h

@@ -163,6 +163,16 @@ _LIBZUTIL_H int printf_color(char *color, char *format, ...);
 _LIBZUTIL_H const char *zfs_basename(const char *path);
 _LIBZUTIL_H ssize_t zfs_dirnamelen(const char *path);
 
+/*
+ * These functions are used by the ZFS libraries and cmd/zpool code, but are
+ * not exported in the ABI.
+ */
+typedef int (*pool_vdev_iter_f)(void *, nvlist_t *, void *);
+int for_each_vdev_cb(void *zhp, nvlist_t *nv, pool_vdev_iter_f func,
+    void *data);
+int for_each_vdev_in_nvlist(nvlist_t *nvroot, pool_vdev_iter_f func,
+    void *data);
+void update_vdevs_config_dev_sysfs_path(nvlist_t *config);
 #ifdef	__cplusplus
 }
 #endif

lib/libzutil/os/freebsd/zutil_import_os.c

@@ -247,3 +247,8 @@ zfs_dev_flush(int fd __unused)
 {
 	return (0);
 }
+
+void
+update_vdevs_config_dev_sysfs_path(nvlist_t *config)
+{
+}

lib/libzutil/os/linux/zutil_import_os.c

@@ -65,6 +65,7 @@
 #include <thread_pool.h>
 #include <libzutil.h>
 #include <libnvpair.h>
+#include <libzfs.h>
 
 #include "zutil_import.h"
 
@@ -757,6 +758,58 @@ encode_device_strings(const char *path, vdev_dev_strs_t *ds,
 #endif
 }
 
+/*
+ * Rescan the enclosure sysfs path for turning on enclosure LEDs and store it
+ * in the nvlist * (if applicable).  Like:
+ *    vdev_enc_sysfs_path: '/sys/class/enclosure/11:0:1:0/SLOT 4'
+ */
+static void
+update_vdev_config_dev_sysfs_path(nvlist_t *nv, char *path)
+{
+	char *upath, *spath;
+
+	/* Add enclosure sysfs path (if disk is in an enclosure). */
+	upath = zfs_get_underlying_path(path);
+	spath = zfs_get_enclosure_sysfs_path(upath);
+
+	if (spath) {
+		nvlist_add_string(nv, ZPOOL_CONFIG_VDEV_ENC_SYSFS_PATH, spath);
+	} else {
+		nvlist_remove_all(nv, ZPOOL_CONFIG_VDEV_ENC_SYSFS_PATH);
+	}
+
+	free(upath);
+	free(spath);
+}
+
+/*
+ * This will get called for each leaf vdev.
+ */
+static int
+sysfs_path_pool_vdev_iter_f(void *hdl_data, nvlist_t *nv, void *data)
+{
+	char *path = NULL;
+	if (nvlist_lookup_string(nv, ZPOOL_CONFIG_PATH, &path) != 0)
+		return (1);
+
+	/* Rescan our enclosure sysfs path for this vdev */
+	update_vdev_config_dev_sysfs_path(nv, path);
+	return (0);
+}
+
+/*
+ * Given an nvlist for our pool (with vdev tree), iterate over all the
+ * leaf vdevs and update their ZPOOL_CONFIG_VDEV_ENC_SYSFS_PATH.
+ */
+void
+update_vdevs_config_dev_sysfs_path(nvlist_t *config)
+{
+	nvlist_t *nvroot = NULL;
+	verify(nvlist_lookup_nvlist(config, ZPOOL_CONFIG_VDEV_TREE,
+	    &nvroot) == 0);
+	for_each_vdev_in_nvlist(nvroot, sysfs_path_pool_vdev_iter_f, NULL);
+}
+
 /*
  * Update a leaf vdev's persistent device strings
  *
@@ -783,7 +836,6 @@ update_vdev_config_dev_strs(nvlist_t *nv)
 	vdev_dev_strs_t vds;
 	char *env, *type, *path;
 	uint64_t wholedisk = 0;
-	char *upath, *spath;
 
 	/*
 	 * For the benefit of legacy ZFS implementations, allow
@@ -830,18 +882,7 @@ update_vdev_config_dev_strs(nvlist_t *nv)
 			(void) nvlist_add_string(nv, ZPOOL_CONFIG_PHYS_PATH,
 			    vds.vds_devphys);
 		}
-
-		/* Add enclosure sysfs path (if disk is in an enclosure). */
-		upath = zfs_get_underlying_path(path);
-		spath = zfs_get_enclosure_sysfs_path(upath);
-		if (spath)
-			nvlist_add_string(nv, ZPOOL_CONFIG_VDEV_ENC_SYSFS_PATH,
-			    spath);
-		else
-			nvlist_remove_all(nv, ZPOOL_CONFIG_VDEV_ENC_SYSFS_PATH);
-
-		free(upath);
-		free(spath);
+		update_vdev_config_dev_sysfs_path(nv, path);
 	} else {
 		/* Clear out any stale entries. */
 		(void) nvlist_remove_all(nv, ZPOOL_CONFIG_DEVID);

lib/libzutil/zutil_import.c

@@ -1705,6 +1705,8 @@ zpool_find_import_cached(libpc_handle_t *hdl, importargs_t *iarg)
 			return (NULL);
 		}
 
+		update_vdevs_config_dev_sysfs_path(src);
+
 		if ((dst = zutil_refresh_config(hdl, src)) == NULL) {
 			nvlist_free(raw);
 			nvlist_free(pools);
@@ -1859,3 +1861,69 @@ zpool_find_config(void *hdl, const char *target, nvlist_t **configp,
 
 	return (0);
 }
+
+/*
+ * Internal function for iterating over the vdevs.
+ *
+ * For each vdev, func() will be called and will be passed 'zhp' (which is
+ * typically the zpool_handle_t cast as a void pointer), the vdev's nvlist, and
+ * a user-defined data pointer).
+ *
+ * The return values from all the func() calls will be OR'd together and
+ * returned.
+ */
+int
+for_each_vdev_cb(void *zhp, nvlist_t *nv, pool_vdev_iter_f func,
+    void *data)
+{
+	nvlist_t **child;
+	uint_t c, children;
+	int ret = 0;
+	int i;
+	char *type;
+
+	const char *list[] = {
+	    ZPOOL_CONFIG_SPARES,
+	    ZPOOL_CONFIG_L2CACHE,
+	    ZPOOL_CONFIG_CHILDREN
+	};
+
+	for (i = 0; i < ARRAY_SIZE(list); i++) {
+		if (nvlist_lookup_nvlist_array(nv, list[i], &child,
+		    &children) == 0) {
+			for (c = 0; c < children; c++) {
+				uint64_t ishole = 0;
+
+				(void) nvlist_lookup_uint64(child[c],
+				    ZPOOL_CONFIG_IS_HOLE, &ishole);
+
+				if (ishole)
+					continue;
+
+				ret |= for_each_vdev_cb(zhp, child[c],
+				    func, data);
+			}
+		}
+	}
+
+	if (nvlist_lookup_string(nv, ZPOOL_CONFIG_TYPE, &type) != 0)
+		return (ret);
+
+	/* Don't run our function on root vdevs */
+	if (strcmp(type, VDEV_TYPE_ROOT) != 0) {
+		ret |= func(zhp, nv, data);
+	}
+
+	return (ret);
+}
+
+/*
+ * Given an ZPOOL_CONFIG_VDEV_TREE nvpair, iterate over all the vdevs, calling
+ * func() for each one.  func() is passed the vdev's nvlist and an optional
+ * user-defined 'data' pointer.
+ */
+int
+for_each_vdev_in_nvlist(nvlist_t *nvroot, pool_vdev_iter_f func, void *data)
+{
+	return (for_each_vdev_cb(NULL, nvroot, func, data));
+}

module/zfs/vdev.c

@@ -2373,6 +2373,7 @@ vdev_validate(vdev_t *vd)
 static void
 vdev_copy_path_impl(vdev_t *svd, vdev_t *dvd)
 {
+	char *old, *new;
 	if (svd->vdev_path != NULL && dvd->vdev_path != NULL) {
 		if (strcmp(svd->vdev_path, dvd->vdev_path) != 0) {
 			zfs_dbgmsg("vdev_copy_path: vdev %llu: path changed "
@@ -2386,6 +2387,29 @@ vdev_copy_path_impl(vdev_t *svd, vdev_t *dvd)
 		zfs_dbgmsg("vdev_copy_path: vdev %llu: path set to '%s'",
 		    (u_longlong_t)dvd->vdev_guid, dvd->vdev_path);
 	}
+
+	/*
+	 * Our enclosure sysfs path may have changed between imports
+	 */
+	old = dvd->vdev_enc_sysfs_path;
+	new = svd->vdev_enc_sysfs_path;
+	if ((old != NULL && new == NULL) ||
+	    (old == NULL && new != NULL) ||
+	    ((old != NULL && new != NULL) && strcmp(new, old) != 0)) {
+		zfs_dbgmsg("vdev_copy_path: vdev %llu: vdev_enc_sysfs_path "
+		    "changed from '%s' to '%s'", (u_longlong_t)dvd->vdev_guid,
+		    old, new);
+
+		if (dvd->vdev_enc_sysfs_path)
+			spa_strfree(dvd->vdev_enc_sysfs_path);
+
+		if (svd->vdev_enc_sysfs_path) {
+			dvd->vdev_enc_sysfs_path = spa_strdup(
+			    svd->vdev_enc_sysfs_path);
+		} else {
+			dvd->vdev_enc_sysfs_path = NULL;
+		}
+	}
 }
 
 /*