diff options
| -rw-r--r-- | cmd/zpool/zpool_vdev.c | 13 | ||||
| -rw-r--r-- | include/sys/fs/zfs.h | 3 | ||||
| -rw-r--r-- | include/sys/spa.h | 11 | ||||
| -rw-r--r-- | lib/libzfs/libzfs_pool.c | 9 | ||||
| -rw-r--r-- | module/zfs/vdev.c | 14 | ||||
| -rw-r--r-- | tests/runfiles/linux.run | 3 | ||||
| -rw-r--r-- | tests/zfs-tests/tests/functional/cli_root/zpool_add/Makefile.am | 3 | ||||
| -rw-r--r-- | tests/zfs-tests/tests/functional/cli_root/zpool_add/add-o_ashift.ksh | 106 |
8 files changed, 153 insertions, 9 deletions
diff --git a/cmd/zpool/zpool_vdev.c b/cmd/zpool/zpool_vdev.c index c0d3076d2..ed607ec85 100644 --- a/cmd/zpool/zpool_vdev.c +++ b/cmd/zpool/zpool_vdev.c @@ -70,6 +70,7 @@ #include <libintl.h> #include <libnvpair.h> #include <limits.h> +#include <sys/spa.h> #include <scsi/scsi.h> #include <scsi/sg.h> #include <stdio.h> @@ -721,8 +722,18 @@ make_leaf_vdev(nvlist_t *props, const char *arg, uint64_t is_log) char *value = NULL; if (nvlist_lookup_string(props, - zpool_prop_to_name(ZPOOL_PROP_ASHIFT), &value) == 0) + zpool_prop_to_name(ZPOOL_PROP_ASHIFT), &value) == 0) { zfs_nicestrtonum(NULL, value, &ashift); + if (ashift != 0 && + (ashift < ASHIFT_MIN || ashift > ASHIFT_MAX)) { + (void) fprintf(stderr, + gettext("invalid 'ashift=%" PRIu64 "' " + "property: only values between %" PRId32 " " + "and %" PRId32 " are allowed.\n"), + ashift, ASHIFT_MIN, ASHIFT_MAX); + return (NULL); + } + } } /* diff --git a/include/sys/fs/zfs.h b/include/sys/fs/zfs.h index 962698c2f..ba6487612 100644 --- a/include/sys/fs/zfs.h +++ b/include/sys/fs/zfs.h @@ -732,7 +732,8 @@ typedef enum vdev_aux { VDEV_AUX_IO_FAILURE, /* experienced I/O failure */ VDEV_AUX_BAD_LOG, /* cannot read log chain(s) */ VDEV_AUX_EXTERNAL, /* external diagnosis */ - VDEV_AUX_SPLIT_POOL /* vdev was split off into another pool */ + VDEV_AUX_SPLIT_POOL, /* vdev was split off into another pool */ + VDEV_AUX_BAD_ASHIFT /* vdev ashift is invalid */ } vdev_aux_t; /* diff --git a/include/sys/spa.h b/include/sys/spa.h index 0f05d04ad..b6e124faa 100644 --- a/include/sys/spa.h +++ b/include/sys/spa.h @@ -122,6 +122,17 @@ _NOTE(CONSTCOND) } while (0) #define SPA_MAXBLOCKSIZE (1ULL << SPA_MAXBLOCKSHIFT) /* + * Alignment Shift (ashift) is an immutable, internal top-level vdev property + * 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). + */ +#define ASHIFT_MIN 9 +#define ASHIFT_MAX 13 + +/* * Size of block to hold the configuration data (a packed nvlist) */ #define SPA_CONFIG_BLOCKSIZE (1ULL << 14) diff --git a/lib/libzfs/libzfs_pool.c b/lib/libzfs/libzfs_pool.c index 616f5061f..16fff89e6 100644 --- a/lib/libzfs/libzfs_pool.c +++ b/lib/libzfs/libzfs_pool.c @@ -538,10 +538,13 @@ zpool_valid_proplist(libzfs_handle_t *hdl, const char *poolname, goto error; } - if (intval != 0 && (intval < 9 || intval > 13)) { + if (intval != 0 && + (intval < ASHIFT_MIN || intval > ASHIFT_MAX)) { zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, - "property '%s' number %d is invalid."), - propname, intval); + "invalid '%s=%d' property: only values " + "between %" PRId32 " and %" PRId32 " " + "are allowed.\n"), + propname, intval, ASHIFT_MIN, ASHIFT_MAX); (void) zfs_error(hdl, EZFS_BADPROP, errbuf); goto error; } diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c index e741a6998..613a9b51e 100644 --- a/module/zfs/vdev.c +++ b/module/zfs/vdev.c @@ -1345,8 +1345,15 @@ vdev_open(vdev_t *vd) */ vd->vdev_asize = asize; vd->vdev_max_asize = max_asize; - if (vd->vdev_ashift == 0) - vd->vdev_ashift = ashift; + if (vd->vdev_ashift == 0) { + vd->vdev_ashift = ashift; /* use detected value */ + } + if (vd->vdev_ashift != 0 && (vd->vdev_ashift < ASHIFT_MIN || + vd->vdev_ashift > ASHIFT_MAX)) { + vdev_set_state(vd, B_TRUE, VDEV_STATE_CANT_OPEN, + VDEV_AUX_BAD_ASHIFT); + return (SET_ERROR(EDOM)); + } } else { /* * Detect if the alignment requirement has increased. @@ -3487,6 +3494,9 @@ vdev_set_state(vdev_t *vd, boolean_t isopen, vdev_state_t state, vdev_aux_t aux) case VDEV_AUX_BAD_LABEL: class = FM_EREPORT_ZFS_DEVICE_BAD_LABEL; break; + case VDEV_AUX_BAD_ASHIFT: + class = FM_EREPORT_ZFS_DEVICE_BAD_ASHIFT; + break; default: class = FM_EREPORT_ZFS_DEVICE_UNKNOWN; } diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run index a7b58474e..73f19ecfa 100644 --- a/tests/runfiles/linux.run +++ b/tests/runfiles/linux.run @@ -230,7 +230,8 @@ tests = ['zpool_001_neg', 'zpool_002_pos', 'zpool_003_pos'] # zpool_add_006_pos - https://github.com/zfsonlinux/zfs/issues/3484 [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'] + 'zpool_add_007_neg', 'zpool_add_008_neg', 'zpool_add_009_neg', + 'add-o_ashift'] [tests/functional/cli_root/zpool_attach] tests = ['zpool_attach_001_neg'] 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 fef934325..60a16e102 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 @@ -12,4 +12,5 @@ dist_pkgdata_SCRIPTS = \ zpool_add_006_pos.ksh \ zpool_add_007_neg.ksh \ zpool_add_008_neg.ksh \ - zpool_add_009_neg.ksh + zpool_add_009_neg.ksh \ + add-o_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 new file mode 100644 index 000000000..a7180b5c5 --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zpool_add/add-o_ashift.ksh @@ -0,0 +1,106 @@ +#!/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 -o ashift=<n> ...' should work with different ashift +# values. +# +# STRATEGY: +# 1. Create a pool with default values. +# 2. Verify 'zpool add -o ashift=<n>' works with allowed values (9-13). +# 3. Verify 'zpool add -o ashift=<n>' doesn't accept other invalid values. +# + +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_assert "zpool add -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") +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 + 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 + +typeset badvals=("off" "on" "1" "8" "14" "1b" "ff" "-") +for badval in ${badvals[@]} +do + log_must $ZPOOL create $TESTPOOL $disk1 + log_mustnot $ZPOOL add -o ashift="$badval" $disk2 + log_must $ZPOOL destroy $TESTPOOL + log_must $ZPOOL labelclear $disk1 + log_must $ZPOOL labelclear $disk2 +done + +log_pass "zpool add -o ashift=<n>' works with different ashift values" |