From eb6f84920d44d933a357f3fdcd318fa352afcef2 Mon Sep 17 00:00:00 2001 From: Tony Hutter Date: Thu, 20 May 2021 14:53:41 -0700 Subject: [PATCH] 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. The largest part of this patch is moving for_each_vdev_vdev() and associated function from zpool_iter.c to libzutil. This was needed to make those functions visible to the cache import codepath for sysfs path re-scan. Signed-off-by: Tony Hutter Fixes: #11950 --- cmd/zpool/zpool_iter.c | 66 ------------------ cmd/zpool/zpool_util.h | 4 -- include/libzfs.h | 7 ++ include/libzutil.h | 1 + lib/libzfs/Makefile.am | 3 +- lib/libzfs/libzfs_config.c | 12 ---- lib/libzutil/Makefile.am | 2 +- lib/libzutil/os/linux/zutil_import_os.c | 68 ++++++++++++++---- lib/libzutil/zutil_import.c | 2 + lib/libzutil/zutil_pool.c | 91 +++++++++++++++++++++++++ module/zfs/vdev.c | 17 +++++ 11 files changed, 176 insertions(+), 97 deletions(-) diff --git a/cmd/zpool/zpool_iter.c b/cmd/zpool/zpool_iter.c index 3d7a0cfc35e6..16813da6f64b 100644 --- a/cmd/zpool/zpool_iter.c +++ b/cmd/zpool/zpool_iter.c @@ -264,72 +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 - * each one. - * - * @zhp: Zpool handle - * @func: Function to call on each vdev - * @data: Custom data to pass to the function - */ -int -for_each_vdev(zpool_handle_t *zhp, pool_vdev_iter_f func, void *data) -{ - nvlist_t *config, *nvroot = NULL; - - if ((config = zpool_get_config(zhp, NULL)) != NULL) { - verify(nvlist_lookup_nvlist(config, ZPOOL_CONFIG_VDEV_TREE, - &nvroot) == 0); - } - return (for_each_vdev_cb(zhp, nvroot, func, data)); -} - /* * Process the vcdl->vdev_cmd_data[] array to figure out all the unique column * names and their widths. When this function is done, vcdl->uniq_cols, diff --git a/cmd/zpool/zpool_util.h b/cmd/zpool/zpool_util.h index 71db4dc35608..2f358b585e4d 100644 --- a/cmd/zpool/zpool_util.h +++ b/cmd/zpool/zpool_util.h @@ -67,10 +67,6 @@ nvlist_t *split_mirror_vdev(zpool_handle_t *zhp, char *newname, 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; zpool_list_t *pool_list_get(int, char **, zprop_list_t **, boolean_t, int *); diff --git a/include/libzfs.h b/include/libzfs.h index 9ef280636d4c..dfb7ebc248d7 100644 --- a/include/libzfs.h +++ b/include/libzfs.h @@ -828,6 +828,13 @@ _LIBZFS_H boolean_t zfs_dataset_exists(libzfs_handle_t *, const char *, _LIBZFS_H int zfs_spa_version(zfs_handle_t *, int *); _LIBZFS_H boolean_t zfs_bookmark_exists(const char *path); +/* Vdev list functions */ +typedef int (*pool_vdev_iter_f)(zpool_handle_t *, nvlist_t *, void *); +extern int for_each_vdev(zpool_handle_t *zhp, pool_vdev_iter_f func, + void *data); +extern int for_each_vdev_in_nvlist(nvlist_t *nvroot, pool_vdev_iter_f func, + void *data); + /* * Mount support functions. */ diff --git a/include/libzutil.h b/include/libzutil.h index 5b0927961800..8653c098bfaf 100644 --- a/include/libzutil.h +++ b/include/libzutil.h @@ -84,6 +84,7 @@ struct udev_device; _LIBZUTIL_H int zfs_device_get_devid(struct udev_device *, char *, size_t); _LIBZUTIL_H int zfs_device_get_physical(struct udev_device *, char *, size_t); +_LIBZUTIL_H void update_vdevs_config_dev_sysfs_path(nvlist_t *config); _LIBZUTIL_H void update_vdev_config_dev_strs(nvlist_t *); /* diff --git a/lib/libzfs/Makefile.am b/lib/libzfs/Makefile.am index e3527ffe7058..7e7776755558 100644 --- a/lib/libzfs/Makefile.am +++ b/lib/libzfs/Makefile.am @@ -75,7 +75,8 @@ libzfs_la_LIBADD = \ $(abs_top_builddir)/lib/libshare/libshare.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 libzfs_la_LIBADD += -lm $(LIBCRYPTO_LIBS) $(ZLIB_LIBS) $(LIBFETCH_LIBS) $(LTLIBINTL) diff --git a/lib/libzfs/libzfs_config.c b/lib/libzfs/libzfs_config.c index a3ecc4a327dc..8b578dacac7c 100644 --- a/lib/libzfs/libzfs_config.c +++ b/lib/libzfs/libzfs_config.c @@ -212,18 +212,6 @@ namespace_reload(libzfs_handle_t *hdl) return (0); } -/* - * Retrieve the configuration for the given pool. The configuration is an nvlist - * describing the vdevs, as well as the statistics associated with each one. - */ -nvlist_t * -zpool_get_config(zpool_handle_t *zhp, nvlist_t **oldconfig) -{ - if (oldconfig) - *oldconfig = zhp->zpool_old_config; - return (zhp->zpool_config); -} - /* * Retrieves a list of enabled features and their refcounts and caches it in * the pool handle. diff --git a/lib/libzutil/Makefile.am b/lib/libzutil/Makefile.am index 2f0357e9f900..1bcc1679ed6c 100644 --- a/lib/libzutil/Makefile.am +++ b/lib/libzutil/Makefile.am @@ -4,7 +4,7 @@ include $(top_srcdir)/config/Rules.am AM_CFLAGS += $(NO_UNUSED_BUT_SET_VARIABLE) AM_CFLAGS += $(LIBBLKID_CFLAGS) $(LIBUDEV_CFLAGS) -DEFAULT_INCLUDES += -I$(srcdir) +DEFAULT_INCLUDES += -I$(srcdir) -I$(top_srcdir)/lib/libzfs noinst_LTLIBRARIES = libzutil.la diff --git a/lib/libzutil/os/linux/zutil_import_os.c b/lib/libzutil/os/linux/zutil_import_os.c index 84c3cb44fec7..40689fe58802 100644 --- a/lib/libzutil/os/linux/zutil_import_os.c +++ b/lib/libzutil/os/linux/zutil_import_os.c @@ -65,6 +65,7 @@ #include #include #include +#include #include "zutil_import.h" @@ -775,6 +776,59 @@ 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(zpool_handle_t *hdl, 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 * @@ -801,7 +855,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 @@ -848,18 +901,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); diff --git a/lib/libzutil/zutil_import.c b/lib/libzutil/zutil_import.c index 9d7fcb8d9685..9eac9ee2cd7c 100644 --- a/lib/libzutil/zutil_import.c +++ b/lib/libzutil/zutil_import.c @@ -1676,6 +1676,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); diff --git a/lib/libzutil/zutil_pool.c b/lib/libzutil/zutil_pool.c index 734650f3cffc..87130114458d 100644 --- a/lib/libzutil/zutil_pool.c +++ b/lib/libzutil/zutil_pool.c @@ -30,6 +30,8 @@ #include #include +#include +#include static void dump_ddt_stat(const ddt_stat_t *dds, int h) @@ -143,3 +145,92 @@ zpool_history_unpack(char *buf, uint64_t bytes_read, uint64_t *leftover, *leftover = bytes_read; return (0); } + +/* + * Retrieve the configuration for the given pool. The configuration is an nvlist + * describing the vdevs, as well as the statistics associated with each one. + */ +nvlist_t * +zpool_get_config(zpool_handle_t *zhp, nvlist_t **oldconfig) +{ + if (oldconfig) + *oldconfig = zhp->zpool_old_config; + return (zhp->zpool_config); +} + +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 through + * all vdevs in the pool, ignoring root vdevs and holes, calling func() on + * each one. + * + * @zhp: Zpool handle + * @func: Function to call on each vdev + * @data: Custom data to pass to the function + */ +int +for_each_vdev(zpool_handle_t *zhp, pool_vdev_iter_f func, void *data) +{ + nvlist_t *config, *nvroot = NULL; + + if ((config = zpool_get_config(zhp, NULL)) != NULL) { + verify(nvlist_lookup_nvlist(config, ZPOOL_CONFIG_VDEV_TREE, + &nvroot) == 0); + } + return (for_each_vdev_cb(zhp, nvroot, func, data)); +} + +/* + * 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)); +} diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c index 5e14d71f1946..e7a32dac3a6f 100644 --- a/module/zfs/vdev.c +++ b/module/zfs/vdev.c @@ -2386,6 +2386,23 @@ 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 */ + if (strcmp(svd->vdev_enc_sysfs_path, dvd->vdev_enc_sysfs_path) != 0) { + zfs_dbgmsg("vdev_copy_path: vdev %llu: vdev_enc_sysfs_path " + "changed from '%s' to '%s'", (u_longlong_t)dvd->vdev_guid, + dvd->vdev_enc_sysfs_path, svd->vdev_enc_sysfs_path); + + 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; + } + } } /*