Fix 'zfs remap <poolname@snapname>'

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 <[email protected]>
Reviewed by: Matthew Ahrens <[email protected]>
Reviewed-by: Giuseppe Di Natale <[email protected]>
Signed-off-by: loli10K <[email protected]>
Closes #7454 

10 files changed, 235 insertions, 2 deletions

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 <filesystem | volume>
+ *
+ * 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 <[email protected]>. 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 <[email protected]>. 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 <[email protected]>. 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 <[email protected]>. 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"