Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 25 Apr 2023 15:25:55 GMT
From:      Mateusz Piotrowski <0mp@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: c98ecfceb361 - main - Add support for zpool user properties
Message-ID:  <202304251525.33PFPtTs029807@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by 0mp:

URL: https://cgit.FreeBSD.org/src/commit/?id=c98ecfceb36159e52c8d7e5cfc0d376393a6a60b

commit c98ecfceb36159e52c8d7e5cfc0d376393a6a60b
Author:     Allan Jude <allanjude@FreeBSD.org>
AuthorDate: 2023-04-21 17:20:36 +0000
Commit:     Mateusz Piotrowski <0mp@FreeBSD.org>
CommitDate: 2023-04-25 15:23:07 +0000

    Add support for zpool user properties
    
    Usage:
    
        zpool set org.freebsd:comment="this is my pool" poolname
    
    Tests are based on zfs_set's user property tests.
    
    Also stop truncating property values at MAXNAMELEN, use ZFS_MAXPROPLEN.
    
    Reviewed by:    markj
    Approved by:    markj
    Co-authored-by: Mateusz Piotrowski <0mp@FreeBSD.org>
    Obtained from:  OpenZFS 8eae2d214cfa Add support for zpool user properties
    Sponsored by:   Beckhoff Automation GmbH & Co. KG.
    Sponsored by:   Klara Inc.
    Differential Revision: https://reviews.freebsd.org/D39657
---
 sys/contrib/openzfs/cmd/zpool/zpool_main.c         |  18 ++-
 sys/contrib/openzfs/include/libzfs.h               |   2 +
 sys/contrib/openzfs/lib/libzfs/libzfs.abi          |   9 ++
 sys/contrib/openzfs/lib/libzfs/libzfs_pool.c       | 101 +++++++++++-
 sys/contrib/openzfs/lib/libzfs/libzfs_util.c       |   1 +
 sys/contrib/openzfs/man/man7/zpoolprops.7          |  55 ++++++-
 sys/contrib/openzfs/module/zfs/spa.c               | 128 ++++++++++-----
 sys/contrib/openzfs/tests/runfiles/common.run      |   3 +-
 .../openzfs/tests/zfs-tests/tests/Makefile.am      |   3 +
 .../cli_root/zpool_set/user_property_001_pos.ksh   |  89 +++++++++++
 .../cli_root/zpool_set/user_property_002_neg.ksh   |  88 ++++++++++
 .../cli_root/zpool_set/zpool_set_common.kshlib     | 178 +++++++++++++++++++++
 12 files changed, 627 insertions(+), 48 deletions(-)

diff --git a/sys/contrib/openzfs/cmd/zpool/zpool_main.c b/sys/contrib/openzfs/cmd/zpool/zpool_main.c
index d79c1608b09f..bf70f68fb431 100644
--- a/sys/contrib/openzfs/cmd/zpool/zpool_main.c
+++ b/sys/contrib/openzfs/cmd/zpool/zpool_main.c
@@ -6069,11 +6069,14 @@ print_pool(zpool_handle_t *zhp, list_cbdata_t *cb)
 		    zpool_prop_get_feature(zhp, pl->pl_user_prop, property,
 		    sizeof (property)) == 0) {
 			propstr = property;
+		} else if (zfs_prop_user(pl->pl_user_prop) &&
+		    zpool_get_userprop(zhp, pl->pl_user_prop, property,
+		    sizeof (property), NULL) == 0) {
+			propstr = property;
 		} else {
 			propstr = "-";
 		}
 
-
 		/*
 		 * If this is being called in scripted mode, or if this is the
 		 * last column and it is left-justified, don't include a width
@@ -10011,7 +10014,7 @@ static int
 get_callback(zpool_handle_t *zhp, void *data)
 {
 	zprop_get_cbdata_t *cbp = (zprop_get_cbdata_t *)data;
-	char value[MAXNAMELEN];
+	char value[ZFS_MAXPROPLEN];
 	zprop_source_t srctype;
 	zprop_list_t *pl;
 	int vid;
@@ -10047,6 +10050,17 @@ get_callback(zpool_handle_t *zhp, void *data)
 				continue;
 
 			if (pl->pl_prop == ZPROP_INVAL &&
+			    zfs_prop_user(pl->pl_user_prop)) {
+				srctype = ZPROP_SRC_LOCAL;
+
+				if (zpool_get_userprop(zhp, pl->pl_user_prop,
+				    value, sizeof (value), &srctype) != 0)
+					continue;
+
+				zprop_print_one_property(zpool_get_name(zhp),
+				    cbp, pl->pl_user_prop, value, srctype,
+				    NULL, NULL);
+			} else if (pl->pl_prop == ZPROP_INVAL &&
 			    (zpool_prop_feature(pl->pl_user_prop) ||
 			    zpool_prop_unsupported(pl->pl_user_prop))) {
 				srctype = ZPROP_SRC_LOCAL;
diff --git a/sys/contrib/openzfs/include/libzfs.h b/sys/contrib/openzfs/include/libzfs.h
index 4f8eeb72ad95..a265f2a19878 100644
--- a/sys/contrib/openzfs/include/libzfs.h
+++ b/sys/contrib/openzfs/include/libzfs.h
@@ -333,6 +333,8 @@ _LIBZFS_H const char *zpool_get_state_str(zpool_handle_t *);
 _LIBZFS_H int zpool_set_prop(zpool_handle_t *, const char *, const char *);
 _LIBZFS_H int zpool_get_prop(zpool_handle_t *, zpool_prop_t, char *,
     size_t proplen, zprop_source_t *, boolean_t literal);
+_LIBZFS_H int zpool_get_userprop(zpool_handle_t *, const char *, char *,
+    size_t proplen, zprop_source_t *);
 _LIBZFS_H uint64_t zpool_get_prop_int(zpool_handle_t *, zpool_prop_t,
     zprop_source_t *);
 _LIBZFS_H int zpool_props_refresh(zpool_handle_t *);
diff --git a/sys/contrib/openzfs/lib/libzfs/libzfs.abi b/sys/contrib/openzfs/lib/libzfs/libzfs.abi
index 99e1b8cdf695..f12424106d39 100644
--- a/sys/contrib/openzfs/lib/libzfs/libzfs.abi
+++ b/sys/contrib/openzfs/lib/libzfs/libzfs.abi
@@ -484,6 +484,7 @@
     <elf-symbol name='zpool_get_state' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='zpool_get_state_str' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='zpool_get_status' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+    <elf-symbol name='zpool_get_userprop' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='zpool_get_vdev_prop' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='zpool_get_vdev_prop_value' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='zpool_history_unpack' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
@@ -6108,6 +6109,14 @@
       <parameter type-id='4c81de99' name='zhp'/>
       <return type-id='80f4b756'/>
     </function-decl>
+    <function-decl name='zpool_get_userprop' mangled-name='zpool_get_userprop' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='zpool_get_userprop'>
+      <parameter type-id='4c81de99' name='zhp'/>
+      <parameter type-id='80f4b756' name='propname'/>
+      <parameter type-id='26a90f95' name='buf'/>
+      <parameter type-id='b59d7dce' name='len'/>
+      <parameter type-id='debc6aa3' name='srctype'/>
+      <return type-id='95e97e5e'/>
+    </function-decl>
     <function-decl name='zpool_set_prop' mangled-name='zpool_set_prop' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='zpool_set_prop'>
       <parameter type-id='4c81de99' name='zhp'/>
       <parameter type-id='80f4b756' name='propname'/>
diff --git a/sys/contrib/openzfs/lib/libzfs/libzfs_pool.c b/sys/contrib/openzfs/lib/libzfs/libzfs_pool.c
index f9b7cc004d6b..6281044723a1 100644
--- a/sys/contrib/openzfs/lib/libzfs/libzfs_pool.c
+++ b/sys/contrib/openzfs/lib/libzfs/libzfs_pool.c
@@ -426,6 +426,37 @@ zpool_get_prop(zpool_handle_t *zhp, zpool_prop_t prop, char *buf,
 	return (0);
 }
 
+/*
+ * Get a zpool property value for 'propname' and return the value in
+ * a pre-allocated buffer.
+ */
+int
+zpool_get_userprop(zpool_handle_t *zhp, const char *propname, char *buf,
+    size_t len, zprop_source_t *srctype)
+{
+	nvlist_t *nv, *nvl;
+	uint64_t ival;
+	const char *value;
+	zprop_source_t source = ZPROP_SRC_LOCAL;
+
+	nvl = zhp->zpool_props;
+	if (nvlist_lookup_nvlist(nvl, propname, &nv) == 0) {
+		if (nvlist_lookup_uint64(nv, ZPROP_SOURCE, &ival) == 0)
+			source = ival;
+		verify(nvlist_lookup_string(nv, ZPROP_VALUE, &value) == 0);
+	} else {
+		source = ZPROP_SRC_DEFAULT;
+		value = "-";
+	}
+
+	if (srctype)
+		*srctype = source;
+
+	(void) strlcpy(buf, value, len);
+
+	return (0);
+}
+
 /*
  * Check if the bootfs name has the same pool name as it is set to.
  * Assuming bootfs is a valid dataset name.
@@ -549,6 +580,44 @@ zpool_valid_proplist(libzfs_handle_t *hdl, const char *poolname,
 				(void) no_memory(hdl);
 				goto error;
 			}
+			continue;
+		} else if (prop == ZPOOL_PROP_INVAL &&
+		    zfs_prop_user(propname)) {
+			/*
+			 * This is a user property: make sure it's a
+			 * string, and that it's less than ZAP_MAXNAMELEN.
+			 */
+			if (nvpair_type(elem) != DATA_TYPE_STRING) {
+				zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+				    "'%s' must be a string"), propname);
+				(void) zfs_error(hdl, EZFS_BADPROP, errbuf);
+				goto error;
+			}
+
+			if (strlen(nvpair_name(elem)) >= ZAP_MAXNAMELEN) {
+				zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+				    "property name '%s' is too long"),
+				    propname);
+				(void) zfs_error(hdl, EZFS_BADPROP, errbuf);
+				goto error;
+			}
+
+			(void) nvpair_value_string(elem, &strval);
+
+			if (strlen(strval) >= ZFS_MAXPROPLEN) {
+				zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+				    "property value '%s' is too long"),
+				    strval);
+				(void) zfs_error(hdl, EZFS_BADPROP, errbuf);
+				goto error;
+			}
+
+			if (nvlist_add_string(retprops, propname,
+			    strval) != 0) {
+				(void) no_memory(hdl);
+				goto error;
+			}
+
 			continue;
 		}
 
@@ -855,9 +924,30 @@ zpool_expand_proplist(zpool_handle_t *zhp, zprop_list_t **plp,
 		features = zpool_get_features(zhp);
 
 	if ((*plp)->pl_all && firstexpand) {
+		/* Handle userprops in the all properties case */
+		if (zhp->zpool_props == NULL && zpool_props_refresh(zhp))
+			return (-1);
+
+		nvp = NULL;
+		while ((nvp = nvlist_next_nvpair(zhp->zpool_props, nvp)) !=
+		    NULL) {
+			const char *propname = nvpair_name(nvp);
+
+			if (!zfs_prop_user(propname))
+				continue;
+
+			entry = zfs_alloc(hdl, sizeof (zprop_list_t));
+			entry->pl_prop = ZPROP_USERPROP;
+			entry->pl_user_prop = zfs_strdup(hdl, propname);
+			entry->pl_width = strlen(entry->pl_user_prop);
+			entry->pl_all = B_TRUE;
+
+			*last = entry;
+			last = &entry->pl_next;
+		}
+
 		for (i = 0; i < SPA_FEATURES; i++) {
-			zprop_list_t *entry = zfs_alloc(hdl,
-			    sizeof (zprop_list_t));
+			entry = zfs_alloc(hdl, sizeof (zprop_list_t));
 			entry->pl_prop = ZPROP_USERPROP;
 			entry->pl_user_prop = zfs_asprintf(hdl, "feature@%s",
 			    spa_feature_table[i].fi_uname);
@@ -874,7 +964,6 @@ zpool_expand_proplist(zpool_handle_t *zhp, zprop_list_t **plp,
 	    nvp != NULL; nvp = nvlist_next_nvpair(features, nvp)) {
 		char *propname;
 		boolean_t found;
-		zprop_list_t *entry;
 
 		if (zfeature_is_supported(nvpair_name(nvp)))
 			continue;
@@ -920,6 +1009,12 @@ zpool_expand_proplist(zpool_handle_t *zhp, zprop_list_t **plp,
 		    NULL, literal) == 0) {
 			if (strlen(buf) > entry->pl_width)
 				entry->pl_width = strlen(buf);
+		} else if (entry->pl_prop == ZPROP_INVAL &&
+		    zfs_prop_user(entry->pl_user_prop) &&
+		    zpool_get_userprop(zhp, entry->pl_user_prop, buf,
+		    sizeof (buf), NULL) == 0) {
+			if (strlen(buf) > entry->pl_width)
+				entry->pl_width = strlen(buf);
 		}
 	}
 
diff --git a/sys/contrib/openzfs/lib/libzfs/libzfs_util.c b/sys/contrib/openzfs/lib/libzfs/libzfs_util.c
index 60695f8a63f4..5592332067f9 100644
--- a/sys/contrib/openzfs/lib/libzfs/libzfs_util.c
+++ b/sys/contrib/openzfs/lib/libzfs/libzfs_util.c
@@ -1774,6 +1774,7 @@ addlist(libzfs_handle_t *hdl, const char *propname, zprop_list_t **listp,
 	 * a user-defined property.
 	 */
 	if (prop == ZPROP_USERPROP && ((type == ZFS_TYPE_POOL &&
+	    !zfs_prop_user(propname) &&
 	    !zpool_prop_feature(propname) &&
 	    !zpool_prop_unsupported(propname)) ||
 	    ((type == ZFS_TYPE_DATASET) && !zfs_prop_user(propname) &&
diff --git a/sys/contrib/openzfs/man/man7/zpoolprops.7 b/sys/contrib/openzfs/man/man7/zpoolprops.7
index 12b9b11903df..7709d85226dc 100644
--- a/sys/contrib/openzfs/man/man7/zpoolprops.7
+++ b/sys/contrib/openzfs/man/man7/zpoolprops.7
@@ -26,8 +26,9 @@
 .\" Copyright 2017 Nexenta Systems, Inc.
 .\" Copyright (c) 2017 Open-E, Inc. All Rights Reserved.
 .\" Copyright (c) 2021, Colm Buckley <colm@tuatha.org>
+.\" Copyright (c) 2023, Klara Inc.
 .\"
-.Dd May 27, 2021
+.Dd April 18, 2023
 .Dt ZPOOLPROPS 7
 .Os
 .
@@ -40,6 +41,12 @@ Each pool has several properties associated with it.
 Some properties are read-only statistics while others are configurable and
 change the behavior of the pool.
 .Pp
+User properties have no effect on ZFS behavior.
+Use them to annotate pools in a way that is meaningful in your environment.
+For more information about user properties, see the
+.Sx User Properties
+section.
+.Pp
 The following are read-only properties:
 .Bl -tag -width "unsupported@guid"
 .It Sy allocated
@@ -431,3 +438,49 @@ backwards compatibility.
 Once feature flags are enabled on a pool this property will no longer have a
 value.
 .El
+.
+.Ss User Properties
+In addition to the standard native properties, ZFS supports arbitrary user
+properties.
+User properties have no effect on ZFS behavior, but applications or
+administrators can use them to annotate pools.
+.Pp
+User property names must contain a colon
+.Pq Qq Sy \&:
+character to distinguish them from native properties.
+They may contain lowercase letters, numbers, and the following punctuation
+characters: colon
+.Pq Qq Sy \&: ,
+dash
+.Pq Qq Sy - ,
+period
+.Pq Qq Sy \&. ,
+and underscore
+.Pq Qq Sy _ .
+The expected convention is that the property name is divided into two portions
+such as
+.Ar module : Ns Ar property ,
+but this namespace is not enforced by ZFS.
+User property names can be at most 256 characters, and cannot begin with a dash
+.Pq Qq Sy - .
+.Pp
+When making programmatic use of user properties, it is strongly suggested to use
+a reversed DNS domain name for the
+.Ar module
+component of property names to reduce the chance that two
+independently-developed packages use the same property name for different
+purposes.
+.Pp
+The values of user properties are arbitrary strings and
+are never validated.
+All of the commands that operate on properties
+.Po Nm zpool Cm list ,
+.Nm zpool Cm get ,
+.Nm zpool Cm set ,
+and so forth
+.Pc
+can be used to manipulate both native properties and user properties.
+Use
+.Nm zpool Cm set Ar name Ns =
+to clear a user property.
+Property values are limited to 8192 bytes.
diff --git a/sys/contrib/openzfs/module/zfs/spa.c b/sys/contrib/openzfs/module/zfs/spa.c
index dc202978c0f6..8c6afb3aac44 100644
--- a/sys/contrib/openzfs/module/zfs/spa.c
+++ b/sys/contrib/openzfs/module/zfs/spa.c
@@ -296,6 +296,22 @@ spa_prop_add_list(nvlist_t *nvl, zpool_prop_t prop, const char *strval,
 	nvlist_free(propval);
 }
 
+/*
+ * Add a user property (source=src, propname=propval) to an nvlist.
+ */
+static void
+spa_prop_add_user(nvlist_t *nvl, const char *propname, char *strval,
+    zprop_source_t src)
+{
+	nvlist_t *propval;
+
+	VERIFY(nvlist_alloc(&propval, NV_UNIQUE_NAME, KM_SLEEP) == 0);
+	VERIFY(nvlist_add_uint64(propval, ZPROP_SOURCE, src) == 0);
+	VERIFY(nvlist_add_string(propval, ZPROP_VALUE, strval) == 0);
+	VERIFY(nvlist_add_nvlist(nvl, propname, propval) == 0);
+	nvlist_free(propval);
+}
+
 /*
  * Get property values from the spa configuration.
  */
@@ -471,7 +487,8 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp)
 		zprop_source_t src = ZPROP_SRC_DEFAULT;
 		zpool_prop_t prop;
 
-		if ((prop = zpool_name_to_prop(za.za_name)) == ZPOOL_PROP_INVAL)
+		if ((prop = zpool_name_to_prop(za.za_name)) ==
+		    ZPOOL_PROP_INVAL && !zfs_prop_user(za.za_name))
 			continue;
 
 		switch (za.za_integer_length) {
@@ -514,7 +531,13 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp)
 				kmem_free(strval, za.za_num_integers);
 				break;
 			}
-			spa_prop_add_list(*nvp, prop, strval, 0, src);
+			if (prop != ZPOOL_PROP_INVAL) {
+				spa_prop_add_list(*nvp, prop, strval, 0, src);
+			} else {
+				src = ZPROP_SRC_LOCAL;
+				spa_prop_add_user(*nvp, za.za_name, strval,
+				    src);
+			}
 			kmem_free(strval, za.za_num_integers);
 			break;
 
@@ -556,36 +579,47 @@ spa_prop_validate(spa_t *spa, nvlist_t *props)
 
 		switch (prop) {
 		case ZPOOL_PROP_INVAL:
-			if (!zpool_prop_feature(propname)) {
-				error = SET_ERROR(EINVAL);
-				break;
-			}
-
 			/*
 			 * Sanitize the input.
 			 */
-			if (nvpair_type(elem) != DATA_TYPE_UINT64) {
-				error = SET_ERROR(EINVAL);
-				break;
-			}
+			if (zfs_prop_user(propname)) {
+				if (strlen(propname) >= ZAP_MAXNAMELEN) {
+					error = SET_ERROR(ENAMETOOLONG);
+					break;
+				}
 
-			if (nvpair_value_uint64(elem, &intval) != 0) {
-				error = SET_ERROR(EINVAL);
-				break;
-			}
+				if (strlen(fnvpair_value_string(elem)) >=
+				    ZAP_MAXVALUELEN) {
+					error = SET_ERROR(E2BIG);
+					break;
+				}
+			} else if (zpool_prop_feature(propname)) {
+				if (nvpair_type(elem) != DATA_TYPE_UINT64) {
+					error = SET_ERROR(EINVAL);
+					break;
+				}
 
-			if (intval != 0) {
-				error = SET_ERROR(EINVAL);
-				break;
-			}
+				if (nvpair_value_uint64(elem, &intval) != 0) {
+					error = SET_ERROR(EINVAL);
+					break;
+				}
+
+				if (intval != 0) {
+					error = SET_ERROR(EINVAL);
+					break;
+				}
+
+				fname = strchr(propname, '@') + 1;
+				if (zfeature_lookup_name(fname, NULL) != 0) {
+					error = SET_ERROR(EINVAL);
+					break;
+				}
 
-			fname = strchr(propname, '@') + 1;
-			if (zfeature_lookup_name(fname, NULL) != 0) {
+				has_feature = B_TRUE;
+			} else {
 				error = SET_ERROR(EINVAL);
 				break;
 			}
-
-			has_feature = B_TRUE;
 			break;
 
 		case ZPOOL_PROP_VERSION:
@@ -792,6 +826,12 @@ spa_prop_set(spa_t *spa, nvlist_t *nvp)
 		    prop == ZPOOL_PROP_READONLY)
 			continue;
 
+		if (prop == ZPOOL_PROP_INVAL &&
+		    zfs_prop_user(nvpair_name(elem))) {
+			need_sync = B_TRUE;
+			break;
+		}
+
 		if (prop == ZPOOL_PROP_VERSION || prop == ZPOOL_PROP_INVAL) {
 			uint64_t ver = 0;
 
@@ -8788,24 +8828,11 @@ spa_sync_props(void *arg, dmu_tx_t *tx)
 		const char *strval, *fname;
 		zpool_prop_t prop;
 		const char *propname;
+		const char *elemname = nvpair_name(elem);
 		zprop_type_t proptype;
 		spa_feature_t fid;
 
-		switch (prop = zpool_name_to_prop(nvpair_name(elem))) {
-		case ZPOOL_PROP_INVAL:
-			/*
-			 * We checked this earlier in spa_prop_validate().
-			 */
-			ASSERT(zpool_prop_feature(nvpair_name(elem)));
-
-			fname = strchr(nvpair_name(elem), '@') + 1;
-			VERIFY0(zfeature_lookup_name(fname, &fid));
-
-			spa_feature_enable(spa, fid, tx);
-			spa_history_log_internal(spa, "set", tx,
-			    "%s=enabled", nvpair_name(elem));
-			break;
-
+		switch (prop = zpool_name_to_prop(elemname)) {
 		case ZPOOL_PROP_VERSION:
 			intval = fnvpair_value_uint64(elem);
 			/*
@@ -8848,7 +8875,7 @@ 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);
+			    "%s=%s", elemname, strval);
 			break;
 		case ZPOOL_PROP_COMPATIBILITY:
 			strval = fnvpair_value_string(elem);
@@ -8867,6 +8894,20 @@ spa_sync_props(void *arg, dmu_tx_t *tx)
 			    "%s=%s", nvpair_name(elem), strval);
 			break;
 
+		case ZPOOL_PROP_INVAL:
+			if (zpool_prop_feature(elemname)) {
+				fname = strchr(elemname, '@') + 1;
+				VERIFY0(zfeature_lookup_name(fname, &fid));
+
+				spa_feature_enable(spa, fid, tx);
+				spa_history_log_internal(spa, "set", tx,
+				    "%s=enabled", elemname);
+				break;
+			} else if (!zfs_prop_user(elemname)) {
+				ASSERT(zpool_prop_feature(elemname));
+				break;
+			}
+			zfs_fallthrough;
 		default:
 			/*
 			 * Set pool property values in the poolprops mos object.
@@ -8881,6 +8922,11 @@ spa_sync_props(void *arg, dmu_tx_t *tx)
 			/* normalize the property name */
 			propname = zpool_prop_to_name(prop);
 			proptype = zpool_prop_get_type(prop);
+			if (prop == ZPOOL_PROP_INVAL &&
+			    zfs_prop_user(elemname)) {
+				propname = elemname;
+				proptype = PROP_TYPE_STRING;
+			}
 
 			if (nvpair_type(elem) == DATA_TYPE_STRING) {
 				ASSERT(proptype == PROP_TYPE_STRING);
@@ -8889,7 +8935,7 @@ spa_sync_props(void *arg, dmu_tx_t *tx)
 				    spa->spa_pool_props_object, propname,
 				    1, strlen(strval) + 1, strval, tx));
 				spa_history_log_internal(spa, "set", tx,
-				    "%s=%s", nvpair_name(elem), strval);
+				    "%s=%s", elemname, strval);
 			} else if (nvpair_type(elem) == DATA_TYPE_UINT64) {
 				intval = fnvpair_value_uint64(elem);
 
@@ -8902,7 +8948,7 @@ spa_sync_props(void *arg, dmu_tx_t *tx)
 				    spa->spa_pool_props_object, propname,
 				    8, 1, &intval, tx));
 				spa_history_log_internal(spa, "set", tx,
-				    "%s=%lld", nvpair_name(elem),
+				    "%s=%lld", elemname,
 				    (longlong_t)intval);
 
 				switch (prop) {
diff --git a/sys/contrib/openzfs/tests/runfiles/common.run b/sys/contrib/openzfs/tests/runfiles/common.run
index 50a9309acea5..bb0aacae456c 100644
--- a/sys/contrib/openzfs/tests/runfiles/common.run
+++ b/sys/contrib/openzfs/tests/runfiles/common.run
@@ -481,7 +481,8 @@ tags = ['functional', 'cli_root', 'zpool_scrub']
 
 [tests/functional/cli_root/zpool_set]
 tests = ['zpool_set_001_pos', 'zpool_set_002_neg', 'zpool_set_003_neg',
-    'zpool_set_ashift', 'zpool_set_features']
+    'zpool_set_ashift', 'zpool_set_features',
+    'user_property_001_pos', 'user_property_002_neg']
 tags = ['functional', 'cli_root', 'zpool_set']
 
 [tests/functional/cli_root/zpool_split]
diff --git a/sys/contrib/openzfs/tests/zfs-tests/tests/Makefile.am b/sys/contrib/openzfs/tests/zfs-tests/tests/Makefile.am
index 92d62b503f65..9b46beaf460f 100644
--- a/sys/contrib/openzfs/tests/zfs-tests/tests/Makefile.am
+++ b/sys/contrib/openzfs/tests/zfs-tests/tests/Makefile.am
@@ -1146,10 +1146,13 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
 	functional/cli_root/zpool_set/cleanup.ksh \
 	functional/cli_root/zpool_set/setup.ksh \
 	functional/cli_root/zpool/setup.ksh \
+	functional/cli_root/zpool_set/zpool_set_common.kshlib \
 	functional/cli_root/zpool_set/zpool_set_001_pos.ksh \
 	functional/cli_root/zpool_set/zpool_set_002_neg.ksh \
 	functional/cli_root/zpool_set/zpool_set_003_neg.ksh \
 	functional/cli_root/zpool_set/zpool_set_ashift.ksh \
+	functional/cli_root/zpool_set/user_property_001_pos.ksh \
+	functional/cli_root/zpool_set/user_property_002_neg.ksh \
 	functional/cli_root/zpool_set/zpool_set_features.ksh \
 	functional/cli_root/zpool_split/cleanup.ksh \
 	functional/cli_root/zpool_split/setup.ksh \
diff --git a/sys/contrib/openzfs/tests/zfs-tests/tests/functional/cli_root/zpool_set/user_property_001_pos.ksh b/sys/contrib/openzfs/tests/zfs-tests/tests/functional/cli_root/zpool_set/user_property_001_pos.ksh
new file mode 100755
index 000000000000..4b9097933f37
--- /dev/null
+++ b/sys/contrib/openzfs/tests/zfs-tests/tests/functional/cli_root/zpool_set/user_property_001_pos.ksh
@@ -0,0 +1,89 @@
+#!/bin/ksh -p
+#
+# CDDL HEADER START
+#
+# The contents of this file are subject to the terms of the
+# Common Development and Distribution License (the "License").
+# You may not use this file except in compliance with the License.
+#
+# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+# or https://opensource.org/licenses/CDDL-1.0.
+# See the License for the specific language governing permissions
+# and limitations under the License.
+#
+# When distributing Covered Code, include this CDDL HEADER in each
+# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+# If applicable, add the following below this CDDL HEADER, with the
+# fields enclosed by brackets "[]" replaced with your own identifying
+# information: Portions Copyright [yyyy] [name of copyright owner]
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright 2007 Sun Microsystems, Inc.  All rights reserved.
+# Use is subject to license terms.
+#
+
+#
+# Copyright (c) 2016 by Delphix. All rights reserved.
+# Copyright (c) 2023 by Klara Inc.
+#
+
+. $STF_SUITE/tests/functional/cli_root/zpool_set/zpool_set_common.kshlib
+
+#
+# DESCRIPTION:
+#	ZFS can set any valid user-defined pool property.
+#
+# STRATEGY:
+#	1. Combine all kind of valid characters into a valid user-defined
+#	   property name.
+#	2. Random get a string as the value.
+#	3. Verify all the valid user-defined pool properties can be set to a
+#	   pool.
+#
+
+verify_runnable "both"
+
+log_assert "ZFS can set any valid user-defined pool property."
+log_onexit cleanup_user_prop $TESTPOOL
+
+typeset -a names=()
+typeset -a values=()
+
+# Longest property name (255 bytes, which is the 256-byte limit minus 1 byte
+# for the null byte)
+names+=("$(awk 'BEGIN { printf "x:"; while (c++ < (256 - 2 - 1)) printf "a" }')")
+values+=("long-property-name")
+# Longest property value (the limits are 1024 on FreeBSD and 4096 on Linux, so
+# pick the right one; the longest value can use limit minus 1 bytes for the
+# null byte)
+if is_linux; then
+	typeset ZFS_MAXPROPLEN=4096
+else
+	typeset ZFS_MAXPROPLEN=1024
+fi
+names+=("long:property:value")
+values+=("$(awk -v max="$ZFS_MAXPROPLEN" 'BEGIN { while (c++ < (max - 1)) printf "A" }')")
+# Valid property names
+for i in {1..10}; do
+	typeset -i len
+	((len = RANDOM % 32))
+	names+=("$(valid_user_property $len)")
+	((len = RANDOM % 512))
+	values+=("$(user_property_value $len)")
+done
+
+typeset -i i=0
+while ((i < ${#names[@]})); do
+	typeset name="${names[$i]}"
+	typeset value="${values[$i]}"
+
+	log_must eval "zpool set $name='$value' $TESTPOOL"
+	log_must eval "check_user_prop $TESTPOOL $name '$value'"
+
+	((i += 1))
+done
+
+log_pass "ZFS can set any valid user-defined pool property passed."
diff --git a/sys/contrib/openzfs/tests/zfs-tests/tests/functional/cli_root/zpool_set/user_property_002_neg.ksh b/sys/contrib/openzfs/tests/zfs-tests/tests/functional/cli_root/zpool_set/user_property_002_neg.ksh
new file mode 100755
index 000000000000..7c8fcba6e471
--- /dev/null
+++ b/sys/contrib/openzfs/tests/zfs-tests/tests/functional/cli_root/zpool_set/user_property_002_neg.ksh
@@ -0,0 +1,88 @@
+#!/bin/ksh -p
+#
+# CDDL HEADER START
+#
+# The contents of this file are subject to the terms of the
+# Common Development and Distribution License (the "License").
+# You may not use this file except in compliance with the License.
+#
+# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+# or https://opensource.org/licenses/CDDL-1.0.
+# See the License for the specific language governing permissions
+# and limitations under the License.
+#
+# When distributing Covered Code, include this CDDL HEADER in each
+# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+# If applicable, add the following below this CDDL HEADER, with the
+# fields enclosed by brackets "[]" replaced with your own identifying
+# information: Portions Copyright [yyyy] [name of copyright owner]
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright 2007 Sun Microsystems, Inc.  All rights reserved.
+# Use is subject to license terms.
+#
+
+#
+# Copyright (c) 2016 by Delphix. All rights reserved.
+# Copyright (c) 2023 by Klara Inc.
+#
+
+. $STF_SUITE/tests/functional/cli_root/zpool_set/zpool_set_common.kshlib
+
+#
+# DESCRIPTION:
+#	ZFS can handle any invalid user-defined pool property.
+#
+# STRATEGY:
+#	1. Combine all kind of invalid user pool property names.
+#	2. Random get a string as the value.
+#	3. Verify all the invalid user-defined pool properties can not be set
+#	   to the pool.
+#
+
+verify_runnable "both"
+
+log_assert "ZFS can handle any invalid user pool property."
+log_onexit cleanup_user_prop $TESTPOOL
+
+typeset -a names=()
+typeset -a values=()
+
+# Too long property name (256 bytes, which is the 256-byte limit minus 1 byte
+# for the null byte plus 1 byte to reach back over the limit)
+names+=("$(awk 'BEGIN { printf "x:"; while (c++ < (256 - 2 - 1 + 1)) printf "a" }')")
+values+=("too-long-property-name")
+# Too long property value (the limits are 1024 on FreeBSD and 4096 on Linux, so
+# pick the right one; the too long value is, e.g., the limit minus 1 bytes for the
+# null byte plus 1 byte to reach back over the limit)
+if is_linux; then
+	typeset ZFS_MAXPROPLEN=4096
+else
+	typeset ZFS_MAXPROPLEN=1024
+fi
+names+=("too:long:property:value")
+values+=("$(awk -v max="$ZFS_MAXPROPLEN" 'BEGIN { while (c++ < (max - 1 + 1)) printf "A" }')")
+# Invalid property names
+for i in {1..10}; do
+	typeset -i len
+	((len = RANDOM % 32))
+	names+=("$(invalid_user_property $len)")
+	((len = RANDOM % 512))
+	values+=("$(user_property_value $len)")
+done
+
+typeset -i i=0
+while ((i < ${#names[@]})); do
+	typeset name="${names[$i]}"
+	typeset value="${values[$i]}"
+
+	log_mustnot zpool set $name=$value $TESTPOOL
+	log_mustnot check_user_prop $TESTPOOL \"$name\" \"$value\"
+
+	((i += 1))
+done
+
+log_pass "ZFS can handle invalid user pool property passed."
diff --git a/sys/contrib/openzfs/tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_common.kshlib b/sys/contrib/openzfs/tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_common.kshlib
new file mode 100644
index 000000000000..346e4a16b2ad
--- /dev/null
+++ b/sys/contrib/openzfs/tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_common.kshlib
@@ -0,0 +1,178 @@
+#
+# CDDL HEADER START
+#
+# The contents of this file are subject to the terms of the
+# Common Development and Distribution License (the "License").
+# You may not use this file except in compliance with the License.
+#
+# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+# or https://opensource.org/licenses/CDDL-1.0.
+# See the License for the specific language governing permissions
+# and limitations under the License.
+#
+# When distributing Covered Code, include this CDDL HEADER in each
+# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+# If applicable, add the following below this CDDL HEADER, with the
+# fields enclosed by brackets "[]" replaced with your own identifying
+# information: Portions Copyright [yyyy] [name of copyright owner]
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
+# Use is subject to license terms.
+#
+
+#
+# Copyright (c) 2014, 2016 by Delphix. All rights reserved.
+# Copyright (c) 2023 by Klara Inc.
+#
+
+. $STF_SUITE/include/libtest.shlib
+
+set -A VALID_NAME_CHAR a b c d e f g h i j k l m n o p q r s t u v w x y z \
+    0 1 2 3 4 5 6 7 8 9 ':' '-' '.' '_'
+set -A INVALID_NAME_CHAR A B C D E F G H I J K L M N O P Q R S T U V W X Y Z \
+    '`' '~' '!' '@' '#' '$' '%' '^' '&' '(' ')' '+' '=' '|' "\\" '{' '[' ']' \
+    '}' ';' '"' '<' ',' '>' '?' '/' ' '
+set -A ALL_CHAR ${VALID_NAME_CHAR[*]} ${INVALID_NAME_CHAR[*]}
+
+#
+# Cleanup all the user properties of the pool.
+#
+# $1 pool name
+#
+function cleanup_user_prop
+{
+	typeset pool=$1
+
+	typeset user_prop
+	user_prop=$(zpool get -H -o property all $pool | grep ":")
+
+	typeset prop
+	for prop in $user_prop; do
+		zpool set $prop="" $pool ||
+			log_must zpool set $prop="" $pool
+	done
+}
+
+#
+# Random select character from the specified character set and combine into a
+# random string
+#
+# $1 character set name
+# $2 String length
+#
+function random_string
+{
+	typeset char_set=${1:-VALID_NAME_CHAR}
+	typeset -i len=${2:-5}
+
+	eval typeset -i count=\${#$char_set[@]}
+
+	# No consumers want an empty string.
+	((len == 0)) && len=3
+
+	typeset str
+	typeset -i i=0
+	while ((i < len)); do
+		typeset -i ind
+		((ind = RANDOM % count))
+		eval str=\${str}\${$char_set[\$ind]}
+
+		((i += 1))
+	done
+
+	echo "$str"
+}
+
+#
+# Get valid user-defined property name
+#
+# $1 user-defined property name length
+#
+function valid_user_property
+{
+	typeset -i sumlen=${1:-10}
+	((sumlen < 2 )) && sumlen=2
+	typeset -i len
+	((len = RANDOM % sumlen))
+	typeset part1 part2
+
+	while true; do
+		part1="$(random_string VALID_NAME_CHAR $len)"
+		if [[ "$part1" == "-"* ]]; then
+			continue
+		fi
+		break
+	done
+	((len = sumlen - (len + 1)))
+
+	while true; do
+		part2="$(random_string VALID_NAME_CHAR $len)"
+		if [[ -z $part1 && -z $part2 ]]; then
+			continue
+		fi
+		break
+	done
+
+	echo "${part1}:${part2}"
+}
+
+#
+# Get invalid user-defined property name
+#
+# $1 user-defined property name length
+#
+function invalid_user_property
+{
+	typeset -i sumlen=${1:-10}
+	((sumlen == 0)) && sumlen=1
+	typeset -i len
+	((len = RANDOM % sumlen))
+
+	typeset part1 part2
+	while true; do
+		part1="$(random_string VALID_NAME_CHAR $len)"
+		((len = sumlen - len))
+		part2="$(random_string INVALID_NAME_CHAR $len)"
+
+		# Avoid $part1 is *:* and $part2 is "=*"
+		if [[ "$part1" == *":"* && "$part2" == "="* ]]; then
+			continue
+		fi
+		break
+	done
+
+	echo "${part1}${part2}"
+}
+
+#
+# Get user-defined property value
+#
+# $1 user-defined property name length
+#
+function user_property_value
+{
+	typeset -i len=${1:-100}
+
+	random_string ALL_CHAR $len
+}
+
+#
+# Check if the user-defined property is identical to the expected value.
+#
+# $1 pool
+# $2 user property
+# $3 expected value
+#
+function check_user_prop
+{
+	typeset pool=$1
+	typeset user_prop="$2"
+	typeset expect_value="$3"
+	typeset value=$(zpool get -p -H -o value "$user_prop" $pool 2>&1)
+
+	[ "$expect_value" = "$value" ]
+}



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202304251525.33PFPtTs029807>