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

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;

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]

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

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
+	destroy_pool $TESTPOOL1
-	log_must rm -f $disk
+	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
+	log_must zpool set ashift=$ashift $TESTPOOL1
-	typeset value=$(get_pool_prop ashift $TESTPOOL)
+	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
+	log_mustnot zpool set ashift=$ashift $TESTPOOL1
-	typeset value=$(get_pool_prop ashift $TESTPOOL)
+	typeset value=$(get_pool_prop ashift $TESTPOOL1)
 	if [[ "$ashift" == "$value" ]]; then
 		log_fail "'zpool set' incorrectly set ashift value to $value"
 	fi

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"