userprops: make clearing pool & vdev userprops work the same

lib/libzfs/libzfs_pool.c

@@ -5342,7 +5342,8 @@ zpool_get_vdev_prop_value(nvlist_t *nvprop, vdev_prop_t prop, char *prop_name,
 			strval = fnvlist_lookup_string(nv, ZPROP_VALUE);
 		} else {
 			/* user prop not found */
-			return (-1);
+			src = ZPROP_SRC_DEFAULT;
+			strval = "-";
 		}
 		(void) strlcpy(buf, strval, len);
 		if (srctype)

module/zfs/spa.c

@@ -9683,9 +9683,17 @@ spa_sync_props(void *arg, dmu_tx_t *tx)
 			if (nvpair_type(elem) == DATA_TYPE_STRING) {
 				ASSERT(proptype == PROP_TYPE_STRING);
 				strval = fnvpair_value_string(elem);
-				VERIFY0(zap_update(mos,
-				    spa->spa_pool_props_object, propname,
-				    1, strlen(strval) + 1, strval, tx));
+				if (strlen(strval) == 0) {
+					/* remove the property if value == "" */
+					(void) zap_remove(mos,
+					    spa->spa_pool_props_object,
+					    propname, tx);
+				} else {
+					VERIFY0(zap_update(mos,
+					    spa->spa_pool_props_object,
+					    propname, 1, strlen(strval) + 1,
+					    strval, tx));
+				}
 				spa_history_log_internal(spa, "set", tx,
 				    "%s=%s", elemname, strval);
 			} else if (nvpair_type(elem) == DATA_TYPE_UINT64) {

module/zfs/vdev.c

@@ -5969,7 +5969,7 @@ vdev_prop_set(vdev_t *vd, nvlist_t *innvl, nvlist_t *outnvl)
 			goto end;
 		}
 
-		if (vdev_prop_readonly(prop)) {
+		if (prop != VDEV_PROP_USERPROP && vdev_prop_readonly(prop)) {
 			error = EROFS;
 			goto end;
 		}

tests/runfiles/common.run

@@ -544,7 +544,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', 'vdev_set_001_pos',
-    'user_property_001_pos', 'user_property_002_neg']
+    'user_property_001_pos', 'user_property_002_neg',
+    'zpool_set_clear_userprop']
 tags = ['functional', 'cli_root', 'zpool_set']
 
 [tests/functional/cli_root/zpool_split]

tests/zfs-tests/tests/Makefile.am

@@ -1243,6 +1243,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
 	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_set/zpool_set_clear_userprop.ksh \
 	functional/cli_root/zpool_split/cleanup.ksh \
 	functional/cli_root/zpool_split/setup.ksh \
 	functional/cli_root/zpool_split/zpool_split_cliargs.ksh \

tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_clear_userprop.ksh

@@ -0,0 +1,44 @@
+#!/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 (c) 2024, Klara, Inc.
+#
+
+. $STF_SUITE/tests/functional/cli_root/zpool_set/zpool_set_common.kshlib
+
+verify_runnable "both"
+
+log_assert "Setting a user-defined property to the empty string removes it."
+log_onexit cleanup_user_prop $TESTPOOL
+
+log_must zpool set cool:pool=hello $TESTPOOL
+log_must check_user_prop $TESTPOOL cool:pool hello local
+log_must zpool set cool:pool= $TESTPOOL
+log_must check_user_prop $TESTPOOL cool:pool '-' default
+
+log_must zpool set cool:vdev=goodbye $TESTPOOL root
+log_must check_vdev_user_prop $TESTPOOL root cool:vdev goodbye local
+log_must zpool set cool:vdev= $TESTPOOL root
+log_must check_vdev_user_prop $TESTPOOL root cool:vdev '-' default
+
+log_pass "Setting a user-defined property to the empty string removes it."

tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_common.kshlib

@@ -160,19 +160,55 @@ function user_property_value
 	random_string ALL_CHAR $len
 }
 
+function _check_user_prop
+{
+	typeset pool="$1"
+	typeset vdev="$2"
+	typeset user_prop="$3"
+	typeset expect_value="$4"
+	typeset expect_source="$5"
+
+	typeset -a \
+	    v=($(zpool get -p -H -o value,source "$user_prop" $pool $vdev 2>&1))
+
+	[[ "$expect_value" == "${v[0]}" && \
+	    -z "$expect_source" || "$expect_source" == "${v[1]}" ]]
+}
+
 #
 # Check if the user-defined property is identical to the expected value.
 #
 # $1 pool
 # $2 user property
 # $3 expected value
+# $4 expected source (optional)
 #
 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)
+	typeset expect_source="${4:-}"
+
+	_check_user_prop $pool '' $user_prop $expect_value $expect_source
+}
+
+#
+# Check if the user-defined property is identical to the expected value.
+#
+# $1 pool
+# $2 vdev
+# $3 user property
+# $4 expected value
+# $5 expected source (optional)
+#
+function check_vdev_user_prop
+{
+	typeset pool="$1"
+	typeset vdev="$2"
+	typeset user_prop="$3"
+	typeset expect_value="$4"
+	typeset expect_source="${5:-}"
 
-	[ "$expect_value" = "$value" ]
+	_check_user_prop $pool $vdev $user_prop $expect_value $expect_source
 }