Fix zpool set feature@<feature>=disabled

Commit e4010f2 accidentally allows zpool to set pool features to
"disabled"; this should only be allowed at pool creation. This commit
adds additional checks and test coverage to 'zpool set'.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
Closes #7402

diff --git a/lib/libzfs/libzfs_pool.c b/lib/libzfs/libzfs_pool.c
index 8a0931f90150fd766ed477523dae6c7a0dfbc1e0..9659e08b77c85e0abbcfe1b1600321deb29dbfe5 100644 (file)
--- a/lib/libzfs/libzfs_pool.c
+++ b/lib/libzfs/libzfs_pool.c
@@ -492,6 +492,15 @@ zpool_valid_proplist(libzfs_handle_t *hdl, const char *poolname,
                                goto error;
                        }
 
+                       if (!flags.create &&
+                           strcmp(strval, ZFS_FEATURE_DISABLED) == 0) {
+                               zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+                                   "property '%s' can only be set to "
+                                   "'disabled' at creation time"), propname);
+                               (void) zfs_error(hdl, EZFS_BADPROP, errbuf);
+                               goto error;
+                       }
+
                        if (nvlist_add_uint64(retprops, propname, 0) != 0) {
                                (void) no_memory(hdl);
                                goto error;

diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run
index 1cf8f91e80bb02a8e0764c15ef9c30496cad7bf9..b5d563d4f0c5db9e314f830c0a1d8e75b7c635d8 100644 (file)
--- a/tests/runfiles/linux.run
+++ b/tests/runfiles/linux.run
@@ -403,7 +403,8 @@ tests = ['zpool_scrub_001_neg', 'zpool_scrub_002_pos', 'zpool_scrub_003_pos',
 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']
+tests = ['zpool_set_001_pos', 'zpool_set_002_neg', 'zpool_set_003_neg',
+    'zpool_set_ashift', 'zpool_set_features']
 tags = ['functional', 'cli_root', 'zpool_set']
 
 [tests/functional/cli_root/zpool_status]

diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_set/Makefile.am b/tests/zfs-tests/tests/functional/cli_root/zpool_set/Makefile.am
index 77471688b6259ea4d13198ebfa3ef4c0e2d38d0a..916e8bb8d20e7af4857cec78f807e9e7ff4ee0aa 100644 (file)
--- a/tests/zfs-tests/tests/functional/cli_root/zpool_set/Makefile.am
+++ b/tests/zfs-tests/tests/functional/cli_root/zpool_set/Makefile.am
@@ -5,4 +5,5 @@ dist_pkgdata_SCRIPTS = \
        zpool_set_001_pos.ksh \
        zpool_set_002_neg.ksh \
        zpool_set_003_neg.ksh \
-       zpool_set_ashift.ksh
+       zpool_set_ashift.ksh \
+       zpool_set_features.ksh

diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_ashift.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_ashift.ksh
index 4a192698ad074befd466a5dc3e53a572b5b837e2..3e7ef33456dde0d95c41ada8f3e9e3042b569f3a 100755 (executable)
--- a/tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_ashift.ksh
+++ b/tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_ashift.ksh
@@ -41,8 +41,8 @@ verify_runnable "global"
 
 function cleanup
 {
-       poolexists $TESTPOOL && destroy_pool $TESTPOOL
-       log_must rm -f $disk
+       destroy_pool $TESTPOOL1
+       rm -f $disk
 }
 
 typeset goodvals=("0" "9" "10" "11" "12" "13" "14" "15" "16")
@@ -54,12 +54,12 @@ log_assert "zpool set can modify 'ashift' property"
 
 disk=$TEST_BASE_DIR/$FILEDISK0
 log_must mkfile $SIZE $disk
-log_must zpool create $TESTPOOL $disk
+log_must zpool create $TESTPOOL1 $disk
 
 for ashift in ${goodvals[@]}
 do
-       log_must zpool set ashift=$ashift $TESTPOOL
-       typeset value=$(get_pool_prop ashift $TESTPOOL)
+       log_must zpool set ashift=$ashift $TESTPOOL1
+       typeset value=$(get_pool_prop ashift $TESTPOOL1)
        if [[ "$ashift" != "$value" ]]; then
                log_fail "'zpool set' did not update ashift value to $ashift "\
                    "(current = $value)"
@@ -68,8 +68,8 @@ done
 
 for ashift in ${badvals[@]}
 do
-       log_mustnot zpool set ashift=$ashift $TESTPOOL
-       typeset value=$(get_pool_prop ashift $TESTPOOL)
+       log_mustnot zpool set ashift=$ashift $TESTPOOL1
+       typeset value=$(get_pool_prop ashift $TESTPOOL1)
        if [[ "$ashift" == "$value" ]]; then
                log_fail "'zpool set' incorrectly set ashift value to $value"
        fi

diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_features.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_features.ksh
new file mode 100755 (executable)
index 0000000..f496a0e
--- /dev/null
+++ b/tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_features.ksh
@@ -0,0 +1,75 @@
+#!/bin/ksh -p
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright 2018, loli10K <ezomori.nozomu@gmail.com>. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+
+#
+# DESCRIPTION:
+# 'zpool set' should be able to enable features on pools
+#
+# STRATEGY:
+# 1. Create a pool with all features disabled
+# 2. Verify 'zpool set' is able to enable a single feature
+# 3. Create a pool with all features enabled
+# 4. Verify 'zpool set' is *not* able to disable a single feature
+# 5. Rinse and repeat for known features
+#
+
+verify_runnable "both"
+
+function cleanup
+{
+       destroy_pool $TESTPOOL1
+       rm -f $FILEVDEV
+}
+
+log_assert "'zpool set' should be able to enable features on pools"
+log_onexit cleanup
+
+typeset -a features=(
+    "async_destroy"
+    "large_blocks"
+    "hole_birth"
+    "large_dnode"
+    "userobj_accounting"
+    "encryption"
+)
+FILEVDEV="$TEST_BASE_DIR/zpool_set_features.$$.dat"
+
+# Verify 'zpool set' is able to enable a single feature
+for feature in "${features[@]}"
+do
+       propname="feature@$feature"
+       truncate -s $SPA_MINDEVSIZE $FILEVDEV
+       log_must zpool create -d -f $TESTPOOL1 $FILEVDEV
+       log_must zpool set "$propname=enabled" $TESTPOOL1
+       propval="$(get_pool_prop $propname $TESTPOOL1)"
+       log_must test "$propval" ==  'enabled' -o "$propval" ==  'active'
+       cleanup
+done
+# Verify 'zpool set' is *not* able to disable a single feature
+for feature in "${features[@]}"
+do
+       propname="feature@$feature"
+       truncate -s $SPA_MINDEVSIZE $FILEVDEV
+       log_must zpool create -f $TESTPOOL1 $FILEVDEV
+       log_mustnot zpool set "$propname=disabled" $TESTPOOL1
+       propval="$(get_pool_prop $propname $TESTPOOL1)"
+       log_must test "$propval" ==  'enabled' -o "$propval" ==  'active'
+       cleanup
+done
+
+log_pass "'zpool set' can enable features on pools"