zpool: fix redundancy check after vdev removal

The presence of indirect vdevs was confusing get_redundancy(), which
considered a pool with e.g. only mirror top-level vdevs and at least
one indirect vdev (due to the removal of a previous vdev) as already
having a broken redundancy, which is not the case. This lead to the
possibility of compromising the redundancy of a pool by adding
mismatched vdevs without requiring the use of `-f`, and with no
visible notice or warning.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Stéphane Lesimple <speed47_github@speed47.net>
Closes #13705
Closes #13711

Author: speed47

cmd/zpool/zpool_vdev.c

@@ -514,9 +514,14 @@ get_replication(nvlist_t *nvroot, boolean_t fatal)
 		if (is_log)
 			continue;
 
-		/* Ignore holes introduced by removing aux devices */
+		/*
+		 * Ignore holes introduced by removing aux devices, along
+		 * with indirect vdevs introduced by previously removed
+		 * vdevs.
+		 */
 		verify(nvlist_lookup_string(nv, ZPOOL_CONFIG_TYPE, &type) == 0);
-		if (strcmp(type, VDEV_TYPE_HOLE) == 0)
+		if (strcmp(type, VDEV_TYPE_HOLE) == 0 ||
+		    strcmp(type, VDEV_TYPE_INDIRECT) == 0)
 			continue;
 
 		if (nvlist_lookup_nvlist_array(nv, ZPOOL_CONFIG_CHILDREN,

tests/zfs-tests/tests/functional/removal/removal_with_indirect.ksh

@@ -0,0 +1,57 @@
+#! /bin/ksh -p
+#
+# CDDL HEADER START
+#
+# 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.
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright (c) 2014, 2018 by Delphix. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/removal/removal.kshlib
+
+TMPDIR=${TMPDIR:-$TEST_BASE_DIR}
+
+DISK1="$TMPDIR/dsk1"
+DISK2="$TMPDIR/dsk2"
+DISK3="$TMPDIR/dsk3"
+DISK4="$TMPDIR/dsk4"
+DISKS="$DISK1 $DISK2 $DISK3 $DISK4"
+
+log_must mkfile $(($MINVDEVSIZE * 2)) $DISK1
+log_must mkfile $(($MINVDEVSIZE * 2)) $DISK2
+log_must mkfile $(($MINVDEVSIZE * 2)) $DISK3
+log_must mkfile $(($MINVDEVSIZE * 2)) $DISK4
+
+function cleanup
+{
+	default_cleanup_noexit
+	log_must rm -f $DISKS
+}
+
+# Build a zpool with 2 mirror vdevs
+log_must default_setup_noexit "mirror $DISK1 $DISK2 mirror $DISK3 $DISK4"
+log_onexit cleanup
+
+# Remove one of the mirrors
+log_must zpool remove $TESTPOOL mirror-1
+log_must wait_for_removal $TESTPOOL
+
+# Attempt to add a single-device vdev, shouldn't work
+log_mustnot zpool add $TESTPOOL $DISK3
+
+# Force it, should work
+log_must zpool add -f $TESTPOOL $DISK3
+
+log_pass "Prevented from adding a non-mirror vdev on a mirrored zpool w/ indirect vdevs"

tests/zfs-tests/tests/functional/removal/remove_mirror.ksh

@@ -49,7 +49,8 @@ log_mustnot zpool remove $TESTPOOL $DISK2
 log_must wait_for_removal $TESTPOOL
 
 # Add back the first disk.
-log_must zpool add $TESTPOOL $DISK1
+# We use -f as we're adding a single vdev to zpool with only mirrors.
+log_must zpool add -f $TESTPOOL $DISK1
 
 # Now attempt to remove the mirror.
 log_must zpool remove $TESTPOOL mirror-1