More ashift improvements

This commit allow higher ashift values (up to 16) in 'zpool create'

The ashift value was previously limited to 13 (8K block) in b41c990
because the limited number of uberblocks we could fit in the
statically sized (128K) vdev label ring buffer could prevent the
ability the safely roll back a pool to recover it.

Since b02fe35 the largest uberblock size we support is 8K: this
allow us to store a minimum number of 16 uberblocks in the vdev
label, even with higher ashift values.

Additionally change 'ashift' pool property behaviour: if set it will
be used as the default hint value in subsequent vdev operations
('zpool add', 'attach' and 'replace'). A custom ashift value can still
be specified from the command line, if desired.

Finally, fix a bug in add-o_ashift.ksh caused by a missing variable.

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

cmd/zpool/zpool_main.c

@@ -678,6 +678,20 @@ zpool_do_add(int argc, char **argv)
 		return (1);
 	}
 
+	/* unless manually specified use "ashift" pool property (if set) */
+	if (!nvlist_exists(props, ZPOOL_CONFIG_ASHIFT)) {
+		int intval;
+		zprop_source_t src;
+		char strval[ZPOOL_MAXPROPLEN];
+
+		intval = zpool_get_prop_int(zhp, ZPOOL_PROP_ASHIFT, &src);
+		if (src != ZPROP_SRC_DEFAULT) {
+			(void) sprintf(strval, "%" PRId32, intval);
+			verify(add_prop_list(ZPOOL_CONFIG_ASHIFT, strval,
+			    &props, B_TRUE) == 0);
+		}
+	}
+
 	/* pass off to get_vdev_spec for processing */
 	nvroot = make_root_vdev(zhp, props, force, !force, B_FALSE, dryrun,
 	    argc, argv);
@@ -5066,6 +5080,20 @@ zpool_do_attach_or_replace(int argc, char **argv, int replacing)
 		return (1);
 	}
 
+	/* unless manually specified use "ashift" pool property (if set) */
+	if (!nvlist_exists(props, ZPOOL_CONFIG_ASHIFT)) {
+		int intval;
+		zprop_source_t src;
+		char strval[ZPOOL_MAXPROPLEN];
+
+		intval = zpool_get_prop_int(zhp, ZPOOL_PROP_ASHIFT, &src);
+		if (src != ZPROP_SRC_DEFAULT) {
+			(void) sprintf(strval, "%" PRId32, intval);
+			verify(add_prop_list(ZPOOL_CONFIG_ASHIFT, strval,
+			    &props, B_TRUE) == 0);
+		}
+	}
+
 	nvroot = make_root_vdev(zhp, props, force, B_FALSE, replacing, B_FALSE,
 	    argc, argv);
 	if (nvroot == NULL) {

cmd/zpool/zpool_vdev.c

@@ -698,7 +698,11 @@ make_leaf_vdev(nvlist_t *props, const char *arg, uint64_t is_log)
 
 		if (nvlist_lookup_string(props,
 		    zpool_prop_to_name(ZPOOL_PROP_ASHIFT), &value) == 0) {
-			zfs_nicestrtonum(NULL, value, &ashift);
+			if (zfs_nicestrtonum(NULL, value, &ashift) != 0) {
+				(void) fprintf(stderr,
+				    gettext("ashift must be a number.\n"));
+				return (NULL);
+			}
 			if (ashift != 0 &&
 			    (ashift < ASHIFT_MIN || ashift > ASHIFT_MAX)) {
 				(void) fprintf(stderr,

include/sys/spa.h

@@ -126,11 +126,11 @@ _NOTE(CONSTCOND) } while (0)
  * which can only be set at vdev creation time. Physical writes are always done
  * according to it, which makes 2^ashift the smallest possible IO on a vdev.
  *
- * We currently allow values ranging from 512 bytes (2^9 = 512) to 8 KiB
+ * We currently allow values ranging from 512 bytes (2^9 = 512) to 64 KiB
- * (2^13 = 8,192).
+ * (2^16 = 65,536).
  */
 #define	ASHIFT_MIN		9
-#define	ASHIFT_MAX		13
+#define	ASHIFT_MAX		16
 
 /*
  * Size of block to hold the configuration data (a packed nvlist)

lib/libzfs/libzfs_pool.c

@@ -530,14 +530,6 @@ zpool_valid_proplist(libzfs_handle_t *hdl, const char *poolname,
 			break;
 
 		case ZPOOL_PROP_ASHIFT:
-			if (!flags.create) {
-				zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
-				    "property '%s' can only be set at "
-				    "creation time"), propname);
-				(void) zfs_error(hdl, EZFS_BADPROP, errbuf);
-				goto error;
-			}
-
 			if (intval != 0 &&
 			    (intval < ASHIFT_MIN || intval > ASHIFT_MAX)) {
 				zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,

man/man8/zpool.8

@@ -560,23 +560,6 @@ Amount of storage space used within the pool.
 .LP
 The space usage properties report actual physical space available to the storage pool. The physical space can be different from the total amount of space that any contained datasets can actually use. The amount of space used in a \fBraidz\fR configuration depends on the characteristics of the data being written. In addition, \fBZFS\fR reserves some space for internal accounting that the \fBzfs\fR(8) command takes into account, but the \fBzpool\fR command does not. For non-full pools of a reasonable size, these effects should be invisible. For small pools, or pools that are close to being completely full, these discrepancies may become more noticeable.
 
-.sp
-.LP
-The following property can be set at creation time:
-.sp
-.ne 2
-.na
-\fB\fBashift\fR=\fIashift\fR\fR
-.ad
-.sp .6
-.RS 4n
-Pool sector size exponent, to the power of 2 (internally referred to as "ashift"). Values from 9 to 13, inclusive, are valid; also, the special value 0 (the default) means to auto-detect using the kernel's block layer and a ZFS internal exception list. I/O operations will be aligned to the specified size boundaries. Additionally, the minimum (disk) write size will be set to the specified size, so this represents a space vs. performance trade-off. The typical case for setting this property is when performance is important and the underlying disks use 4KiB sectors but report 512B sectors to the OS (for compatibility reasons); in that case, set \fBashift=12\fR (which is 1<<12 = 4096).
-.LP
-For optimal performance, the pool sector size should be greater than or equal to the sector size of the underlying disks. Since the property cannot be changed after pool creation, if in a given pool, you \fIever\fR want to use drives that \fIreport\fR 4KiB sectors, you must set \fBashift=12\fR at pool creation time.
-.LP
-Keep in mind is that the \fBashift\fR is \fIvdev\fR specific and is not a \fIpool\fR global.  This means that when adding new vdevs to an existing pool you may need to specify the \fBashift\fR.
-.RE
-
 .sp
 .LP
 The following property can be set at creation time and import time:
@@ -610,6 +593,18 @@ To write to a read-only pool, a export and import of the pool is required.
 .sp
 .LP
 The following properties can be set at creation time and import time, and later changed with the \fBzpool set\fR command:
+.sp
+.ne 2
+.na
+\fB\fBashift\fR=\fIashift\fR\fR
+.ad
+.sp .6
+.RS 4n
+Pool sector size exponent, to the power of 2 (internally referred to as "ashift"). Values from 9 to 16, inclusive, are valid; also, the special value 0 (the default) means to auto-detect using the kernel's block layer and a ZFS internal exception list. I/O operations will be aligned to the specified size boundaries. Additionally, the minimum (disk) write size will be set to the specified size, so this represents a space vs. performance trade-off. For optimal performance, the pool sector size should be greater than or equal to the sector size of the underlying disks. The typical case for setting this property is when performance is important and the underlying disks use 4KiB sectors but report 512B sectors to the OS (for compatibility reasons); in that case, set \fBashift=12\fR (which is 1<<12 = 4096).
+.LP
+When set, this property is used as the default hint value in \fIsubsequent\fR vdev operations (add, attach and replace). Changing this value will \fInot\fR modify any existing vdev, not even on disk replacement; however it can be used, for instance, to replace a dying 512B sectors disk with a newer 4KiB sectors device: this will probably result in bad performance but at the same time could prevent loss of data.
+.RE
+
 .sp
 .ne 2
 .na
@@ -830,7 +825,7 @@ Display full paths for vdevs instead of only the last component of the path.  Th
 .ad
 .sp .6
 .RS 4n
-Sets the given pool properties. See the "Properties" section for a list of valid properties that can be set. The only property supported at the moment is \fBashift\fR.  \fBDo note\fR that some properties (among them \fBashift\fR) are \fInot\fR inherited from a previous vdev. They are vdev specific, not pool specific.
+Sets the given pool properties. See the "Properties" section for a list of valid properties that can be set. The only property supported at the moment is \fBashift\fR.
 .RE
 
 Do not add a disk that is currently configured as a quorum device to a zpool. After a disk is in the pool, that disk can then be configured as a quorum device.
@@ -860,7 +855,7 @@ Forces use of \fInew_device\fR, even if its appears to be in use. Not all device
 .ad
 .sp .6
 .RS 4n
-Sets the given pool properties. See the "Properties" section for a list of valid properties that can be set. The only property supported at the moment is "ashift".
+Sets the given pool properties. See the "Properties" section for a list of valid properties that can be set. The only property supported at the moment is \fBashift\fR.
 .RE
 
 .RE
@@ -2011,7 +2006,7 @@ Forces use of \fInew_device\fR, even if its appears to be in use. Not all device
 .ad
 .sp .6n
 .RS 6n
-Sets the given pool properties. See the "Properties" section for a list of valid properties that can be set. The only property supported at the moment is \fBashift\fR.  \fBDo note\fR that some properties (among them \fBashift\fR) are \fInot\fR inherited from a previous vdev. They are vdev specific, not pool specific.
+Sets the given pool properties. See the "Properties" section for a list of valid properties that can be set. The only property supported at the moment is \fBashift\fR.
 .RE
 
 .RE

module/zcommon/zpool_prop.c

@@ -99,15 +99,13 @@ zpool_prop_init(void)
 	    PROP_READONLY, ZFS_TYPE_POOL, "<1.00x or higher if deduped>",
 	    "DEDUP");
 
-	/* readonly onetime number properties */
-	zprop_register_number(ZPOOL_PROP_ASHIFT, "ashift", 0, PROP_ONETIME,
-	    ZFS_TYPE_POOL, "<ashift, 9-13, or 0=default>", "ASHIFT");
-
 	/* default number properties */
 	zprop_register_number(ZPOOL_PROP_VERSION, "version", SPA_VERSION,
 	    PROP_DEFAULT, ZFS_TYPE_POOL, "<version>", "VERSION");
 	zprop_register_number(ZPOOL_PROP_DEDUPDITTO, "dedupditto", 0,
 	    PROP_DEFAULT, ZFS_TYPE_POOL, "<threshold (min 100)>", "DEDUPDITTO");
+	zprop_register_number(ZPOOL_PROP_ASHIFT, "ashift", 0, PROP_DEFAULT,
+	    ZFS_TYPE_POOL, "<ashift, 9-16, or 0=default>", "ASHIFT");
 
 	/* default index (boolean) properties */
 	zprop_register_index(ZPOOL_PROP_DELEGATION, "delegation", 1,

tests/runfiles/linux.run

@@ -227,10 +227,10 @@ tests = ['zpool_001_neg', 'zpool_002_pos', 'zpool_003_pos']
 [tests/functional/cli_root/zpool_add]
 tests = ['zpool_add_001_pos', 'zpool_add_002_pos', 'zpool_add_003_pos',
     'zpool_add_007_neg', 'zpool_add_008_neg', 'zpool_add_009_neg',
-    'add-o_ashift']
+    'add-o_ashift', 'add_prop_ashift']
 
 [tests/functional/cli_root/zpool_attach]
-tests = ['zpool_attach_001_neg']
+tests = ['zpool_attach_001_neg', 'attach-o_ashift']
 
 # DISABLED:
 # zpool_clear_001_pos - https://github.com/zfsonlinux/zfs/issues/5634
@@ -258,7 +258,8 @@ tests = [
     'zpool_create_024_pos',
     'zpool_create_features_001_pos', 'zpool_create_features_002_pos',
     'zpool_create_features_003_pos', 'zpool_create_features_004_neg',
-    'zpool_create_features_005_pos']
+    'zpool_create_features_005_pos',
+    'create-o_ashift']
 
 # DISABLED:
 # zpool_destroy_001_pos - needs investigation
@@ -323,7 +324,7 @@ tests = ['zpool_online_001_pos', 'zpool_online_002_neg']
 tests = ['zpool_remove_001_neg', 'zpool_remove_002_pos']
 
 [tests/functional/cli_root/zpool_replace]
-tests = ['zpool_replace_001_neg']
+tests = ['zpool_replace_001_neg', 'replace-o_ashift', 'replace_prop_ashift']
 
 [tests/functional/cli_root/zpool_scrub]
 tests = ['zpool_scrub_001_neg', 'zpool_scrub_002_pos', 'zpool_scrub_003_pos',

tests/zfs-tests/include/libtest.shlib

@@ -1824,6 +1824,29 @@ function snapshot_mountpoint
 	echo $(get_prop mountpoint $fs)/.zfs/snapshot/$snap
 }
 
+#
+# Given a device and 'ashift' value verify it's correctly set on every label
+#
+function verify_ashift # device ashift
+{
+	typeset device="$1"
+	typeset ashift="$2"
+
+	zdb -e -lll $device | awk -v ashift=$ashift '/ashift: / {
+	    if (ashift != $2)
+	        exit 1;
+	    else
+	        count++;
+	    } END {
+	    if (count != 4)
+	        exit 1;
+	    else
+	        exit 0;
+	    }'
+
+	return $?
+}
+
 #
 # Given a pool and file system, this function will verify the file system
 # using the zdb internal tool. Note that the pool is exported and imported
@@ -3219,6 +3242,21 @@ function wait_freeing #pool
 	done
 }
 
+#
+# Wait for every device replace operation to complete
+#
+# $1 pool name
+#
+function wait_replacing #pool
+{
+	typeset pool=${1:-$TESTPOOL}
+	while true; do
+		[[ "" == "$(zpool status $pool |
+		    awk '/replacing-[0-9]+/ {print $1}')" ]] && break
+		log_must sleep 1
+	done
+}
+
 #
 # Check if ZED is currently running, if not start ZED.
 #

tests/zfs-tests/tests/functional/cli_root/zpool_add/Makefile.am

@@ -13,4 +13,5 @@ dist_pkgdata_SCRIPTS = \
 	zpool_add_007_neg.ksh \
 	zpool_add_008_neg.ksh \
 	zpool_add_009_neg.ksh \
-	add-o_ashift.ksh
+	add-o_ashift.ksh \
+	add_prop_ashift.ksh

tests/zfs-tests/tests/functional/cli_root/zpool_add/add-o_ashift.ksh

@@ -34,7 +34,7 @@
 #
 # STRATEGY:
 #	1. Create a pool with default values.
-#	2. Verify 'zpool add -o ashift=<n>' works with allowed values (9-13).
+#	2. Verify 'zpool add -o ashift=<n>' works with allowed values (9-16).
 #	3. Verify 'zpool add -o ashift=<n>' doesn't accept other invalid values.
 #
 
@@ -43,29 +43,7 @@ verify_runnable "global"
 function cleanup
 {
 	poolexists $TESTPOOL && destroy_pool $TESTPOOL
-	log_must rm $disk1 $disk2
+	log_must rm -f $disk1 $disk2
-}
-
-#
-# Verify every label in device $1 contains ashift value $2
-# $1 device
-# $2 ashift value
-#
-function verify_device_ashift
-{
-	typeset device=$1
-	typeset value=$2
-	typeset ashift
-
-	zdb -e -l $device | grep " ashift:" | {
-		while read ashift ; do
-			if [[ "ashift: $value" != "$ashift" ]]; then
-				return 1
-			fi
-		done
-	}
-
-	return 0
 }
 
 log_assert "zpool add -o ashift=<n>' works with different ashift values"
@@ -76,12 +54,12 @@ disk2=$TEST_BASE_DIR/$FILEDISK1
 log_must mkfile $SIZE $disk1
 log_must mkfile $SIZE $disk2
 
-typeset ashifts=("9" "10" "11" "12" "13")
+typeset ashifts=("9" "10" "11" "12" "13" "14" "15" "16")
 for ashift in ${ashifts[@]}
 do
 	log_must zpool create $TESTPOOL $disk1
 	log_must zpool add -o ashift=$ashift $TESTPOOL $disk2
-	verify_device_ashift $disk2 $ashift
+	verify_ashift $disk2 $ashift
 	if [[ $? -ne 0 ]]
 	then
 		log_fail "Device was added without setting ashift value to "\
@@ -93,11 +71,12 @@ do
 	log_must zpool labelclear $disk2
 done
 
-typeset badvals=("off" "on" "1" "8" "14" "1b" "ff" "-")
+typeset badvals=("off" "on" "1" "8" "17" "1b" "ff" "-")
 for badval in ${badvals[@]}
 do
 	log_must zpool create $TESTPOOL $disk1
-	log_mustnot zpool add -o ashift="$badval" $disk2
+	log_mustnot zpool add $TESTPOOL -o ashift="$badval" $disk2
+	# clean things for the next run
 	log_must zpool destroy $TESTPOOL
 	log_must zpool labelclear $disk1
 	log_mustnot zpool labelclear $disk2

tests/zfs-tests/tests/functional/cli_root/zpool_add/add_prop_ashift.ksh

@@ -0,0 +1,94 @@
+#!/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 http://www.opensolaris.org/os/licensing.
+# 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 2017, loli10K. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/cli_root/zpool_create/zpool_create.shlib
+
+#
+# DESCRIPTION:
+#	'zpool add' should use the ashift pool property value as default.
+#
+# STRATEGY:
+#	1. Create a pool with default values.
+#	2. Verify 'zpool add' uses the ashift pool property value when adding
+#	   a new device.
+#	3. Verify the default ashift value can still be overridden by manually
+#	   specifying '-o ashift=<n>' from the command line.
+#
+
+verify_runnable "global"
+
+function cleanup
+{
+	poolexists $TESTPOOL && destroy_pool $TESTPOOL
+	log_must rm -f $disk1 $disk2
+}
+
+log_assert "'zpool add' uses the ashift pool property value as default."
+log_onexit cleanup
+
+disk1=$TEST_BASE_DIR/$FILEDISK0
+disk2=$TEST_BASE_DIR/$FILEDISK1
+log_must mkfile $SIZE $disk1
+log_must mkfile $SIZE $disk2
+
+typeset ashifts=("9" "10" "11" "12" "13" "14" "15" "16")
+for ashift in ${ashifts[@]}
+do
+	log_must zpool create -o ashift=$ashift $TESTPOOL $disk1
+	log_must zpool add $TESTPOOL $disk2
+	verify_ashift $disk2 $ashift
+	if [[ $? -ne 0 ]]
+	then
+		log_fail "Device was added without setting ashift value to "\
+		    "$ashift"
+	fi
+	# clean things for the next run
+	log_must zpool destroy $TESTPOOL
+	log_must zpool labelclear $disk1
+	log_must zpool labelclear $disk2
+done
+
+for ashift in ${ashifts[@]}
+do
+	for cmdval in ${ashifts[@]}
+	do
+		log_must zpool create -o ashift=$ashift $TESTPOOL $disk1
+		log_must zpool add $TESTPOOL -o ashift=$cmdval $disk2
+		verify_ashift $disk2 $cmdval
+		if [[ $? -ne 0 ]]
+		then
+			log_fail "Device was added without setting ashift " \
+			    "value to $cmdval"
+		fi
+		# clean things for the next run
+		log_must zpool destroy $TESTPOOL
+		log_must zpool labelclear $disk1
+		log_must zpool labelclear $disk2
+	done
+done
+
+log_pass "'zpool add' uses the ashift pool property value."

tests/zfs-tests/tests/functional/cli_root/zpool_attach/Makefile.am

@@ -2,4 +2,5 @@ pkgdatadir = $(datadir)/@PACKAGE@/zfs-tests/tests/functional/cli_root/zpool_atta
 dist_pkgdata_SCRIPTS = \
 	setup.ksh \
 	cleanup.ksh \
-	zpool_attach_001_neg.ksh
+	zpool_attach_001_neg.ksh \
+	attach-o_ashift.ksh

tests/zfs-tests/tests/functional/cli_root/zpool_attach/attach-o_ashift.ksh

@@ -0,0 +1,107 @@
+#!/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 http://www.opensolaris.org/os/licensing.
+# 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 2017, loli10K. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/cli_root/zpool_create/zpool_create.shlib
+
+#
+# DESCRIPTION:
+#	'zpool attach -o ashift=<n> ...' should work with different ashift
+#	values.
+#
+# STRATEGY:
+#	1. Create various pools with different ashift values.
+#	2. Verify 'attach -o ashift=<n>' works only with allowed values.
+#
+
+verify_runnable "global"
+
+function cleanup
+{
+	poolexists $TESTPOOL1 && destroy_pool $TESTPOOL1
+	log_must rm -f $disk1
+	log_must rm -f $disk2
+}
+
+log_assert "zpool attach -o ashift=<n>' works with different ashift values"
+log_onexit cleanup
+
+disk1=$TEST_BASE_DIR/$FILEDISK0
+disk2=$TEST_BASE_DIR/$FILEDISK1
+log_must mkfile $SIZE $disk1
+log_must mkfile $SIZE $disk2
+
+typeset ashifts=("9" "10" "11" "12" "13" "14" "15" "16")
+for ashift in ${ashifts[@]}
+do
+	for cmdval in ${ashifts[@]}
+	do
+		log_must zpool create -o ashift=$ashift $TESTPOOL1 $disk1
+		verify_ashift $disk1 $ashift
+		if [[ $? -ne 0 ]]
+		then
+			log_fail "Pool was created without setting ashift " \
+			    "value to $ashift"
+		fi
+		# ashift_of(attached_disk) <= ashift_of(existing_vdev)
+		if [[ $cmdval -le $ashift ]]
+		then
+			log_must zpool attach -o ashift=$cmdval $TESTPOOL1 \
+			    $disk1 $disk2
+			verify_ashift $disk2 $ashift
+			if [[ $? -ne 0 ]]
+			then
+				log_fail "Device was attached without " \
+				    "setting ashift value to $ashift"
+			fi
+		else
+			log_mustnot zpool attach -o ashift=$cmdval $TESTPOOL1 \
+			    $disk1 $disk2
+		fi
+		# clean things for the next run
+		log_must zpool destroy $TESTPOOL1
+		log_must zpool labelclear $disk1
+		# depending on if we expect to have failed the 'zpool attach'
+		if [[ $cmdval -le $ashift ]]
+		then
+			log_must zpool labelclear $disk2
+		else
+			log_mustnot zpool labelclear $disk2
+		fi
+	done
+done
+
+typeset badvals=("off" "on" "1" "8" "17" "1b" "ff" "-")
+for badval in ${badvals[@]}
+do
+	log_must zpool create $TESTPOOL1 $disk1
+	log_mustnot zpool attach $TESTPOOL1 -o ashift=$badval $disk1 $disk2
+	log_must zpool destroy $TESTPOOL1
+	log_must zpool labelclear $disk1
+	log_mustnot zpool labelclear $disk2
+done
+
+log_pass "zpool attach -o ashift=<n>' works with different ashift values"

tests/zfs-tests/tests/functional/cli_root/zpool_create/Makefile.am

@@ -31,4 +31,5 @@ dist_pkgdata_SCRIPTS = \
 	zpool_create_features_002_pos.ksh \
 	zpool_create_features_003_pos.ksh \
 	zpool_create_features_004_neg.ksh \
-	zpool_create_features_005_pos.ksh
+	zpool_create_features_005_pos.ksh \
+	create-o_ashift.ksh

tests/zfs-tests/tests/functional/cli_root/zpool_create/create-o_ashift.ksh

@@ -0,0 +1,140 @@
+#!/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 http://www.opensolaris.org/os/licensing.
+# 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 2016, loli10K. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/cli_root/zpool_create/zpool_create.shlib
+
+#
+# DESCRIPTION:
+#	'zpool create -o ashift=<n> ...' should work with different ashift
+#	values.
+#
+# STRATEGY:
+#	1. Create various pools with different ashift values.
+#	2. Verify -o ashift=<n> works only with allowed values (9-16).
+#	   Also verify that the lowest number of uberblocks in a label is 16 and
+#	   smallest uberblock size is 8K even with higher ashift values.
+#
+
+verify_runnable "global"
+
+function cleanup
+{
+	poolexists $TESTPOOL && destroy_pool $TESTPOOL
+	log_must rm -f $disk
+}
+
+#
+# FIXME: Ugly hack to force a $1 number of txg syncs.
+# This is used on Linux because we can't force it via sync(1) like on Illumos.
+# This could eventually (an hopefully) be replaced by a 'zpool sync' command.
+# $1 pool name
+# $2 number of txg syncs
+#
+function txg_sync
+{
+	typeset pool=$1
+	typeset count=$2
+	typeset -i i=0;
+
+	while [ $i -lt $count ]
+	do
+		zfs snapshot $pool@sync$$$i
+		if [[ $? -ne 0 ]]
+		then
+			log_fail "Failed to sync pool $pool (snapshot $i)"
+			return 1
+		fi
+		((i = i + 1))
+	done
+
+	return 0
+}
+
+#
+# Verify device $1 labels contains $2 valid uberblocks in every label
+# $1 device
+# $2 uberblocks count
+#
+function verify_device_uberblocks
+{
+	typeset device=$1
+	typeset ubcount=$2
+
+	zdb -quuul $device | egrep '^(\s+)?Uberblock' |
+	    egrep -v 'invalid$' | awk \
+	    -v ubcount=$ubcount '{ uberblocks[$0]++; }
+	    END { for (i in uberblocks) {
+		count++;
+		if (uberblocks[i] != 4) { exit 1; }
+	    }
+	    if (count != ubcount) { exit 1; } }'
+
+	return $?
+}
+
+log_assert "zpool create -o ashift=<n>' works with different ashift values"
+log_onexit cleanup
+
+disk=$TEST_BASE_DIR/$FILEDISK0
+log_must mkfile $SIZE $disk
+
+typeset ashifts=("9" "10" "11" "12" "13" "14" "15" "16")
+# since Illumos 4958 the largest uberblock is 8K so we have at least of 16/label
+typeset ubcount=("128" "128" "64" "32" "16" "16" "16" "16")
+typeset -i i=0;
+while [ $i -lt "${#ashifts[@]}" ]
+do
+	typeset ashift=${ashifts[$i]}
+	log_must zpool create -o ashift=$ashift $TESTPOOL $disk
+	typeset pprop=$(get_pool_prop ashift $TESTPOOL)
+	verify_ashift $disk $ashift
+	if [[ $? -ne 0 || "$pprop" != "$ashift" ]]
+	then
+		log_fail "Pool was created without setting ashift value to "\
+		    "$ashift (current = $pprop)"
+	fi
+	# force 128 txg sync to fill the uberblock ring
+	log_must eval "txg_sync $TESTPOOL 128"
+	verify_device_uberblocks $disk ${ubcount[$i]}
+	if [[ $? -ne 0 ]]
+	then
+		log_fail "Pool was created with unexpected number of uberblocks"
+	fi
+	# clean things for the next run
+	log_must zpool destroy $TESTPOOL
+	log_must zpool labelclear $disk
+	log_must eval "verify_device_uberblocks $disk 0"
+	((i = i + 1))
+done
+
+typeset badvals=("off" "on" "1" "8" "17" "1b" "ff" "-")
+for badval in ${badvals[@]}
+do
+	log_mustnot zpool create -o ashift="$badval" $TESTPOOL $disk
+done
+
+log_pass "zpool create -o ashift=<n>' works with different ashift values"

tests/zfs-tests/tests/functional/cli_root/zpool_replace/Makefile.am

@@ -2,4 +2,6 @@ pkgdatadir = $(datadir)/@PACKAGE@/zfs-tests/tests/functional/cli_root/zpool_repl
 dist_pkgdata_SCRIPTS = \
 	setup.ksh \
 	cleanup.ksh \
-	zpool_replace_001_neg.ksh
+	zpool_replace_001_neg.ksh \
+	replace-o_ashift.ksh \
+	replace_prop_ashift.ksh

tests/zfs-tests/tests/functional/cli_root/zpool_replace/replace-o_ashift.ksh

@@ -0,0 +1,109 @@
+#!/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 http://www.opensolaris.org/os/licensing.
+# 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 2017, loli10K. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/cli_root/zpool_create/zpool_create.shlib
+
+#
+# DESCRIPTION:
+#	'zpool replace -o ashift=<n> ...' should work with different ashift
+#	values.
+#
+# STRATEGY:
+#	1. Create various pools with different ashift values.
+#	2. Verify 'replace -o ashift=<n>' works only with allowed values.
+#
+
+verify_runnable "global"
+
+function cleanup
+{
+	poolexists $TESTPOOL1 && destroy_pool $TESTPOOL1
+	log_must rm -f $disk1
+	log_must rm -f $disk2
+}
+
+log_assert "zpool replace -o ashift=<n>' works with different ashift values"
+log_onexit cleanup
+
+disk1=$TEST_BASE_DIR/$FILEDISK0
+disk2=$TEST_BASE_DIR/$FILEDISK1
+log_must mkfile $SIZE $disk1
+log_must mkfile $SIZE $disk2
+
+typeset ashifts=("9" "10" "11" "12" "13" "14" "15" "16")
+for ashift in ${ashifts[@]}
+do
+	for cmdval in ${ashifts[@]}
+	do
+		log_must zpool create -o ashift=$ashift $TESTPOOL1 $disk1
+		verify_ashift $disk1 $ashift
+		if [[ $? -ne 0 ]]
+		then
+			log_fail "Pool was created without setting ashift " \
+			    "value to $ashift"
+		fi
+		# ashift_of(replacing_disk) <= ashift_of(existing_vdev)
+		if [[ $cmdval -le $ashift ]]
+		then
+			log_must zpool replace -o ashift=$cmdval $TESTPOOL1 \
+			    $disk1 $disk2
+			verify_ashift $disk2 $ashift
+			if [[ $? -ne 0 ]]
+			then
+				log_fail "Device was replaced without " \
+				    "setting ashift value to $ashift"
+			fi
+			wait_replacing $TESTPOOL1
+		else
+			log_mustnot zpool replace -o ashift=$cmdval $TESTPOOL1 \
+			    $disk1 $disk2
+		fi
+		# clean things for the next run
+		log_must zpool destroy $TESTPOOL1
+		# depending on if we expect to have failed the 'zpool replace'
+		if [[ $cmdval -le $ashift ]]
+		then
+			log_mustnot zpool labelclear $disk1
+			log_must zpool labelclear $disk2
+		else
+			log_must zpool labelclear $disk1
+			log_mustnot zpool labelclear $disk2
+		fi
+	done
+done
+
+typeset badvals=("off" "on" "1" "8" "17" "1b" "ff" "-")
+for badval in ${badvals[@]}
+do
+	log_must zpool create $TESTPOOL1 $disk1
+	log_mustnot zpool replace -o ashift=$badval $TESTPOOL1 $disk1 $disk2
+	log_must zpool destroy $TESTPOOL1
+	log_must zpool labelclear $disk1
+	log_mustnot zpool labelclear $disk2
+done
+
+log_pass "zpool replace -o ashift=<n>' works with different ashift values"

tests/zfs-tests/tests/functional/cli_root/zpool_replace/replace_prop_ashift.ksh

@@ -0,0 +1,97 @@
+#!/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 http://www.opensolaris.org/os/licensing.
+# 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 2017, loli10K. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/cli_root/zpool_create/zpool_create.shlib
+
+#
+# DESCRIPTION:
+#	'zpool replace' should use the ashift pool property value as default.
+#
+# STRATEGY:
+#	1. Create a pool with default values.
+#	2. Verify 'zpool replace' uses the ashift pool property value when
+#	   replacing an existing device.
+#	3. Verify the default ashift value can still be overridden by manually
+#	   specifying '-o ashift=<n>' from the command line.
+#
+
+verify_runnable "global"
+
+function cleanup
+{
+	poolexists $TESTPOOL1 && destroy_pool $TESTPOOL1
+	log_must rm -f $disk1 $disk2
+}
+
+log_assert "'zpool replace' uses the ashift pool property value as default."
+log_onexit cleanup
+
+disk1=$TEST_BASE_DIR/$FILEDISK0
+disk2=$TEST_BASE_DIR/$FILEDISK1
+log_must mkfile $SIZE $disk1
+log_must mkfile $SIZE $disk2
+
+typeset ashifts=("9" "10" "11" "12" "13" "14" "15" "16")
+for ashift in ${ashifts[@]}
+do
+	for pprop in ${ashifts[@]}
+	do
+		log_must zpool create -o ashift=$ashift $TESTPOOL1 $disk1
+		log_must zpool set ashift=$pprop $TESTPOOL1
+		# ashift_of(replacing_disk) <= ashift_of(existing_vdev)
+		if [[ $pprop -le $ashift ]]
+		then
+			log_must zpool replace $TESTPOOL1 $disk1 $disk2
+			wait_replacing $TESTPOOL1
+			verify_ashift $disk2 $ashift
+			if [[ $? -ne 0 ]]
+			then
+				log_fail "Device was replaced without " \
+				    "setting ashift value to $ashift"
+			fi
+		else
+			# cannot replace if pool prop ashift > vdev ashift
+			log_mustnot zpool replace $TESTPOOL1 $disk1 $disk2
+			# verify we can override the pool prop value manually
+			log_must zpool replace -o ashift=$ashift $TESTPOOL1 \
+			    $disk1 $disk2
+			wait_replacing $TESTPOOL1
+			verify_ashift $disk2 $ashift
+			if [[ $? -ne 0 ]]
+			then
+				log_fail "Device was replaced without " \
+				    "setting ashift value to $ashift"
+			fi
+		fi
+		# clean things for the next run
+		log_must zpool destroy $TESTPOOL1
+		log_mustnot zpool labelclear $disk1
+		log_must zpool labelclear $disk2
+	done
+done
+
+log_pass "'zpool replace' uses the ashift pool property value."

tests/zfs-tests/tests/functional/cli_root/zpool_set/Makefile.am

@@ -2,4 +2,5 @@ pkgdatadir = $(datadir)/@PACKAGE@/zfs-tests/tests/functional/cli_root/zpool_set
 dist_pkgdata_SCRIPTS = \
 	zpool_set_001_pos.ksh \
 	zpool_set_002_neg.ksh \
-	zpool_set_003_neg.ksh
+	zpool_set_003_neg.ksh \
+	zpool_set_ashift.ksh

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

@@ -0,0 +1,78 @@
+#!/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 http://www.opensolaris.org/os/licensing.
+# 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 2017, loli10K. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/cli_root/zpool_create/zpool_create.shlib
+
+#
+# DESCRIPTION:
+#
+# zpool set can modify 'ashift' property
+#
+# STRATEGY:
+# 1. Create a pool
+# 2. Verify that we can set 'ashift' only to allowed values on that pool
+#
+
+verify_runnable "global"
+
+function cleanup
+{
+	poolexists $TESTPOOL && destroy_pool $TESTPOOL
+	log_must rm -f $disk
+}
+
+typeset goodvals=("0" "9" "10" "11" "12" "13" "14" "15" "16")
+typeset badvals=("off" "on" "1" "8" "17" "1b" "ff" "-")
+
+log_onexit cleanup
+
+log_assert "zpool set can modify 'ashift' property"
+
+disk=$TEST_BASE_DIR/$FILEDISK0
+log_must mkfile $SIZE $disk
+log_must zpool create $TESTPOOL $disk
+
+for ashift in ${goodvals[@]}
+do
+	log_must zpool set ashift=$ashift $TESTPOOL
+	typeset value=$(get_pool_prop ashift $TESTPOOL)
+	if [[ "$ashift" != "$value" ]]; then
+		log_fail "'zpool set' did not update ashift value to $ashift "\
+		    "(current = $value)"
+	fi
+done
+
+for ashift in ${badvals[@]}
+do
+	log_mustnot zpool set ashift=$ashift $TESTPOOL
+	typeset value=$(get_pool_prop ashift $TESTPOOL)
+	if [[ "$ashift" == "$value" ]]; then
+		log_fail "'zpool set' incorrectly set ashift value to $value"
+	fi
+done
+
+log_pass "zpool set can modify 'ashift' property"