spa_prop_get: require caller to supply output nvlist

All callers to spa_prop_get() and spa_prop_get_nvlist() supplied their
own preallocated nvlist (except ztest), so we can remove the option to
have them allocate one if none is supplied.

This sidesteps a bug in spa_prop_get(), where the error var wasn't
initialised, which could lead to the provided nvlist being freed at the
end.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Jorgen Lundman <lundman@lundman.net>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes #16505
This commit is contained in:
Rob Norris 2024-09-07 01:45:58 +10:00 committed by GitHub
parent 17dd66deda
commit b109925820
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 53 additions and 66 deletions

View File

@ -6215,13 +6215,14 @@ void
ztest_spa_prop_get_set(ztest_ds_t *zd, uint64_t id) ztest_spa_prop_get_set(ztest_ds_t *zd, uint64_t id)
{ {
(void) zd, (void) id; (void) zd, (void) id;
nvlist_t *props = NULL;
(void) pthread_rwlock_rdlock(&ztest_name_lock); (void) pthread_rwlock_rdlock(&ztest_name_lock);
(void) ztest_spa_prop_set_uint64(ZPOOL_PROP_AUTOTRIM, ztest_random(2)); (void) ztest_spa_prop_set_uint64(ZPOOL_PROP_AUTOTRIM, ztest_random(2));
VERIFY0(spa_prop_get(ztest_spa, &props)); nvlist_t *props = fnvlist_alloc();
VERIFY0(spa_prop_get(ztest_spa, props));
if (ztest_opts.zo_verbose >= 6) if (ztest_opts.zo_verbose >= 6)
dump_nvlist(props, 4); dump_nvlist(props, 4);

View File

@ -1201,9 +1201,9 @@ extern void spa_boot_init(void);
/* properties */ /* properties */
extern int spa_prop_set(spa_t *spa, nvlist_t *nvp); extern int spa_prop_set(spa_t *spa, nvlist_t *nvp);
extern int spa_prop_get(spa_t *spa, nvlist_t **nvp); extern int spa_prop_get(spa_t *spa, nvlist_t *nvp);
extern int spa_prop_get_nvlist(spa_t *spa, char **props, extern int spa_prop_get_nvlist(spa_t *spa, char **props,
unsigned int n_props, nvlist_t **outnvl); unsigned int n_props, nvlist_t *outnvl);
extern void spa_prop_clear_bootfs(spa_t *spa, uint64_t obj, dmu_tx_t *tx); extern void spa_prop_clear_bootfs(spa_t *spa, uint64_t obj, dmu_tx_t *tx);
extern void spa_configfile_set(spa_t *, nvlist_t *, boolean_t); extern void spa_configfile_set(spa_t *, nvlist_t *, boolean_t);

View File

@ -366,21 +366,15 @@ spa_prop_add(spa_t *spa, const char *propname, nvlist_t *outnvl)
int int
spa_prop_get_nvlist(spa_t *spa, char **props, unsigned int n_props, spa_prop_get_nvlist(spa_t *spa, char **props, unsigned int n_props,
nvlist_t **outnvl) nvlist_t *outnvl)
{ {
int err = 0; int err = 0;
if (props == NULL) if (props == NULL)
return (0); return (0);
if (*outnvl == NULL) {
err = nvlist_alloc(outnvl, NV_UNIQUE_NAME, KM_SLEEP);
if (err)
return (err);
}
for (unsigned int i = 0; i < n_props && err == 0; i++) { for (unsigned int i = 0; i < n_props && err == 0; i++) {
err = spa_prop_add(spa, props[i], *outnvl); err = spa_prop_add(spa, props[i], outnvl);
} }
return (err); return (err);
@ -406,7 +400,7 @@ spa_prop_add_user(nvlist_t *nvl, const char *propname, char *strval,
* Get property values from the spa configuration. * Get property values from the spa configuration.
*/ */
static void static void
spa_prop_get_config(spa_t *spa, nvlist_t **nvp) spa_prop_get_config(spa_t *spa, nvlist_t *nv)
{ {
vdev_t *rvd = spa->spa_root_vdev; vdev_t *rvd = spa->spa_root_vdev;
dsl_pool_t *pool = spa->spa_dsl_pool; dsl_pool_t *pool = spa->spa_dsl_pool;
@ -428,48 +422,48 @@ spa_prop_get_config(spa_t *spa, nvlist_t **nvp)
size += metaslab_class_get_space(spa_dedup_class(spa)); size += metaslab_class_get_space(spa_dedup_class(spa));
size += metaslab_class_get_space(spa_embedded_log_class(spa)); size += metaslab_class_get_space(spa_embedded_log_class(spa));
spa_prop_add_list(*nvp, ZPOOL_PROP_NAME, spa_name(spa), 0, src); spa_prop_add_list(nv, ZPOOL_PROP_NAME, spa_name(spa), 0, src);
spa_prop_add_list(*nvp, ZPOOL_PROP_SIZE, NULL, size, src); spa_prop_add_list(nv, ZPOOL_PROP_SIZE, NULL, size, src);
spa_prop_add_list(*nvp, ZPOOL_PROP_ALLOCATED, NULL, alloc, src); spa_prop_add_list(nv, ZPOOL_PROP_ALLOCATED, NULL, alloc, src);
spa_prop_add_list(*nvp, ZPOOL_PROP_FREE, NULL, spa_prop_add_list(nv, ZPOOL_PROP_FREE, NULL,
size - alloc, src); size - alloc, src);
spa_prop_add_list(*nvp, ZPOOL_PROP_CHECKPOINT, NULL, spa_prop_add_list(nv, ZPOOL_PROP_CHECKPOINT, NULL,
spa->spa_checkpoint_info.sci_dspace, src); spa->spa_checkpoint_info.sci_dspace, src);
spa_prop_add_list(*nvp, ZPOOL_PROP_FRAGMENTATION, NULL, spa_prop_add_list(nv, ZPOOL_PROP_FRAGMENTATION, NULL,
metaslab_class_fragmentation(mc), src); metaslab_class_fragmentation(mc), src);
spa_prop_add_list(*nvp, ZPOOL_PROP_EXPANDSZ, NULL, spa_prop_add_list(nv, ZPOOL_PROP_EXPANDSZ, NULL,
metaslab_class_expandable_space(mc), src); metaslab_class_expandable_space(mc), src);
spa_prop_add_list(*nvp, ZPOOL_PROP_READONLY, NULL, spa_prop_add_list(nv, ZPOOL_PROP_READONLY, NULL,
(spa_mode(spa) == SPA_MODE_READ), src); (spa_mode(spa) == SPA_MODE_READ), src);
cap = (size == 0) ? 0 : (alloc * 100 / size); cap = (size == 0) ? 0 : (alloc * 100 / size);
spa_prop_add_list(*nvp, ZPOOL_PROP_CAPACITY, NULL, cap, src); spa_prop_add_list(nv, ZPOOL_PROP_CAPACITY, NULL, cap, src);
spa_prop_add_list(*nvp, ZPOOL_PROP_DEDUPRATIO, NULL, spa_prop_add_list(nv, ZPOOL_PROP_DEDUPRATIO, NULL,
ddt_get_pool_dedup_ratio(spa), src); ddt_get_pool_dedup_ratio(spa), src);
spa_prop_add_list(*nvp, ZPOOL_PROP_BCLONEUSED, NULL, spa_prop_add_list(nv, ZPOOL_PROP_BCLONEUSED, NULL,
brt_get_used(spa), src); brt_get_used(spa), src);
spa_prop_add_list(*nvp, ZPOOL_PROP_BCLONESAVED, NULL, spa_prop_add_list(nv, ZPOOL_PROP_BCLONESAVED, NULL,
brt_get_saved(spa), src); brt_get_saved(spa), src);
spa_prop_add_list(*nvp, ZPOOL_PROP_BCLONERATIO, NULL, spa_prop_add_list(nv, ZPOOL_PROP_BCLONERATIO, NULL,
brt_get_ratio(spa), src); brt_get_ratio(spa), src);
spa_prop_add_list(*nvp, ZPOOL_PROP_DEDUP_TABLE_SIZE, NULL, spa_prop_add_list(nv, ZPOOL_PROP_DEDUP_TABLE_SIZE, NULL,
ddt_get_ddt_dsize(spa), src); ddt_get_ddt_dsize(spa), src);
spa_prop_add_list(*nvp, ZPOOL_PROP_HEALTH, NULL, spa_prop_add_list(nv, ZPOOL_PROP_HEALTH, NULL,
rvd->vdev_state, src); rvd->vdev_state, src);
version = spa_version(spa); version = spa_version(spa);
if (version == zpool_prop_default_numeric(ZPOOL_PROP_VERSION)) { if (version == zpool_prop_default_numeric(ZPOOL_PROP_VERSION)) {
spa_prop_add_list(*nvp, ZPOOL_PROP_VERSION, NULL, spa_prop_add_list(nv, ZPOOL_PROP_VERSION, NULL,
version, ZPROP_SRC_DEFAULT); version, ZPROP_SRC_DEFAULT);
} else { } else {
spa_prop_add_list(*nvp, ZPOOL_PROP_VERSION, NULL, spa_prop_add_list(nv, ZPOOL_PROP_VERSION, NULL,
version, ZPROP_SRC_LOCAL); version, ZPROP_SRC_LOCAL);
} }
spa_prop_add_list(*nvp, ZPOOL_PROP_LOAD_GUID, spa_prop_add_list(nv, ZPOOL_PROP_LOAD_GUID,
NULL, spa_load_guid(spa), src); NULL, spa_load_guid(spa), src);
} }
@ -479,62 +473,62 @@ spa_prop_get_config(spa_t *spa, nvlist_t **nvp)
* when opening pools before this version freedir will be NULL. * when opening pools before this version freedir will be NULL.
*/ */
if (pool->dp_free_dir != NULL) { if (pool->dp_free_dir != NULL) {
spa_prop_add_list(*nvp, ZPOOL_PROP_FREEING, NULL, spa_prop_add_list(nv, ZPOOL_PROP_FREEING, NULL,
dsl_dir_phys(pool->dp_free_dir)->dd_used_bytes, dsl_dir_phys(pool->dp_free_dir)->dd_used_bytes,
src); src);
} else { } else {
spa_prop_add_list(*nvp, ZPOOL_PROP_FREEING, spa_prop_add_list(nv, ZPOOL_PROP_FREEING,
NULL, 0, src); NULL, 0, src);
} }
if (pool->dp_leak_dir != NULL) { if (pool->dp_leak_dir != NULL) {
spa_prop_add_list(*nvp, ZPOOL_PROP_LEAKED, NULL, spa_prop_add_list(nv, ZPOOL_PROP_LEAKED, NULL,
dsl_dir_phys(pool->dp_leak_dir)->dd_used_bytes, dsl_dir_phys(pool->dp_leak_dir)->dd_used_bytes,
src); src);
} else { } else {
spa_prop_add_list(*nvp, ZPOOL_PROP_LEAKED, spa_prop_add_list(nv, ZPOOL_PROP_LEAKED,
NULL, 0, src); NULL, 0, src);
} }
} }
spa_prop_add_list(*nvp, ZPOOL_PROP_GUID, NULL, spa_guid(spa), src); spa_prop_add_list(nv, ZPOOL_PROP_GUID, NULL, spa_guid(spa), src);
if (spa->spa_comment != NULL) { if (spa->spa_comment != NULL) {
spa_prop_add_list(*nvp, ZPOOL_PROP_COMMENT, spa->spa_comment, spa_prop_add_list(nv, ZPOOL_PROP_COMMENT, spa->spa_comment,
0, ZPROP_SRC_LOCAL); 0, ZPROP_SRC_LOCAL);
} }
if (spa->spa_compatibility != NULL) { if (spa->spa_compatibility != NULL) {
spa_prop_add_list(*nvp, ZPOOL_PROP_COMPATIBILITY, spa_prop_add_list(nv, ZPOOL_PROP_COMPATIBILITY,
spa->spa_compatibility, 0, ZPROP_SRC_LOCAL); spa->spa_compatibility, 0, ZPROP_SRC_LOCAL);
} }
if (spa->spa_root != NULL) if (spa->spa_root != NULL)
spa_prop_add_list(*nvp, ZPOOL_PROP_ALTROOT, spa->spa_root, spa_prop_add_list(nv, ZPOOL_PROP_ALTROOT, spa->spa_root,
0, ZPROP_SRC_LOCAL); 0, ZPROP_SRC_LOCAL);
if (spa_feature_is_enabled(spa, SPA_FEATURE_LARGE_BLOCKS)) { if (spa_feature_is_enabled(spa, SPA_FEATURE_LARGE_BLOCKS)) {
spa_prop_add_list(*nvp, ZPOOL_PROP_MAXBLOCKSIZE, NULL, spa_prop_add_list(nv, ZPOOL_PROP_MAXBLOCKSIZE, NULL,
MIN(zfs_max_recordsize, SPA_MAXBLOCKSIZE), ZPROP_SRC_NONE); MIN(zfs_max_recordsize, SPA_MAXBLOCKSIZE), ZPROP_SRC_NONE);
} else { } else {
spa_prop_add_list(*nvp, ZPOOL_PROP_MAXBLOCKSIZE, NULL, spa_prop_add_list(nv, ZPOOL_PROP_MAXBLOCKSIZE, NULL,
SPA_OLD_MAXBLOCKSIZE, ZPROP_SRC_NONE); SPA_OLD_MAXBLOCKSIZE, ZPROP_SRC_NONE);
} }
if (spa_feature_is_enabled(spa, SPA_FEATURE_LARGE_DNODE)) { if (spa_feature_is_enabled(spa, SPA_FEATURE_LARGE_DNODE)) {
spa_prop_add_list(*nvp, ZPOOL_PROP_MAXDNODESIZE, NULL, spa_prop_add_list(nv, ZPOOL_PROP_MAXDNODESIZE, NULL,
DNODE_MAX_SIZE, ZPROP_SRC_NONE); DNODE_MAX_SIZE, ZPROP_SRC_NONE);
} else { } else {
spa_prop_add_list(*nvp, ZPOOL_PROP_MAXDNODESIZE, NULL, spa_prop_add_list(nv, ZPOOL_PROP_MAXDNODESIZE, NULL,
DNODE_MIN_SIZE, ZPROP_SRC_NONE); DNODE_MIN_SIZE, ZPROP_SRC_NONE);
} }
if ((dp = list_head(&spa->spa_config_list)) != NULL) { if ((dp = list_head(&spa->spa_config_list)) != NULL) {
if (dp->scd_path == NULL) { if (dp->scd_path == NULL) {
spa_prop_add_list(*nvp, ZPOOL_PROP_CACHEFILE, spa_prop_add_list(nv, ZPOOL_PROP_CACHEFILE,
"none", 0, ZPROP_SRC_LOCAL); "none", 0, ZPROP_SRC_LOCAL);
} else if (strcmp(dp->scd_path, spa_config_path) != 0) { } else if (strcmp(dp->scd_path, spa_config_path) != 0) {
spa_prop_add_list(*nvp, ZPOOL_PROP_CACHEFILE, spa_prop_add_list(nv, ZPOOL_PROP_CACHEFILE,
dp->scd_path, 0, ZPROP_SRC_LOCAL); dp->scd_path, 0, ZPROP_SRC_LOCAL);
} }
} }
@ -544,19 +538,13 @@ spa_prop_get_config(spa_t *spa, nvlist_t **nvp)
* Get zpool property values. * Get zpool property values.
*/ */
int int
spa_prop_get(spa_t *spa, nvlist_t **nvp) spa_prop_get(spa_t *spa, nvlist_t *nv)
{ {
objset_t *mos = spa->spa_meta_objset; objset_t *mos = spa->spa_meta_objset;
zap_cursor_t zc; zap_cursor_t zc;
zap_attribute_t za; zap_attribute_t za;
dsl_pool_t *dp; dsl_pool_t *dp;
int err; int err = 0;
if (*nvp == NULL) {
err = nvlist_alloc(nvp, NV_UNIQUE_NAME, KM_SLEEP);
if (err)
return (err);
}
dp = spa_get_dsl(spa); dp = spa_get_dsl(spa);
dsl_pool_config_enter(dp, FTAG); dsl_pool_config_enter(dp, FTAG);
@ -565,7 +553,7 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp)
/* /*
* Get properties from the spa config. * Get properties from the spa config.
*/ */
spa_prop_get_config(spa, nvp); spa_prop_get_config(spa, nv);
/* If no pool property object, no more prop to get. */ /* If no pool property object, no more prop to get. */
if (mos == NULL || spa->spa_pool_props_object == 0) if (mos == NULL || spa->spa_pool_props_object == 0)
@ -610,7 +598,7 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp)
intval = za.za_first_integer; intval = za.za_first_integer;
} }
spa_prop_add_list(*nvp, prop, strval, intval, src); spa_prop_add_list(nv, prop, strval, intval, src);
if (strval != NULL) if (strval != NULL)
kmem_free(strval, ZFS_MAX_DATASET_NAME_LEN); kmem_free(strval, ZFS_MAX_DATASET_NAME_LEN);
@ -627,10 +615,10 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp)
break; break;
} }
if (prop != ZPOOL_PROP_INVAL) { if (prop != ZPOOL_PROP_INVAL) {
spa_prop_add_list(*nvp, prop, strval, 0, src); spa_prop_add_list(nv, prop, strval, 0, src);
} else { } else {
src = ZPROP_SRC_LOCAL; src = ZPROP_SRC_LOCAL;
spa_prop_add_user(*nvp, za.za_name, strval, spa_prop_add_user(nv, za.za_name, strval,
src); src);
} }
kmem_free(strval, za.za_num_integers); kmem_free(strval, za.za_num_integers);
@ -644,11 +632,9 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp)
out: out:
mutex_exit(&spa->spa_props_lock); mutex_exit(&spa->spa_props_lock);
dsl_pool_config_exit(dp, FTAG); dsl_pool_config_exit(dp, FTAG);
if (err && err != ENOENT) {
nvlist_free(*nvp); if (err && err != ENOENT)
*nvp = NULL;
return (err); return (err);
}
return (0); return (0);
} }

View File

@ -3050,7 +3050,6 @@ static const zfs_ioc_key_t zfs_keys_get_props[] = {
static int static int
zfs_ioc_pool_get_props(const char *pool, nvlist_t *innvl, nvlist_t *outnvl) zfs_ioc_pool_get_props(const char *pool, nvlist_t *innvl, nvlist_t *outnvl)
{ {
nvlist_t *nvp = outnvl;
spa_t *spa; spa_t *spa;
char **props = NULL; char **props = NULL;
unsigned int n_props = 0; unsigned int n_props = 0;
@ -3069,16 +3068,17 @@ zfs_ioc_pool_get_props(const char *pool, nvlist_t *innvl, nvlist_t *outnvl)
*/ */
mutex_enter(&spa_namespace_lock); mutex_enter(&spa_namespace_lock);
if ((spa = spa_lookup(pool)) != NULL) { if ((spa = spa_lookup(pool)) != NULL) {
error = spa_prop_get(spa, &nvp); error = spa_prop_get(spa, outnvl);
if (error == 0 && props != NULL) if (error == 0 && props != NULL)
error = spa_prop_get_nvlist(spa, props, n_props, error = spa_prop_get_nvlist(spa, props, n_props,
&nvp); outnvl);
} }
mutex_exit(&spa_namespace_lock); mutex_exit(&spa_namespace_lock);
} else { } else {
error = spa_prop_get(spa, &nvp); error = spa_prop_get(spa, outnvl);
if (error == 0 && props != NULL) if (error == 0 && props != NULL)
error = spa_prop_get_nvlist(spa, props, n_props, &nvp); error = spa_prop_get_nvlist(spa, props, n_props,
outnvl);
spa_close(spa, FTAG); spa_close(spa, FTAG);
} }