From b4555c777a0be3c0dba29662d278c57099c60a87 Mon Sep 17 00:00:00 2001 From: LOLi Date: Thu, 19 Apr 2018 18:45:17 +0200 Subject: [PATCH] Fix 'zfs remap ' Only filesystems and volumes are valid 'zfs remap' parameters: when passed a snapshot name zfs_remap_indirects() does not handle the EINVAL returned from libzfs_core, which results in failing an assertion and consequently crashing. Reviewed-by: Brian Behlendorf Reviewed by: Matthew Ahrens Reviewed-by: Giuseppe Di Natale Signed-off-by: loli10K Closes #7454 --- cmd/zfs/zfs_main.c | 18 +++++ configure.ac | 1 + lib/libzfs/libzfs_dataset.c | 16 +++- tests/runfiles/linux.run | 4 + .../tests/functional/cli_root/Makefile.am | 1 + .../functional/cli_root/zfs_remap/Makefile.am | 7 ++ .../functional/cli_root/zfs_remap/cleanup.ksh | 19 +++++ .../functional/cli_root/zfs_remap/setup.ksh | 17 ++++ .../cli_root/zfs_remap/zfs_remap_cliargs.ksh | 78 +++++++++++++++++++ .../zfs_remap/zfs_remap_obsolete_counts.ksh | 76 ++++++++++++++++++ 10 files changed, 235 insertions(+), 2 deletions(-) create mode 100644 tests/zfs-tests/tests/functional/cli_root/zfs_remap/Makefile.am create mode 100755 tests/zfs-tests/tests/functional/cli_root/zfs_remap/cleanup.ksh create mode 100755 tests/zfs-tests/tests/functional/cli_root/zfs_remap/setup.ksh create mode 100755 tests/zfs-tests/tests/functional/cli_root/zfs_remap/zfs_remap_cliargs.ksh create mode 100755 tests/zfs-tests/tests/functional/cli_root/zfs_remap/zfs_remap_obsolete_counts.ksh diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c index 45d9a661b..7d81dcaee 100644 --- a/cmd/zfs/zfs_main.c +++ b/cmd/zfs/zfs_main.c @@ -7108,11 +7108,29 @@ zfs_do_diff(int argc, char **argv) return (err != 0); } + +/* + * zfs remap + * + * Remap the indirect blocks in the given fileystem or volume. + */ static int zfs_do_remap(int argc, char **argv) { const char *fsname; int err = 0; + int c; + + /* check options */ + while ((c = getopt(argc, argv, "")) != -1) { + switch (c) { + case '?': + (void) fprintf(stderr, + gettext("invalid option '%c'\n"), optopt); + usage(B_FALSE); + } + } + if (argc != 2) { (void) fprintf(stderr, gettext("wrong number of arguments\n")); usage(B_FALSE); diff --git a/configure.ac b/configure.ac index ebce007a6..6dc313b4d 100644 --- a/configure.ac +++ b/configure.ac @@ -211,6 +211,7 @@ AC_CONFIG_FILES([ tests/zfs-tests/tests/functional/cli_root/zfs_promote/Makefile tests/zfs-tests/tests/functional/cli_root/zfs_property/Makefile tests/zfs-tests/tests/functional/cli_root/zfs_receive/Makefile + tests/zfs-tests/tests/functional/cli_root/zfs_remap/Makefile tests/zfs-tests/tests/functional/cli_root/zfs_rename/Makefile tests/zfs-tests/tests/functional/cli_root/zfs_reservation/Makefile tests/zfs-tests/tests/functional/cli_root/zfs_rollback/Makefile diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c index 3eb3aebbc..ecbc12bfe 100644 --- a/lib/libzfs/libzfs_dataset.c +++ b/lib/libzfs/libzfs_dataset.c @@ -4075,12 +4075,24 @@ zfs_remap_indirects(libzfs_handle_t *hdl, const char *fs) char errbuf[1024]; (void) snprintf(errbuf, sizeof (errbuf), dgettext(TEXT_DOMAIN, - "cannot remap filesystem '%s' "), fs); + "cannot remap dataset '%s'"), fs); err = lzc_remap(fs); if (err != 0) { - (void) zfs_standard_error(hdl, err, errbuf); + switch (err) { + case ENOTSUP: + zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, + "pool must be upgraded")); + (void) zfs_error(hdl, EZFS_BADVERSION, errbuf); + break; + case EINVAL: + (void) zfs_error(hdl, EZFS_BADTYPE, errbuf); + break; + default: + (void) zfs_standard_error(hdl, err, errbuf); + break; + } } return (err); diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run index 7e4a5a7c3..cb2c27553 100644 --- a/tests/runfiles/linux.run +++ b/tests/runfiles/linux.run @@ -203,6 +203,10 @@ tests = ['zfs_receive_001_pos', 'zfs_receive_002_pos', 'zfs_receive_003_pos', 'zfs_receive_raw_incremental'] tags = ['functional', 'cli_root', 'zfs_receive'] +[tests/functional/cli_root/zfs_remap] +tests = ['zfs_remap_cliargs', 'zfs_remap_obsolete_counts'] +tags = ['functional', 'cli_root', 'zfs_remap'] + # zfs_rename_006_pos - https://github.com/zfsonlinux/zfs/issues/5647 # zfs_rename_009_neg - https://github.com/zfsonlinux/zfs/issues/5648 [tests/functional/cli_root/zfs_rename] diff --git a/tests/zfs-tests/tests/functional/cli_root/Makefile.am b/tests/zfs-tests/tests/functional/cli_root/Makefile.am index d34b40877..aab49b168 100644 --- a/tests/zfs-tests/tests/functional/cli_root/Makefile.am +++ b/tests/zfs-tests/tests/functional/cli_root/Makefile.am @@ -20,6 +20,7 @@ SUBDIRS = \ zfs_promote \ zfs_property \ zfs_receive \ + zfs_remap \ zfs_rename \ zfs_reservation \ zfs_rollback \ diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_remap/Makefile.am b/tests/zfs-tests/tests/functional/cli_root/zfs_remap/Makefile.am new file mode 100644 index 000000000..91abff68c --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_remap/Makefile.am @@ -0,0 +1,7 @@ +pkgdatadir = $(datadir)/@PACKAGE@/zfs-tests/tests/functional/cli_root/zfs_remap + +dist_pkgdata_SCRIPTS = \ + setup.ksh \ + cleanup.ksh \ + zfs_remap_cliargs.ksh \ + zfs_remap_obsolete_counts.ksh diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_remap/cleanup.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_remap/cleanup.ksh new file mode 100755 index 000000000..e78deacd5 --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_remap/cleanup.ksh @@ -0,0 +1,19 @@ +#!/bin/ksh -p +# +# This file and its contents are supplied under the terms of the +# Common Development and Distribution License ("CDDL"), version 1.0. +# You may only use this file in accordance with the terms of version +# 1.0 of the CDDL. +# +# A full copy of the text of the CDDL should have accompanied this +# source. A copy of the CDDL is also available via the Internet at +# http://www.illumos.org/license/CDDL. +# + +# +# Copyright 2018, loli10K . All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib + +default_cleanup diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_remap/setup.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_remap/setup.ksh new file mode 100755 index 000000000..4497dbd74 --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_remap/setup.ksh @@ -0,0 +1,17 @@ +#!/bin/ksh -p +# +# This file and its contents are supplied under the terms of the +# Common Development and Distribution License ("CDDL"), version 1.0. +# You may only use this file in accordance with the terms of version +# 1.0 of the CDDL. +# +# A full copy of the text of the CDDL should have accompanied this +# source. A copy of the CDDL is also available via the Internet at +# http://www.illumos.org/license/CDDL. +# + +# +# Copyright 2018, loli10K . All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_remap/zfs_remap_cliargs.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_remap/zfs_remap_cliargs.ksh new file mode 100755 index 000000000..9a0b7d619 --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_remap/zfs_remap_cliargs.ksh @@ -0,0 +1,78 @@ +#!/bin/ksh -p +# +# This file and its contents are supplied under the terms of the +# Common Development and Distribution License ("CDDL"), version 1.0. +# You may only use this file in accordance with the terms of version +# 1.0 of the CDDL. +# +# A full copy of the text of the CDDL should have accompanied this +# source. A copy of the CDDL is also available via the Internet at +# http://www.illumos.org/license/CDDL. +# + +# +# Copyright 2018, loli10K . All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/tests/functional/removal/removal.kshlib + +# +# DESCRIPTION: +# 'zfs remap' should only work with supported parameters. +# +# STRATEGY: +# 1. Prepare a pool where a top-level VDEV has been removed +# 2. Verify every supported parameter to 'zfs remap' is accepted +# 3. Verify other unsupported parameters raise an error +# + +verify_runnable "both" + +function cleanup +{ + destroy_pool $TESTPOOL + rm -f $DISK1 $DISK2 +} + +log_assert "'zfs remap' should only work with supported parameters" +log_onexit cleanup + +f="$TESTPOOL/fs" +v="$TESTPOOL/vol" +s="$TESTPOOL/fs@snap" +b="$TESTPOOL/fs#bmark" +c="$TESTPOOL/clone" + +typeset goodparams=("$f" "$v" "$c") +typeset badparams=("-H" "-p" "-?" "$s" "$b" "$f $f" "$f $v" "$f $s") + +DISK1="$TEST_BASE_DIR/zfs_remap-1" +DISK2="$TEST_BASE_DIR/zfs_remap-2" + +# 1. Prepare a pool where a top-level VDEV has been removed +log_must truncate -s $(($MINVDEVSIZE * 2)) $DISK1 +log_must zpool create $TESTPOOL $DISK1 +log_must zfs create $f +log_must zfs create -V 1M -s $v +log_must zfs snap $s +log_must zfs bookmark $s $b +log_must zfs clone $s $c +log_must truncate -s $(($MINVDEVSIZE * 2)) $DISK2 +log_must zpool add $TESTPOOL $DISK2 +log_must zpool remove $TESTPOOL $DISK1 +log_must wait_for_removal $TESTPOOL + +# 2. Verify every supported parameter to 'zfs remap' is accepted +for param in "${goodparams[@]}" +do + log_must zfs remap $param +done + +# 3. Verify other unsupported parameters raise an error +for param in "${badparams[@]}" +do + log_mustnot zfs remap $param +done + +log_pass "'zfs remap' only works with supported parameters" diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_remap/zfs_remap_obsolete_counts.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_remap/zfs_remap_obsolete_counts.ksh new file mode 100755 index 000000000..15d3ae493 --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_remap/zfs_remap_obsolete_counts.ksh @@ -0,0 +1,76 @@ +#!/bin/ksh -p +# +# This file and its contents are supplied under the terms of the +# Common Development and Distribution License ("CDDL"), version 1.0. +# You may only use this file in accordance with the terms of version +# 1.0 of the CDDL. +# +# A full copy of the text of the CDDL should have accompanied this +# source. A copy of the CDDL is also available via the Internet at +# http://www.illumos.org/license/CDDL. +# + +# +# Copyright 2018, loli10K . All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/tests/functional/removal/removal.kshlib + +# +# DESCRIPTION: +# 'zfs remap' depends on 'feature@obsolete_counts' being active +# +# STRATEGY: +# 1. Prepare a pool where a top-level VDEV has been removed and with +# feature@obsolete_counts disabled +# 2. Verify any 'zfs remap' command cannot be executed +# 3. Verify the same commands complete successfully when +# feature@obsolete_counts is enabled +# + +verify_runnable "both" + +function cleanup +{ + destroy_pool $TESTPOOL + rm -f $DISK1 $DISK2 +} + +log_assert "'zfs remap' depends on feature@obsolete_counts being active" +log_onexit cleanup + +f="$TESTPOOL/fs" +v="$TESTPOOL/vol" +s="$TESTPOOL/fs@snap" +c="$TESTPOOL/clone" + +DISK1="$TEST_BASE_DIR/zfs_remap-1" +DISK2="$TEST_BASE_DIR/zfs_remap-2" + +# 1. Prepare a pool where a top-level VDEV has been removed with +# feature@obsolete_counts disabled +log_must truncate -s $(($MINVDEVSIZE * 2)) $DISK1 +log_must zpool create -o feature@obsolete_counts=disabled $TESTPOOL $DISK1 +log_must zfs create $f +log_must zfs create -V 1M -s $v +log_must zfs snap $s +log_must zfs clone $s $c +log_must truncate -s $(($MINVDEVSIZE * 2)) $DISK2 +log_must zpool add $TESTPOOL $DISK2 +log_must zpool remove $TESTPOOL $DISK1 +log_must wait_for_removal $TESTPOOL + +# 2. Verify any 'zfs remap' command cannot be executed +log_mustnot zfs remap $f +log_mustnot zfs remap $v +log_mustnot zfs remap $c + +# 3. Verify the same commands complete successfully when +# feature@obsolete_counts is enabled +log_must zpool set feature@obsolete_counts=enabled $TESTPOOL +log_must zfs remap $f +log_must zfs remap $v +log_must zfs remap $c + +log_pass "'zfs remap' correctly depends on feature@obsolete_counts being active" -- 2.39.2