From 2f995309907b3a269530174c91f599209e1ffdc1 Mon Sep 17 00:00:00 2001 From: Rob Norris Date: Mon, 16 Dec 2024 07:58:24 +1100 Subject: [PATCH] import: add 'newname' property to rename pool on next import Renaming a pool is possible by providing an additional argument to `zpool import`. This assumes that the operator is able to comfortably export and re-import the pool, which is generally not the case for root/boot pools. This adds a 'newname' pseudo-property to the pool. If it exists at import time, it will be used to the name the pool. This allows the operator to set the new name ahead of the next export/import cycle (eg, before rebooting). The old method of renaming the pool is still available, and takes precedence over the property. The property itself is cleared on every successful import, regardless of the name used. This is justified as a policy of "most recent name wins", but really is just to avoid surprising renames when the property is left set and forgotten. Because the value is needed during import, the property is stored on the label rather than in the props ZAP. TODO: - Consider where this is applied; might be better done entrely inside the module rather than in zpool/libzfs, because it changes the call sequence required a little. - To that end, need to find what alternate pool-importing code does and expects (bootloaders etc) - Understand ZFS_IMPORT_VERBATIM and make this work with it (which is possible the FreeBSD answer to the above two) - General concept and design review. Sponsored-by: https://despairlabs.com/sponsor/ Signed-off-by: Rob Norris --- cmd/zpool/zpool_main.c | 24 +++++++++++++++++++++++- include/sys/fs/zfs.h | 2 ++ include/sys/spa_impl.h | 2 ++ lib/libzfs/libzfs_pool.c | 17 +++++++++++++++++ lib/libzutil/zutil_import.c | 7 +++++++ module/zcommon/zpool_prop.c | 2 ++ module/zfs/spa.c | 32 ++++++++++++++++++++++++++++++++ module/zfs/spa_config.c | 3 +++ 8 files changed, 88 insertions(+), 1 deletion(-) diff --git a/cmd/zpool/zpool_main.c b/cmd/zpool/zpool_main.c index 506427a10672..5f58040a12eb 100644 --- a/cmd/zpool/zpool_main.c +++ b/cmd/zpool/zpool_main.c @@ -3888,7 +3888,29 @@ do_import(nvlist_t *config, const char *newname, const char *mntopts, if (zpool_import_props(g_zfs, config, newname, props, flags) != 0) return (1); - if (newname != NULL) + /* + * Figure out the imported name, so we can use it to open the pool + * and mount the filesystems. + * + * XXX ideally, zpool_import_props() would return the name it used, + * or there would be some other mechanism to get it. Alas, there + * is not. Choices seem to be to break a public function, or to + * smuggle it through one of the nvlists. Not keen on either + * until we get there -- robn, 2024-12-16 + */ + if (newname == NULL) { + int error = nvlist_lookup_string(config, ZPOOL_CONFIG_NEWNAME, + &newname); + if (error == 0) { + fprintf(stderr, gettext("Pool '%s' has newname " + "property set and was renamed to '%s'\n"), name, + newname); + name = newname; + } + else + VERIFY3U(error, ==, ENOENT); + } + else name = newname; if ((zhp = zpool_open_canfail(g_zfs, name)) == NULL) diff --git a/include/sys/fs/zfs.h b/include/sys/fs/zfs.h index dc474e3739f3..eb0962616f44 100644 --- a/include/sys/fs/zfs.h +++ b/include/sys/fs/zfs.h @@ -266,6 +266,7 @@ typedef enum { ZPOOL_PROP_DEDUP_TABLE_QUOTA, ZPOOL_PROP_DEDUPCACHED, ZPOOL_PROP_LAST_SCRUBBED_TXG, + ZPOOL_PROP_NEWNAME, ZPOOL_NUM_PROPS } zpool_prop_t; @@ -865,6 +866,7 @@ typedef struct zpool_load_policy { #define ZPOOL_CONFIG_EXPANSION_TIME "expansion_time" /* not stored */ #define ZPOOL_CONFIG_REBUILD_STATS "org.openzfs:rebuild_stats" #define ZPOOL_CONFIG_COMPATIBILITY "compatibility" +#define ZPOOL_CONFIG_NEWNAME "org.openzfs:newname" /* * The persistent vdev state is stored as separate values rather than a single diff --git a/include/sys/spa_impl.h b/include/sys/spa_impl.h index b0a2d46ff2c4..2ff2205d7b70 100644 --- a/include/sys/spa_impl.h +++ b/include/sys/spa_impl.h @@ -472,6 +472,8 @@ struct spa { boolean_t spa_waiters_cancel; /* waiters should return */ char *spa_compatibility; /* compatibility file(s) */ + char *spa_newname; /* pool name on next import */ + uint64_t spa_dedup_table_quota; /* property DDT maximum size */ uint64_t spa_dedup_dsize; /* cached on-disk size of DDT */ uint64_t spa_dedup_class_full_txg; /* txg dedup class was full */ diff --git a/lib/libzfs/libzfs_pool.c b/lib/libzfs/libzfs_pool.c index f256535e8ea0..cec0049ff2ac 100644 --- a/lib/libzfs/libzfs_pool.c +++ b/lib/libzfs/libzfs_pool.c @@ -307,6 +307,7 @@ zpool_get_prop(zpool_handle_t *zhp, zpool_prop_t prop, char *buf, case ZPOOL_PROP_CACHEFILE: case ZPOOL_PROP_COMMENT: case ZPOOL_PROP_COMPATIBILITY: + case ZPOOL_PROP_NEWNAME: if (zhp->zpool_props != NULL || zpool_get_all_props(zhp) == 0) { (void) strlcpy(buf, @@ -876,6 +877,13 @@ zpool_valid_proplist(libzfs_handle_t *hdl, const char *poolname, "any effect\n", propname); break; + case ZPOOL_PROP_NEWNAME: + if (zpool_name_valid(hdl, B_FALSE, strval)) + break; + zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, + "invalid pool name '%s'"), strval); + (void) zfs_error(hdl, EZFS_INVALIDNAME, errbuf); + goto error; default: break; } @@ -2193,6 +2201,15 @@ zpool_import_props(libzfs_handle_t *hdl, nvlist_t *config, const char *newname, (void) snprintf(errbuf, sizeof (errbuf), dgettext(TEXT_DOMAIN, "cannot import pool '%s'"), origname); + if (newname == NULL) { + error = nvlist_lookup_string(config, ZPOOL_CONFIG_NEWNAME, + &newname); + if (error != 0 && error != ENOENT) + return (zpool_standard_error(hdl, error, + dgettext(TEXT_DOMAIN, "error looking up newname"))); + error = 0; + } + if (newname != NULL) { if (!zpool_name_valid(hdl, B_FALSE, newname)) return (zfs_error_fmt(hdl, EZFS_INVALIDNAME, diff --git a/lib/libzutil/zutil_import.c b/lib/libzutil/zutil_import.c index 77fa0ce38b2a..2890763e8ddd 100644 --- a/lib/libzutil/zutil_import.c +++ b/lib/libzutil/zutil_import.c @@ -589,10 +589,12 @@ get_configs(libpc_handle_t *hdl, pool_list_t *pl, boolean_t active_ok, * pool state * hostid (if available) * hostname (if available) + * new name (if available) */ uint64_t state, version; const char *comment = NULL; const char *compatibility = NULL; + const char *newname = NULL; version = fnvlist_lookup_uint64(tmp, ZPOOL_CONFIG_VERSION); @@ -635,6 +637,11 @@ get_configs(libpc_handle_t *hdl, pool_list_t *pl, boolean_t active_ok, ZPOOL_CONFIG_HOSTNAME, hostname); } + if (nvlist_lookup_string(tmp, + ZPOOL_CONFIG_NEWNAME, &newname) == 0) + fnvlist_add_string(config, + ZPOOL_CONFIG_NEWNAME, newname); + config_seen = B_TRUE; } diff --git a/module/zcommon/zpool_prop.c b/module/zcommon/zpool_prop.c index a709679b9032..6280f2f38965 100644 --- a/module/zcommon/zpool_prop.c +++ b/module/zcommon/zpool_prop.c @@ -82,6 +82,8 @@ zpool_prop_init(void) zprop_register_string(ZPOOL_PROP_COMPATIBILITY, "compatibility", "off", PROP_DEFAULT, ZFS_TYPE_POOL, " | off | legacy", "COMPATIBILITY", sfeatures); + zprop_register_string(ZPOOL_PROP_NEWNAME, "newname", NULL, + PROP_DEFAULT, ZFS_TYPE_POOL, "", "NEWNAME", sfeatures); /* readonly number properties */ zprop_register_number(ZPOOL_PROP_SIZE, "size", 0, PROP_READONLY, diff --git a/module/zfs/spa.c b/module/zfs/spa.c index b83c982c13fd..5fa0b6afcce3 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -504,6 +504,11 @@ spa_prop_get_config(spa_t *spa, nvlist_t *nv) spa->spa_compatibility, 0, ZPROP_SRC_LOCAL); } + if (spa->spa_newname != NULL) { + spa_prop_add_list(nv, ZPOOL_PROP_NEWNAME, + spa->spa_newname, 0, ZPROP_SRC_LOCAL); + } + if (spa->spa_root != NULL) spa_prop_add_list(nv, ZPOOL_PROP_ALTROOT, spa->spa_root, 0, ZPROP_SRC_LOCAL); @@ -3900,6 +3905,7 @@ spa_ld_parse_config(spa_t *spa, spa_import_type_t type) uint64_t pool_guid; const char *comment; const char *compatibility; + const char *newname; /* * Versioning wasn't explicitly added to the label until later, so if @@ -3953,6 +3959,10 @@ spa_ld_parse_config(spa_t *spa, spa_import_type_t type) &compatibility) == 0) spa->spa_compatibility = spa_strdup(compatibility); + ASSERT(spa->spa_newname == NULL); + if (nvlist_lookup_string(config, ZPOOL_CONFIG_NEWNAME, &newname) == 0) + spa->spa_newname = spa_strdup(newname); + (void) nvlist_lookup_uint64(config, ZPOOL_CONFIG_POOL_TXG, &spa->spa_config_txg); @@ -6861,6 +6871,12 @@ spa_import(char *pool, nvlist_t *config, nvlist_t *props, uint64_t flags) spa_aux_check_removed(&spa->spa_l2cache); } + /* Clear newname after each successful import */ + if (spa->spa_newname != NULL) { + spa_strfree(spa->spa_newname); + spa->spa_newname = NULL; + } + if (spa_writeable(spa)) { /* * Update the config cache to include the newly-imported pool. @@ -9642,6 +9658,22 @@ spa_sync_props(void *arg, dmu_tx_t *tx) spa_async_request(spa, SPA_ASYNC_CONFIG_UPDATE); } + spa_history_log_internal(spa, "set", tx, + "%s=%s", nvpair_name(elem), strval); + break; + case ZPOOL_PROP_NEWNAME: + strval = fnvpair_value_string(elem); + if (spa->spa_newname != NULL) + spa_strfree(spa->spa_newname); + spa->spa_newname = spa_strdup(strval); + /* + * Dirty the configuration on vdevs as above. + */ + if (tx->tx_txg != TXG_INITIAL) { + vdev_config_dirty(spa->spa_root_vdev); + spa_async_request(spa, SPA_ASYNC_CONFIG_UPDATE); + } + spa_history_log_internal(spa, "set", tx, "%s=%s", nvpair_name(elem), strval); break; diff --git a/module/zfs/spa_config.c b/module/zfs/spa_config.c index a77874ea0dd3..500ae2aeffca 100644 --- a/module/zfs/spa_config.c +++ b/module/zfs/spa_config.c @@ -466,6 +466,9 @@ spa_config_generate(spa_t *spa, vdev_t *vd, uint64_t txg, int getstats) if (spa->spa_compatibility != NULL) fnvlist_add_string(config, ZPOOL_CONFIG_COMPATIBILITY, spa->spa_compatibility); + if (spa->spa_newname != NULL) + fnvlist_add_string(config, ZPOOL_CONFIG_NEWNAME, + spa->spa_newname); hostid = spa_get_hostid(spa); if (hostid != 0)