]> granicus.if.org Git - zfs/commitdiff
More ashift improvements
authorLOLi <loli10K@users.noreply.github.com>
Wed, 3 May 2017 16:31:05 +0000 (18:31 +0200)
committerBrian Behlendorf <behlendorf1@llnl.gov>
Wed, 3 May 2017 16:31:05 +0000 (09:31 -0700)
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

20 files changed:
cmd/zpool/zpool_main.c
cmd/zpool/zpool_vdev.c
include/sys/spa.h
lib/libzfs/libzfs_pool.c
man/man8/zpool.8
module/zcommon/zpool_prop.c
tests/runfiles/linux.run
tests/zfs-tests/include/libtest.shlib
tests/zfs-tests/tests/functional/cli_root/zpool_add/Makefile.am
tests/zfs-tests/tests/functional/cli_root/zpool_add/add-o_ashift.ksh
tests/zfs-tests/tests/functional/cli_root/zpool_add/add_prop_ashift.ksh [new file with mode: 0755]
tests/zfs-tests/tests/functional/cli_root/zpool_attach/Makefile.am
tests/zfs-tests/tests/functional/cli_root/zpool_attach/attach-o_ashift.ksh [new file with mode: 0755]
tests/zfs-tests/tests/functional/cli_root/zpool_create/Makefile.am
tests/zfs-tests/tests/functional/cli_root/zpool_create/create-o_ashift.ksh [new file with mode: 0755]
tests/zfs-tests/tests/functional/cli_root/zpool_replace/Makefile.am
tests/zfs-tests/tests/functional/cli_root/zpool_replace/replace-o_ashift.ksh [new file with mode: 0755]
tests/zfs-tests/tests/functional/cli_root/zpool_replace/replace_prop_ashift.ksh [new file with mode: 0755]
tests/zfs-tests/tests/functional/cli_root/zpool_set/Makefile.am
tests/zfs-tests/tests/functional/cli_root/zpool_set/zpool_set_ashift.ksh [new file with mode: 0755]

index e8c8dba793d031ad4f18fc29ef20afb43ce67d01..edcfe2db78b13bd5bb7851d8237f0d675d9c3d48 100644 (file)
@@ -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) {
index c96157eff6327b9ae4570ded37f0f99fed3be344..d3c28170430f2c43e025447b6dbd166b577b7cf6 100644 (file)
@@ -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,
index e2f27ed60b82ce9ae9e8b392342e7b4e0bc07133..22a396689b178b06297c6219bf7be225b4ab705f 100644 (file)
@@ -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
- * (2^13 = 8,192).
+ * We currently allow values ranging from 512 bytes (2^9 = 512) to 64 KiB
+ * (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)
index 087fb98eb4311a8f184227427df357afa6bd72f0..e64d0365a2ef101840737140d40d1436e11d6f1f 100644 (file)
@@ -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,
index c9593d9664fda162ab1180eeed0415c702b2552e..89ef1bc7beaff59f71a4fabcb6a5e8f9b8013532 100644 (file)
@@ -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
index 4a5836e5b73905d0544abe685e25f3f7e57ed4e5..77b4d62ffde2c76db9d095d29af9c3c0b1aada00 100644 (file)
@@ -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,
index 8439f8e6691b8c7054df5cacbbb5ffee8c3bebc8..cf0dd126ff8a408dd08b8e8ba14b75aaa1c16bb3 100644 (file)
@@ -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',
index 07cf9a1e65aed7d583010f7e1d9114e4da11ecf9..aafc47b2b15103c88ba5f998ec1563534392a462 100644 (file)
@@ -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.
 #
index 60a16e1024416647526989685b225c5c9e081404..30be8ecfc60f25b8c34d8ab8c4dcbe877a403659 100644 (file)
@@ -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
index 6998a50cf8039e5f608a7c861c32dac311e2effb..8556f298e71b11f50d433e570ac665ef799dae4e 100755 (executable)
@@ -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
-}
-
-#
-# 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_must rm -f $disk1 $disk2
 }
 
 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
diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_add/add_prop_ashift.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_add/add_prop_ashift.ksh
new file mode 100755 (executable)
index 0000000..29debe1
--- /dev/null
@@ -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."
index f642108e622bfb0aa4fe03ff2785280b0f7308d7..cc742f33d2aae021d318747899e3622cbe8128af 100644 (file)
@@ -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
diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_attach/attach-o_ashift.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_attach/attach-o_ashift.ksh
new file mode 100755 (executable)
index 0000000..58ab57d
--- /dev/null
@@ -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"
index 8cfc5bd00231cf70e1111fae418960bb306993d0..5af41e6a35bc622e47f5f1bc8c7cbd6ddecc373d 100644 (file)
@@ -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
diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_create/create-o_ashift.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_create/create-o_ashift.ksh
new file mode 100755 (executable)
index 0000000..33ff453
--- /dev/null
@@ -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"
index ce1b63072909a790ff706dbc4e42be37ff2d41a7..2e3ea69f2ca90fb1070585e0bc42ce1404a192d5 100644 (file)
@@ -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
diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_replace/replace-o_ashift.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_replace/replace-o_ashift.ksh
new file mode 100755 (executable)
index 0000000..77f85c6
--- /dev/null
@@ -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"
diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_replace/replace_prop_ashift.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_replace/replace_prop_ashift.ksh
new file mode 100755 (executable)
index 0000000..714f118
--- /dev/null
@@ -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."
index 185d4b185bd6b704494d0670a0dc6de96beffdc9..e01dfde6b60ecb6e1701417dc3794f4f4208327d 100644 (file)
@@ -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
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
new file mode 100755 (executable)
index 0000000..4a19269
--- /dev/null
@@ -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"