diff --git a/lib/libzfs/libzfs_pool.c b/lib/libzfs/libzfs_pool.c index f256535e8ea0..64f9d1f6eb49 100644 --- a/lib/libzfs/libzfs_pool.c +++ b/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) diff --git a/module/zfs/spa.c b/module/zfs/spa.c index b83c982c13fd..c93c7945f192 100644 --- a/module/zfs/spa.c +++ b/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) { diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c index 250590f062ea..85b6ee32158d 100644 --- a/module/zfs/vdev.c +++ b/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; } diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index a69d36df2f98..6abb3b4213bb 100644 --- a/tests/runfiles/common.run +++ b/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] diff --git a/tests/zfs-tests/tests/Makefile.am b/tests/zfs-tests/tests/Makefile.am index 67630cb564ae..588249be45da 100644 --- a/tests/zfs-tests/tests/Makefile.am +++ b/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 \ diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_clear_userprop.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_clear_userprop.ksh new file mode 100755 index 000000000000..d9395ea8a15b --- /dev/null +++ b/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." diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_common.kshlib b/tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_common.kshlib index 346e4a16b2ad..e095d315c2b5 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_common.kshlib +++ b/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 }