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 <behlendorf1@llnl.gov>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
Closes #7454

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);

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

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);

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]

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 \

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

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 <ezomori.nozomu@gmail.com>. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+
+default_cleanup

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 <ezomori.nozomu@gmail.com>. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib

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 <ezomori.nozomu@gmail.com>. 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"

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 <ezomori.nozomu@gmail.com>. 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"