From 64fd09bad24e2daed089c00cb97fa5347b41f29f Mon Sep 17 00:00:00 2001 From: George Wilson Date: Tue, 7 Nov 2023 11:31:52 -0700 Subject: [PATCH] Add ashift validation when adding devices to a pool Currently, zpool add allows users to add top-level vdevs that have different ashifts but doing so prevents users from being able to perform a top-level vdev removal. Often times consumers may not realize that they have mismatched ashifts until the top-level removal fails. This feature adds ashift validation to the zpool add command and will fail the operation if the sector size of the specified vdev does not match the existing pool. In addition a new flag, -a, is added to the command to disable the ashift validation checks. Signed-off-by: George Wilson --- cmd/zpool/zpool_main.c | 15 ++- cmd/ztest.c | 8 +- include/libzfs.h | 6 +- include/sys/fs/zfs.h | 3 +- include/sys/spa.h | 4 +- lib/libzfs/libzfs_pool.c | 5 +- lib/libzfs/libzfs_util.c | 8 +- man/man8/zpool-add.8 | 10 +- module/zfs/spa.c | 14 +- module/zfs/zfs_ioctl.c | 4 +- tests/runfiles/common.run | 3 +- tests/zfs-tests/tests/Makefile.am | 1 + .../cli_root/zpool_add/add-o_ashift.ksh | 17 ++- .../cli_root/zpool_add/add_prop_ashift.ksh | 16 ++- .../cli_root/zpool_add/zpool_add-a.ksh | 124 ++++++++++++++++++ .../cli_root/zpool_add/zpool_add_004_pos.ksh | 2 +- 16 files changed, 210 insertions(+), 30 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/cli_root/zpool_add/zpool_add-a.ksh diff --git a/cmd/zpool/zpool_main.c b/cmd/zpool/zpool_main.c index 0783271f4734..fd8112da84fa 100644 --- a/cmd/zpool/zpool_main.c +++ b/cmd/zpool/zpool_main.c @@ -22,7 +22,7 @@ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. * Copyright 2011 Nexenta Systems, Inc. All rights reserved. - * Copyright (c) 2011, 2020 by Delphix. All rights reserved. + * Copyright (c) 2011, 2024 by Delphix. All rights reserved. * Copyright (c) 2012 by Frederik Wessels. All rights reserved. * Copyright (c) 2012 by Cyril Plisko. All rights reserved. * Copyright (c) 2013 by Prasad Joshi (sTec). All rights reserved. @@ -347,7 +347,7 @@ get_usage(zpool_help_t idx) { switch (idx) { case HELP_ADD: - return (gettext("\tadd [-fgLnP] [-o property=value] " + return (gettext("\tadd [-afgLnP] [-o property=value] " " ...\n")); case HELP_ATTACH: return (gettext("\tattach [-fsw] [-o property=value] " @@ -1009,8 +1009,9 @@ add_prop_list_default(const char *propname, const char *propval, } /* - * zpool add [-fgLnP] [-o property=value] ... + * zpool add [-afgLnP] [-o property=value] ... * + * -a Disable the ashift validation checks * -f Force addition of devices, even if they appear in use * -g Display guid for individual vdev name. * -L Follow links when resolving vdev path name. @@ -1028,6 +1029,7 @@ zpool_do_add(int argc, char **argv) { boolean_t force = B_FALSE; boolean_t dryrun = B_FALSE; + boolean_t ashift_check = B_TRUE; int name_flags = 0; int c; nvlist_t *nvroot; @@ -1039,8 +1041,11 @@ zpool_do_add(int argc, char **argv) char *propval; /* check options */ - while ((c = getopt(argc, argv, "fgLno:P")) != -1) { + while ((c = getopt(argc, argv, "afgLno:P")) != -1) { switch (c) { + case 'a': + ashift_check = B_FALSE; + break; case 'f': force = B_TRUE; break; @@ -1224,7 +1229,7 @@ zpool_do_add(int argc, char **argv) ret = 0; } else { - ret = (zpool_add(zhp, nvroot) != 0); + ret = (zpool_add(zhp, nvroot, ashift_check) != 0); } nvlist_free(props); diff --git a/cmd/ztest.c b/cmd/ztest.c index 1d414a9f6fd5..684ab586bb93 100644 --- a/cmd/ztest.c +++ b/cmd/ztest.c @@ -20,7 +20,7 @@ */ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2011, 2018 by Delphix. All rights reserved. + * Copyright (c) 2011, 2024 by Delphix. All rights reserved. * Copyright 2011 Nexenta Systems, Inc. All rights reserved. * Copyright (c) 2013 Steven Hartland. All rights reserved. * Copyright (c) 2014 Integros [integros.com] @@ -3375,7 +3375,7 @@ ztest_vdev_add_remove(ztest_ds_t *zd, uint64_t id) "log" : NULL, raidz_children, zs->zs_mirrors, 1); - error = spa_vdev_add(spa, nvroot); + error = spa_vdev_add(spa, nvroot, B_FALSE); fnvlist_free(nvroot); switch (error) { @@ -3438,7 +3438,7 @@ ztest_vdev_class_add(ztest_ds_t *zd, uint64_t id) nvroot = make_vdev_root(NULL, NULL, NULL, ztest_opts.zo_vdev_size, 0, class, raidz_children, zs->zs_mirrors, 1); - error = spa_vdev_add(spa, nvroot); + error = spa_vdev_add(spa, nvroot, B_FALSE); fnvlist_free(nvroot); if (error == ENOSPC) @@ -3545,7 +3545,7 @@ ztest_vdev_aux_add_remove(ztest_ds_t *zd, uint64_t id) */ nvlist_t *nvroot = make_vdev_root(NULL, aux, NULL, (ztest_opts.zo_vdev_size * 5) / 4, 0, NULL, 0, 0, 1); - error = spa_vdev_add(spa, nvroot); + error = spa_vdev_add(spa, nvroot, B_FALSE); switch (error) { case 0: diff --git a/include/libzfs.h b/include/libzfs.h index 4f06b5d3c24c..ee72519d3424 100644 --- a/include/libzfs.h +++ b/include/libzfs.h @@ -21,7 +21,7 @@ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2011, 2022 by Delphix. All rights reserved. + * Copyright (c) 2011, 2024 by Delphix. All rights reserved. * Copyright Joyent, Inc. * Copyright (c) 2013 Steven Hartland. All rights reserved. * Copyright (c) 2016, Intel Corporation. @@ -158,6 +158,7 @@ typedef enum zfs_error { EZFS_RESUME_EXISTS, /* Resume on existing dataset without force */ EZFS_SHAREFAILED, /* filesystem share failed */ EZFS_RAIDZ_EXPAND_IN_PROGRESS, /* a raidz is currently expanding */ + EZFS_ASHIFT_MISMATCH, /* can't add vdevs with different ashifts */ EZFS_UNKNOWN } zfs_error_t; @@ -261,7 +262,8 @@ _LIBZFS_H boolean_t zpool_skip_pool(const char *); _LIBZFS_H int zpool_create(libzfs_handle_t *, const char *, nvlist_t *, nvlist_t *, nvlist_t *); _LIBZFS_H int zpool_destroy(zpool_handle_t *, const char *); -_LIBZFS_H int zpool_add(zpool_handle_t *, nvlist_t *); +_LIBZFS_H int zpool_add(zpool_handle_t *, nvlist_t *, + boolean_t validate_ashift); typedef struct splitflags { /* do not split, but return the config that would be split off */ diff --git a/include/sys/fs/zfs.h b/include/sys/fs/zfs.h index 025567e2183f..21f99bacccf3 100644 --- a/include/sys/fs/zfs.h +++ b/include/sys/fs/zfs.h @@ -21,7 +21,7 @@ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2011, 2020 by Delphix. All rights reserved. + * Copyright (c) 2011, 2024 by Delphix. All rights reserved. * Copyright 2011 Nexenta Systems, Inc. All rights reserved. * Copyright (c) 2013, 2017 Joyent, Inc. All rights reserved. * Copyright (c) 2014 Integros [integros.com] @@ -1603,6 +1603,7 @@ typedef enum { ZFS_ERR_RESUME_EXISTS, ZFS_ERR_CRYPTO_NOTSUP, ZFS_ERR_RAIDZ_EXPAND_IN_PROGRESS, + ZFS_ERR_ASHIFT_MISMATCH, } zfs_errno_t; /* diff --git a/include/sys/spa.h b/include/sys/spa.h index cada3c841037..1e1c730cc3bc 100644 --- a/include/sys/spa.h +++ b/include/sys/spa.h @@ -20,7 +20,7 @@ */ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2011, 2021 by Delphix. All rights reserved. + * Copyright (c) 2011, 2024 by Delphix. All rights reserved. * Copyright 2011 Nexenta Systems, Inc. All rights reserved. * Copyright (c) 2014 Spectra Logic Corporation, All rights reserved. * Copyright 2013 Saso Kiselkov. All rights reserved. @@ -782,7 +782,7 @@ extern int bpobj_enqueue_free_cb(void *arg, const blkptr_t *bp, dmu_tx_t *tx); #define SPA_ASYNC_DETACH_SPARE 0x4000 /* device manipulation */ -extern int spa_vdev_add(spa_t *spa, nvlist_t *nvroot); +extern int spa_vdev_add(spa_t *spa, nvlist_t *nvroot, boolean_t ashift_check); extern int spa_vdev_attach(spa_t *spa, uint64_t guid, nvlist_t *nvroot, int replacing, int rebuild); extern int spa_vdev_detach(spa_t *spa, uint64_t guid, uint64_t pguid, diff --git a/lib/libzfs/libzfs_pool.c b/lib/libzfs/libzfs_pool.c index 402c14a6baee..1e9cd8f8d960 100644 --- a/lib/libzfs/libzfs_pool.c +++ b/lib/libzfs/libzfs_pool.c @@ -22,7 +22,7 @@ /* * Copyright 2015 Nexenta Systems, Inc. All rights reserved. * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2011, 2020 by Delphix. All rights reserved. + * Copyright (c) 2011, 2024 by Delphix. All rights reserved. * Copyright 2016 Igor Kozhukhov * Copyright (c) 2018 Datto Inc. * Copyright (c) 2017 Open-E, Inc. All Rights Reserved. @@ -1724,7 +1724,7 @@ zpool_discard_checkpoint(zpool_handle_t *zhp) * necessary verification to ensure that the vdev specification is well-formed. */ int -zpool_add(zpool_handle_t *zhp, nvlist_t *nvroot) +zpool_add(zpool_handle_t *zhp, nvlist_t *nvroot, boolean_t ashift_check) { zfs_cmd_t zc = {"\0"}; int ret; @@ -1756,6 +1756,7 @@ zpool_add(zpool_handle_t *zhp, nvlist_t *nvroot) zcmd_write_conf_nvlist(hdl, &zc, nvroot); (void) strlcpy(zc.zc_name, zhp->zpool_name, sizeof (zc.zc_name)); + zc.zc_cookie = ashift_check; if (zfs_ioctl(hdl, ZFS_IOC_VDEV_ADD, &zc) != 0) { switch (errno) { diff --git a/lib/libzfs/libzfs_util.c b/lib/libzfs/libzfs_util.c index 8e70af2e5830..73ae0950ccb6 100644 --- a/lib/libzfs/libzfs_util.c +++ b/lib/libzfs/libzfs_util.c @@ -22,7 +22,7 @@ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. * Copyright 2020 Joyent, Inc. All rights reserved. - * Copyright (c) 2011, 2020 by Delphix. All rights reserved. + * Copyright (c) 2011, 2024 by Delphix. All rights reserved. * Copyright 2016 Igor Kozhukhov * Copyright (c) 2017 Datto Inc. * Copyright (c) 2020 The FreeBSD Foundation @@ -319,6 +319,9 @@ libzfs_error_description(libzfs_handle_t *hdl) "dataset without force")); case EZFS_RAIDZ_EXPAND_IN_PROGRESS: return (dgettext(TEXT_DOMAIN, "raidz expansion in progress")); + case EZFS_ASHIFT_MISMATCH: + return (dgettext(TEXT_DOMAIN, "adding devices with " + "different physical sector sizes is not allowed")); case EZFS_UNKNOWN: return (dgettext(TEXT_DOMAIN, "unknown error")); default: @@ -768,6 +771,9 @@ zpool_standard_error_fmt(libzfs_handle_t *hdl, int error, const char *fmt, ...) case ZFS_ERR_RAIDZ_EXPAND_IN_PROGRESS: zfs_verror(hdl, EZFS_RAIDZ_EXPAND_IN_PROGRESS, fmt, ap); break; + case ZFS_ERR_ASHIFT_MISMATCH: + zfs_verror(hdl, EZFS_ASHIFT_MISMATCH, fmt, ap); + break; default: zfs_error_aux(hdl, "%s", zfs_strerror(error)); zfs_verror(hdl, EZFS_UNKNOWN, fmt, ap); diff --git a/man/man8/zpool-add.8 b/man/man8/zpool-add.8 index 8ccdcccc7b06..d22aa489b710 100644 --- a/man/man8/zpool-add.8 +++ b/man/man8/zpool-add.8 @@ -24,6 +24,7 @@ .\" Copyright (c) 2018 George Melikov. All Rights Reserved. .\" Copyright 2017 Nexenta Systems, Inc. .\" Copyright (c) 2017 Open-E, Inc. All Rights Reserved. +.\" Copyright (c) 2024 by Delphix. All Rights Reserved. .\" .Dd March 16, 2022 .Dt ZPOOL-ADD 8 @@ -35,7 +36,7 @@ .Sh SYNOPSIS .Nm zpool .Cm add -.Op Fl fgLnP +.Op Fl afgLnP .Oo Fl o Ar property Ns = Ns Ar value Oc .Ar pool vdev Ns … . @@ -53,6 +54,13 @@ option, and the device checks performed are described in the .Nm zpool Cm create subcommand. .Bl -tag -width Ds +.It Fl a +Disable the ashift validation which allows mismatched ashift values in the +pool. +Adding top-level +.Ar vdev Ns s +with different sector sizes will prohibit future device removal operations, see +.Xr zpool-remove 8 . .It Fl f Forces use of .Ar vdev Ns s , diff --git a/module/zfs/spa.c b/module/zfs/spa.c index b144d0652930..fee652bdfcc1 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -21,7 +21,7 @@ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2011, 2020 by Delphix. All rights reserved. + * Copyright (c) 2011, 2024 by Delphix. All rights reserved. * Copyright (c) 2018, Nexenta Systems, Inc. All rights reserved. * Copyright (c) 2014 Spectra Logic Corporation, All rights reserved. * Copyright 2013 Saso Kiselkov. All rights reserved. @@ -7082,7 +7082,7 @@ spa_draid_feature_incr(void *arg, dmu_tx_t *tx) * Add a device to a storage pool. */ int -spa_vdev_add(spa_t *spa, nvlist_t *nvroot) +spa_vdev_add(spa_t *spa, nvlist_t *nvroot, boolean_t ashift_check) { uint64_t txg, ndraid = 0; int error; @@ -7173,6 +7173,16 @@ spa_vdev_add(spa_t *spa, nvlist_t *nvroot) } } + if (ashift_check) { + for (int c = 0; c < vd->vdev_children; c++) { + tvd = vd->vdev_child[c]; + if (tvd->vdev_ashift != spa->spa_max_ashift) { + return (spa_vdev_exit(spa, vd, txg, + ZFS_ERR_ASHIFT_MISMATCH)); + } + } + } + for (int c = 0; c < vd->vdev_children; c++) { tvd = vd->vdev_child[c]; vdev_remove_child(vd, tvd); diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c index b2b06881bdd4..88172fc85460 100644 --- a/module/zfs/zfs_ioctl.c +++ b/module/zfs/zfs_ioctl.c @@ -27,7 +27,7 @@ * Copyright (c) 2014, 2016 Joyent, Inc. All rights reserved. * Copyright 2016 Nexenta Systems, Inc. All rights reserved. * Copyright (c) 2014, Joyent, Inc. All rights reserved. - * Copyright (c) 2011, 2020 by Delphix. All rights reserved. + * Copyright (c) 2011, 2024 by Delphix. All rights reserved. * Copyright (c) 2013 by Saso Kiselkov. All rights reserved. * Copyright (c) 2013 Steven Hartland. All rights reserved. * Copyright (c) 2014 Integros [integros.com] @@ -1886,7 +1886,7 @@ zfs_ioc_vdev_add(zfs_cmd_t *zc) error = get_nvlist(zc->zc_nvlist_conf, zc->zc_nvlist_conf_size, zc->zc_iflags, &config); if (error == 0) { - error = spa_vdev_add(spa, config); + error = spa_vdev_add(spa, config, zc->zc_cookie); nvlist_free(config); } spa_close(spa, FTAG); diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index 502b4de2bae9..82c7b8aaf3a3 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -372,7 +372,8 @@ tags = ['functional', 'cli_root', 'zpool'] tests = ['zpool_add_001_pos', 'zpool_add_002_pos', 'zpool_add_003_pos', 'zpool_add_004_pos', 'zpool_add_006_pos', 'zpool_add_007_neg', 'zpool_add_008_neg', 'zpool_add_009_neg', 'zpool_add_010_pos', - 'add-o_ashift', 'add_prop_ashift', 'zpool_add_dryrun_output'] + 'add-o_ashift', 'add_prop_ashift', 'zpool_add_dryrun_output', + 'zpool_add-a'] tags = ['functional', 'cli_root', 'zpool_add'] [tests/functional/cli_root/zpool_attach] diff --git a/tests/zfs-tests/tests/Makefile.am b/tests/zfs-tests/tests/Makefile.am index fe9c92108725..f74492e5632c 100644 --- a/tests/zfs-tests/tests/Makefile.am +++ b/tests/zfs-tests/tests/Makefile.am @@ -988,6 +988,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \ functional/cli_root/zpool_add/add_prop_ashift.ksh \ functional/cli_root/zpool_add/cleanup.ksh \ functional/cli_root/zpool_add/setup.ksh \ + functional/cli_root/zpool_add/zpool_add-a.ksh \ functional/cli_root/zpool_add/zpool_add_001_pos.ksh \ functional/cli_root/zpool_add/zpool_add_002_pos.ksh \ functional/cli_root/zpool_add/zpool_add_003_pos.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 7ecaf849e44b..e2d57b8fdb9b 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 @@ -22,7 +22,7 @@ # # Copyright 2017, loli10K. All rights reserved. -# Copyright (c) 2020 by Delphix. All rights reserved. +# Copyright (c) 2020, 2024 by Delphix. All rights reserved. # . $STF_SUITE/include/libtest.shlib @@ -60,12 +60,23 @@ log_must mkfile $SIZE $disk2 logical_ashift=$(get_tunable VDEV_FILE_LOGICAL_ASHIFT) orig_ashift=$(get_tunable VDEV_FILE_PHYSICAL_ASHIFT) max_auto_ashift=$(get_tunable VDEV_MAX_AUTO_ASHIFT) +opt="" typeset ashifts=("9" "10" "11" "12" "13" "14" "15" "16") for ashift in ${ashifts[@]} do + # + # Need to add the -a option to disable the ashift mismatch + # checks in zpool add. + # + if [[ $ashift -eq $orig_ashift ]]; then + opt="" + else + opt="-a" + fi + log_must zpool create $TESTPOOL $disk1 - log_must zpool add -o ashift=$ashift $TESTPOOL $disk2 + log_must zpool add $opt -o ashift=$ashift $TESTPOOL $disk2 log_must verify_ashift $disk2 $ashift # clean things for the next run @@ -78,7 +89,7 @@ do # log_must zpool create $TESTPOOL $disk1 log_must set_tunable32 VDEV_FILE_PHYSICAL_ASHIFT $ashift - log_must zpool add $TESTPOOL $disk2 + log_must zpool add $opt $TESTPOOL $disk2 exp=$(( (ashift <= max_auto_ashift) ? ashift : logical_ashift )) log_must verify_ashift $disk2 $exp 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 index 228f62232aae..497f6621ee4d 100755 --- 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 @@ -22,7 +22,7 @@ # # Copyright 2017, loli10K. All rights reserved. -# Copyright (c) 2020 by Delphix. All rights reserved. +# Copyright (c) 2020, 2024 by Delphix. All rights reserved. # . $STF_SUITE/include/libtest.shlib @@ -68,8 +68,13 @@ log_must set_tunable32 VDEV_FILE_PHYSICAL_ASHIFT 16 typeset ashifts=("9" "10" "11" "12" "13" "14" "15" "16") for ashift in ${ashifts[@]} do + if [ $ashift -eq $orig_ashift ];then + opt="" + else + opt="-a" + fi log_must zpool create -o ashift=$ashift $TESTPOOL $disk1 - log_must zpool add $TESTPOOL $disk2 + log_must zpool add $opt $TESTPOOL $disk2 log_must verify_ashift $disk2 $ashift # clean things for the next run @@ -82,8 +87,13 @@ for ashift in ${ashifts[@]} do for cmdval in ${ashifts[@]} do + if [ $ashift -eq $cmdval ];then + opt="" + else + opt="-a" + fi log_must zpool create -o ashift=$ashift $TESTPOOL $disk1 - log_must zpool add -o ashift=$cmdval $TESTPOOL $disk2 + log_must zpool add $opt -o ashift=$cmdval $TESTPOOL $disk2 log_must verify_ashift $disk2 $cmdval # clean things for the next run diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_add/zpool_add-a.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_add/zpool_add-a.ksh new file mode 100755 index 000000000000..38f067364673 --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zpool_add/zpool_add-a.ksh @@ -0,0 +1,124 @@ +#!/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 https://opensource.org/licenses/CDDL-1.0. +# 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 (c) 2024 by Delphix. All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/tests/functional/cli_root/zpool_add/zpool_add.kshlib + +# +# DESCRIPTION: +# 'zpool add' should only work if the ashift matches the pool +# ashift unless the '-a' option is provided. +# +# STRATEGY: +# 1. Create a pool with default values. +# 2. Add a disk with a specific ashift +# 3. Verify that the operation is allowed if ashift match +# 4. Attempt to add a vdev with a mismatched ashift and ensure it fails +# 5. Rerun the operation but specify the -a to disable the check +# + +verify_runnable "global" + +function cleanup +{ + log_must set_tunable32 VDEV_FILE_PHYSICAL_ASHIFT $orig_physical_ashift + log_must set_tunable32 VDEV_FILE_LOGICAL_ASHIFT $orig_logical_ashift + poolexists $TESTPOOL && destroy_pool $TESTPOOL + rm -f $disk1 $disk2 +} + +log_assert "zpool add only works when device ashift matches pool ashift" +log_onexit cleanup + +disk1=$TEST_BASE_DIR/disk1 +disk2=$TEST_BASE_DIR/disk2 +log_must mkfile $SIZE $disk1 +log_must mkfile $SIZE $disk2 + +orig_logical_ashift=$(get_tunable VDEV_FILE_LOGICAL_ASHIFT) +orig_physical_ashift=$(get_tunable VDEV_FILE_PHYSICAL_ASHIFT) + +typeset ashifts=("9" "10" "11" "12" "13" "14" "15" "16") +for ashift_disk1 in ${ashifts[@]} +do + for ashift_disk2 in ${ashifts[@]} + do + # + # The default case without any parameters + # + if [[ $ashift_disk1 -eq $ashift_disk2 ]]; then + log_must zpool create $TESTPOOL $disk1 + log_must zpool add $TESTPOOL $disk2 + + # clean things for the next run + log_must set_tunable32 VDEV_FILE_PHYSICAL_ASHIFT $orig_physical_ashift + log_must set_tunable32 VDEV_FILE_LOGICAL_ASHIFT $orig_logical_ashift + log_must zpool destroy $TESTPOOL + log_must zpool labelclear $disk1 + log_must zpool labelclear $disk2 + continue + fi + + # + # 1. Create a pool with a specific ashift + # 2. Try to add a vdev with a mismatched ashift + # 3. Disable the ashift validation to add the vdev + # + log_must zpool create -o ashift=$ashift_disk1 $TESTPOOL $disk1 + log_must verify_ashift $disk1 $ashift_disk1 + log_mustnot zpool add -o ashift=$ashift_disk2 $TESTPOOL $disk2 + log_must zpool add -a -o ashift=$ashift_disk2 $TESTPOOL $disk2 + log_must verify_ashift $disk2 $ashift_disk2 + + # clean things for the next run + log_must zpool destroy $TESTPOOL + log_must zpool labelclear $disk1 + log_must zpool labelclear $disk2 + + # + # Test the tunable. + # + log_must set_tunable32 VDEV_FILE_PHYSICAL_ASHIFT $ashift_disk1 + log_must set_tunable32 VDEV_FILE_LOGICAL_ASHIFT $ashift_disk1 + log_must zpool create $TESTPOOL $disk1 + log_must verify_ashift $disk1 $(get_tunable VDEV_FILE_LOGICAL_ASHIFT) + + log_must set_tunable32 VDEV_FILE_PHYSICAL_ASHIFT $ashift_disk2 + log_must set_tunable32 VDEV_FILE_LOGICAL_ASHIFT $ashift_disk2 + log_mustnot zpool add $TESTPOOL $disk2 + log_must zpool add -a $TESTPOOL $disk2 + log_must verify_ashift $disk2 $(get_tunable VDEV_FILE_LOGICAL_ASHIFT) + + # clean things for the next run + log_must set_tunable32 VDEV_FILE_PHYSICAL_ASHIFT $orig_physical_ashift + log_must set_tunable32 VDEV_FILE_LOGICAL_ASHIFT $orig_logical_ashift + log_must zpool destroy $TESTPOOL + log_must zpool labelclear $disk1 + log_must zpool labelclear $disk2 + done +done + +log_pass "zpool add only works when device ashift matches pool ashift" diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_add/zpool_add_004_pos.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_add/zpool_add_004_pos.ksh index 646edc1a4557..706c88e3e1f5 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zpool_add/zpool_add_004_pos.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zpool_add/zpool_add_004_pos.ksh @@ -70,7 +70,7 @@ if is_freebsd; then recursive=$(get_tunable VOL_RECURSIVE) log_must set_tunable64 VOL_RECURSIVE 1 fi -log_must zpool add $TESTPOOL $ZVOL_DEVDIR/$TESTPOOL1/$TESTVOL +log_must zpool add -a $TESTPOOL $ZVOL_DEVDIR/$TESTPOOL1/$TESTVOL log_must vdevs_in_pool "$TESTPOOL" "$ZVOL_DEVDIR/$TESTPOOL1/$TESTVOL"