git: d59a76183470 - main - spa_prop_get: require caller to supply output nvlist
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 05 Sep 2024 17:28:14 UTC
The branch main has been updated by glebius:
URL: https://cgit.FreeBSD.org/src/commit/?id=d59a76183470685bdf0b88013d2baad1f04f030f
commit d59a76183470685bdf0b88013d2baad1f04f030f
Author: Rob Norris <rob.norris@klarasystems.com>
AuthorDate: 2024-09-04 02:09:43 +0000
Commit: Gleb Smirnoff <glebius@FreeBSD.org>
CommitDate: 2024-09-05 17:27:35 +0000
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.
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
(cherry picked from commit 366d6fecf6bb3c59668b0f3b89f2a610595f3d2f)
---
sys/contrib/openzfs/cmd/ztest.c | 5 +-
sys/contrib/openzfs/include/sys/spa.h | 4 +-
sys/contrib/openzfs/module/zfs/spa.c | 100 +++++++++++++----------------
sys/contrib/openzfs/module/zfs/zfs_ioctl.c | 10 +--
4 files changed, 53 insertions(+), 66 deletions(-)
diff --git a/sys/contrib/openzfs/cmd/ztest.c b/sys/contrib/openzfs/cmd/ztest.c
index 6a9264ddcc4c..eb68c27b1dc1 100644
--- a/sys/contrib/openzfs/cmd/ztest.c
+++ b/sys/contrib/openzfs/cmd/ztest.c
@@ -6211,13 +6211,14 @@ void
ztest_spa_prop_get_set(ztest_ds_t *zd, uint64_t id)
{
(void) zd, (void) id;
- nvlist_t *props = NULL;
(void) pthread_rwlock_rdlock(&ztest_name_lock);
(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)
dump_nvlist(props, 4);
diff --git a/sys/contrib/openzfs/include/sys/spa.h b/sys/contrib/openzfs/include/sys/spa.h
index 3998f5a6de73..0fa3149e6c6f 100644
--- a/sys/contrib/openzfs/include/sys/spa.h
+++ b/sys/contrib/openzfs/include/sys/spa.h
@@ -1196,9 +1196,9 @@ extern void spa_boot_init(void);
/* properties */
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,
- 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_configfile_set(spa_t *, nvlist_t *, boolean_t);
diff --git a/sys/contrib/openzfs/module/zfs/spa.c b/sys/contrib/openzfs/module/zfs/spa.c
index cafc7196c354..7a3dd29769ca 100644
--- a/sys/contrib/openzfs/module/zfs/spa.c
+++ b/sys/contrib/openzfs/module/zfs/spa.c
@@ -366,21 +366,15 @@ spa_prop_add(spa_t *spa, const char *propname, nvlist_t *outnvl)
int
spa_prop_get_nvlist(spa_t *spa, char **props, unsigned int n_props,
- nvlist_t **outnvl)
+ nvlist_t *outnvl)
{
int err = 0;
if (props == NULL)
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++) {
- err = spa_prop_add(spa, props[i], *outnvl);
+ err = spa_prop_add(spa, props[i], outnvl);
}
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.
*/
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;
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_embedded_log_class(spa));
- spa_prop_add_list(*nvp, ZPOOL_PROP_NAME, spa_name(spa), 0, src);
- spa_prop_add_list(*nvp, ZPOOL_PROP_SIZE, NULL, size, src);
- spa_prop_add_list(*nvp, ZPOOL_PROP_ALLOCATED, NULL, alloc, src);
- spa_prop_add_list(*nvp, ZPOOL_PROP_FREE, NULL,
+ spa_prop_add_list(nv, ZPOOL_PROP_NAME, spa_name(spa), 0, src);
+ spa_prop_add_list(nv, ZPOOL_PROP_SIZE, NULL, size, src);
+ spa_prop_add_list(nv, ZPOOL_PROP_ALLOCATED, NULL, alloc, src);
+ spa_prop_add_list(nv, ZPOOL_PROP_FREE, NULL,
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_prop_add_list(*nvp, ZPOOL_PROP_FRAGMENTATION, NULL,
+ spa_prop_add_list(nv, ZPOOL_PROP_FRAGMENTATION, NULL,
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);
- 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);
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);
- spa_prop_add_list(*nvp, ZPOOL_PROP_BCLONEUSED, NULL,
+ spa_prop_add_list(nv, ZPOOL_PROP_BCLONEUSED, NULL,
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);
- spa_prop_add_list(*nvp, ZPOOL_PROP_BCLONERATIO, NULL,
+ spa_prop_add_list(nv, ZPOOL_PROP_BCLONERATIO, NULL,
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);
- spa_prop_add_list(*nvp, ZPOOL_PROP_HEALTH, NULL,
+ spa_prop_add_list(nv, ZPOOL_PROP_HEALTH, NULL,
rvd->vdev_state, src);
version = spa_version(spa);
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);
} else {
- spa_prop_add_list(*nvp, ZPOOL_PROP_VERSION, NULL,
+ spa_prop_add_list(nv, ZPOOL_PROP_VERSION, NULL,
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);
}
@@ -479,62 +473,62 @@ spa_prop_get_config(spa_t *spa, nvlist_t **nvp)
* when opening pools before this version freedir will be 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,
src);
} else {
- spa_prop_add_list(*nvp, ZPOOL_PROP_FREEING,
+ spa_prop_add_list(nv, ZPOOL_PROP_FREEING,
NULL, 0, src);
}
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,
src);
} else {
- spa_prop_add_list(*nvp, ZPOOL_PROP_LEAKED,
+ spa_prop_add_list(nv, ZPOOL_PROP_LEAKED,
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) {
- 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);
}
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);
}
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);
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);
} 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);
}
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);
} 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);
}
if ((dp = list_head(&spa->spa_config_list)) != 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);
} 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);
}
}
@@ -544,19 +538,13 @@ spa_prop_get_config(spa_t *spa, nvlist_t **nvp)
* Get zpool property values.
*/
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;
zap_cursor_t zc;
zap_attribute_t za;
dsl_pool_t *dp;
- int err;
-
- if (*nvp == NULL) {
- err = nvlist_alloc(nvp, NV_UNIQUE_NAME, KM_SLEEP);
- if (err)
- return (err);
- }
+ int err = 0;
dp = spa_get_dsl(spa);
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.
*/
- spa_prop_get_config(spa, nvp);
+ spa_prop_get_config(spa, nv);
/* If no pool property object, no more prop to get. */
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;
}
- spa_prop_add_list(*nvp, prop, strval, intval, src);
+ spa_prop_add_list(nv, prop, strval, intval, src);
if (strval != NULL)
kmem_free(strval, ZFS_MAX_DATASET_NAME_LEN);
@@ -627,10 +615,10 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp)
break;
}
if (prop != ZPOOL_PROP_INVAL) {
- spa_prop_add_list(*nvp, prop, strval, 0, src);
+ spa_prop_add_list(nv, prop, strval, 0, src);
} else {
src = ZPROP_SRC_LOCAL;
- spa_prop_add_user(*nvp, za.za_name, strval,
+ spa_prop_add_user(nv, za.za_name, strval,
src);
}
kmem_free(strval, za.za_num_integers);
@@ -644,11 +632,9 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp)
out:
mutex_exit(&spa->spa_props_lock);
dsl_pool_config_exit(dp, FTAG);
- if (err && err != ENOENT) {
- nvlist_free(*nvp);
- *nvp = NULL;
+
+ if (err && err != ENOENT)
return (err);
- }
return (0);
}
diff --git a/sys/contrib/openzfs/module/zfs/zfs_ioctl.c b/sys/contrib/openzfs/module/zfs/zfs_ioctl.c
index 897335dd4e4f..3e2fb73b11ed 100644
--- a/sys/contrib/openzfs/module/zfs/zfs_ioctl.c
+++ b/sys/contrib/openzfs/module/zfs/zfs_ioctl.c
@@ -3022,7 +3022,6 @@ static const zfs_ioc_key_t zfs_keys_get_props[] = {
static int
zfs_ioc_pool_get_props(const char *pool, nvlist_t *innvl, nvlist_t *outnvl)
{
- nvlist_t *nvp = outnvl;
spa_t *spa;
char **props = NULL;
unsigned int n_props = 0;
@@ -3041,16 +3040,17 @@ zfs_ioc_pool_get_props(const char *pool, nvlist_t *innvl, nvlist_t *outnvl)
*/
mutex_enter(&spa_namespace_lock);
if ((spa = spa_lookup(pool)) != NULL) {
- error = spa_prop_get(spa, &nvp);
+ error = spa_prop_get(spa, outnvl);
if (error == 0 && props != NULL)
error = spa_prop_get_nvlist(spa, props, n_props,
- &nvp);
+ outnvl);
}
mutex_exit(&spa_namespace_lock);
} else {
- error = spa_prop_get(spa, &nvp);
+ error = spa_prop_get(spa, outnvl);
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);
}