From 2a8430a260595a8f01f34bb695e455517cc0ae11 Mon Sep 17 00:00:00 2001 From: Tony Hutter Date: Mon, 4 Oct 2021 12:32:16 -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. Reviewed-by: Brian Behlendorf Signed-off-by: Tony Hutter Closes #11950 Closes #12095 --- cmd/zpool/Makefile.am | 3 +- cmd/zpool/zpool_iter.c | 50 +---------------- cmd/zpool/zpool_main.c | 3 +- cmd/zpool/zpool_util.h | 2 +- include/libzutil.h | 10 ++++ lib/libzutil/os/freebsd/zutil_import_os.c | 5 ++ lib/libzutil/os/linux/zutil_import_os.c | 67 +++++++++++++++++----- lib/libzutil/zutil_import.c | 68 +++++++++++++++++++++++ module/zfs/vdev.c | 24 ++++++++ 9 files changed, 169 insertions(+), 63 deletions(-) diff --git a/cmd/zpool/Makefile.am b/cmd/zpool/Makefile.am index aad45d4f74..fa494c030e 100644 --- a/cmd/zpool/Makefile.am +++ b/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) diff --git a/cmd/zpool/zpool_iter.c b/cmd/zpool/zpool_iter.c index 3d7a0cfc35..abfa2b7f6b 100644 --- a/cmd/zpool/zpool_iter.c +++ b/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,7 +558,7 @@ 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; @@ -611,6 +566,7 @@ for_each_vdev_run_cb(zpool_handle_t *zhp, nvlist_t *nv, void *cb_vcdl) 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); diff --git a/cmd/zpool/zpool_main.c b/cmd/zpool/zpool_main.c index bfef6fc432..d42ff18d83 100644 --- a/cmd/zpool/zpool_main.c +++ b/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); diff --git a/cmd/zpool/zpool_util.h b/cmd/zpool/zpool_util.h index 4002e57940..6665eaf0d4 100644 --- a/cmd/zpool/zpool_util.h +++ b/cmd/zpool/zpool_util.h @@ -27,6 +27,7 @@ #include #include +#include #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; diff --git a/include/libzutil.h b/include/libzutil.h index ef17bd5426..2329458760 100644 --- a/include/libzutil.h +++ b/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 diff --git a/lib/libzutil/os/freebsd/zutil_import_os.c b/lib/libzutil/os/freebsd/zutil_import_os.c index 2d8900ce24..3da661f4c5 100644 --- a/lib/libzutil/os/freebsd/zutil_import_os.c +++ b/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) +{ +} diff --git a/lib/libzutil/os/linux/zutil_import_os.c b/lib/libzutil/os/linux/zutil_import_os.c index 5defb526f2..ab692401d8 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" @@ -757,6 +758,58 @@ no_dev: #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); diff --git a/lib/libzutil/zutil_import.c b/lib/libzutil/zutil_import.c index 04b9f26abb..9eb55aaf77 100644 --- a/lib/libzutil/zutil_import.c +++ b/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)); +} diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c index 2763bd8de1..0ba76f6b88 100644 --- a/module/zfs/vdev.c +++ b/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; + } + } } /*