diff options
20 files changed, 739 insertions, 73 deletions
diff --git a/cmd/zpool/zpool_main.c b/cmd/zpool/zpool_main.c index e8c8dba79..edcfe2db7 100644 --- a/cmd/zpool/zpool_main.c +++ b/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) { diff --git a/cmd/zpool/zpool_vdev.c b/cmd/zpool/zpool_vdev.c index c96157eff..d3c281704 100644 --- a/cmd/zpool/zpool_vdev.c +++ b/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, diff --git a/include/sys/spa.h b/include/sys/spa.h index e2f27ed60..22a396689 100644 --- a/include/sys/spa.h +++ b/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 - * (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) diff --git a/lib/libzfs/libzfs_pool.c b/lib/libzfs/libzfs_pool.c index 087fb98eb..e64d0365a 100644 --- a/lib/libzfs/libzfs_pool.c +++ b/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, diff --git a/man/man8/zpool.8 b/man/man8/zpool.8 index c9593d966..89ef1bc7b 100644 --- a/man/man8/zpool.8 +++ b/man/man8/zpool.8 @@ -562,23 +562,6 @@ The space usage properties report actual physical space available to the storage .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: .sp .ne 2 @@ -613,6 +596,18 @@ The following properties can be set at creation time and import time, and later .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 \fB\fBautoexpand\fR=\fBoff\fR | \fBon\fR\fR .ad .sp .6 @@ -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 diff --git a/module/zcommon/zpool_prop.c b/module/zcommon/zpool_prop.c index 4a5836e5b..77b4d62ff 100644 --- a/module/zcommon/zpool_prop.c +++ b/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, diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run index 8439f8e66..cf0dd126f 100644 --- a/tests/runfiles/linux.run +++ b/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', diff --git a/tests/zfs-tests/include/libtest.shlib b/tests/zfs-tests/include/libtest.shlib index 07cf9a1e6..aafc47b2b 100644 --- a/tests/zfs-tests/include/libtest.shlib +++ b/tests/zfs-tests/include/libtest.shlib @@ -1825,6 +1825,29 @@ function snapshot_mountpoint } # +# 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 # to ensure it has consistent state. @@ -3220,6 +3243,21 @@ function wait_freeing #pool } # +# 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. # function zed_start diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_add/Makefile.am b/tests/zfs-tests/tests/functional/cli_root/zpool_add/Makefile.am index 60a16e102..30be8ecfc 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zpool_add/Makefile.am +++ b/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 diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_add/add-o_ashift.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_add/add-o_ashift.ksh index 6998a50cf..8556f298e 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zpool_add/add-o_ashift.ksh +++ b/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 -} - -# -# 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 index 000000000..29debe106 --- /dev/null +++ b/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." diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_attach/Makefile.am b/tests/zfs-tests/tests/functional/cli_root/zpool_attach/Makefile.am index f642108e6..cc742f33d 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zpool_attach/Makefile.am +++ b/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 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 index 000000000..58ab57db0 --- /dev/null +++ b/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" diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_create/Makefile.am b/tests/zfs-tests/tests/functional/cli_root/zpool_create/Makefile.am index 8cfc5bd00..5af41e6a3 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zpool_create/Makefile.am +++ b/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 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 index 000000000..33ff4534d --- /dev/null +++ b/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" diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_replace/Makefile.am b/tests/zfs-tests/tests/functional/cli_root/zpool_replace/Makefile.am index ce1b63072..2e3ea69f2 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zpool_replace/Makefile.am +++ b/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 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 index 000000000..77f85c6be --- /dev/null +++ b/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" 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 index 000000000..714f1180f --- /dev/null +++ b/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." 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 185d4b185..e01dfde6b 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zpool_set/Makefile.am +++ b/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 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 index 000000000..4a192698a --- /dev/null +++ b/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" |