9591 ms_shift can be incorrectly changed in MOS config for indirect vdevs that have been historically expanded

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: George Wilson <gwilson@zfsmail.com>
Reviewed by: John Kennedy <john.kennedy@delphix.com>
Reviewed by: Prashanth Sreenivasa <pks@delphix.com>
Reviewed by: Tim Chase <tim@chase2k.com>
Approved by: Richard Lowe <richlowe@richlowe.net>

5 files changed, 109 insertions, 52 deletions

diff --git a/usr/src/pkg/manifests/system-test-zfstest.mf b/usr/src/pkg/manifests/system-test-zfstest.mf
index 140fc1221e..681c719275 100644
--- a/usr/src/pkg/manifests/system-test-zfstest.mf
+++ b/usr/src/pkg/manifests/system-test-zfstest.mf
@@ -2365,7 +2365,7 @@ file path=opt/zfs-tests/tests/functional/refreserv/refreserv_004_pos mode=0555
 file path=opt/zfs-tests/tests/functional/refreserv/refreserv_005_pos mode=0555
 file path=opt/zfs-tests/tests/functional/refreserv/setup mode=0555
 file path=opt/zfs-tests/tests/functional/removal/cleanup mode=0555
-file path=opt/zfs-tests/tests/functional/removal/removal.kshlib mode=0555
+file path=opt/zfs-tests/tests/functional/removal/removal.kshlib mode=0444
 file path=opt/zfs-tests/tests/functional/removal/removal_all_vdev mode=0555
 file path=opt/zfs-tests/tests/functional/removal/removal_check_space mode=0555
 file path=opt/zfs-tests/tests/functional/removal/removal_condense_export \
@@ -2396,6 +2396,7 @@ file path=opt/zfs-tests/tests/functional/removal/removal_with_snapshot \
     mode=0555
 file path=opt/zfs-tests/tests/functional/removal/removal_with_write mode=0555
 file path=opt/zfs-tests/tests/functional/removal/removal_with_zdb mode=0555
+file path=opt/zfs-tests/tests/functional/removal/remove_expanded mode=0555
 file path=opt/zfs-tests/tests/functional/removal/remove_mirror mode=0555
 file path=opt/zfs-tests/tests/functional/removal/remove_mirror_sanity \
     mode=0555
diff --git a/usr/src/test/zfs-tests/tests/functional/removal/Makefile b/usr/src/test/zfs-tests/tests/functional/removal/Makefile
index b4b9fa0031..f889ceb372 100644
--- a/usr/src/test/zfs-tests/tests/functional/removal/Makefile
+++ b/usr/src/test/zfs-tests/tests/functional/removal/Makefile
@@ -10,57 +10,12 @@
 #
 
 #
-# Copyright (c) 2014, 2017 by Delphix. All rights reserved.
+# Copyright (c) 2014, 2018 by Delphix. All rights reserved.
 #
 
 include $(SRC)/Makefile.master
 
 ROOTOPTPKG = $(ROOT)/opt/zfs-tests
-TESTDIR = $(ROOTOPTPKG)/tests/functional/removal
+TARGETDIR = $(ROOTOPTPKG)/tests/functional/removal
 
-PROGS = cleanup \
-        removal_all_vdev \
-        removal_check_space \
-        removal_condense_export \
-        removal_multiple_indirection \
-        removal_remap \
-        removal_reservation \
-        removal_sanity \
-        removal_remap_deadlists \
-        removal_with_add \
-        removal_with_create_fs \
-        removal_with_dedup \
-        removal_with_export \
-        removal_with_ganging \
-        removal_with_remap \
-        removal_with_remove \
-        removal_with_scrub \
-        removal_with_send \
-        removal_with_send_recv \
-        removal_with_snapshot \
-        removal_with_write \
-        removal_with_zdb \
-        removal_resume_export \
-        remove_mirror \
-        remove_mirror_sanity \
-        remove_raidz
-
-FILES = removal.kshlib
-
-CMDS = $(PROGS:%=$(TESTDIR)/%) $(FILES:%=$(TESTDIR)/%)
-$(CMDS) := FILEMODE = 0555
-
-all lint clean clobber:
-
-install: $(CMDS)
-
-$(CMDS): $(TESTDIR)
-
-$(TESTDIR):
-	$(INS.dir)
-
-$(TESTDIR)/%: %.ksh
-	$(INS.rename)
-
-$(TESTDIR)/%: %
-	$(INS.file)
+include $(SRC)/test/zfs-tests/Makefile.com
diff --git a/usr/src/test/zfs-tests/tests/functional/removal/remove_expanded.ksh b/usr/src/test/zfs-tests/tests/functional/removal/remove_expanded.ksh
new file mode 100644
index 0000000000..9beb84e591
--- /dev/null
+++ b/usr/src/test/zfs-tests/tests/functional/removal/remove_expanded.ksh
@@ -0,0 +1,89 @@
+#! /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) 2018 by Delphix. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/removal/removal.kshlib
+
+#
+# BACKGROUND:
+#
+# ztest hit an issue where it ran zdb and zdb failed because
+# it couldn't access some indirect mappings at the end of a
+# vdev. The issue was that the vdev's ms_shift had changed after
+# it was removed by the addition of another vdev. This test is
+# a regression test for ensuring this case doesn't come up again.
+#
+
+
+TMPDIR=${TMPDIR:-/tmp}
+DISK0=$TMPDIR/dsk0
+DISK1=$TMPDIR/dsk1
+DISK2=$TMPDIR/dsk2
+
+log_must mkfile -n $MINVDEVSIZE $DISK0
+log_must mkfile -n $(($MINVDEVSIZE * 3)) $DISK1
+log_must mkfile -n $MINVDEVSIZE $DISK2
+
+function cleanup
+{
+	default_cleanup_noexit
+	log_must rm -f $DISK0 $DISK1 $DISK2
+}
+
+#
+# Setup the pool with one disk .
+#
+log_must default_setup_noexit "$DISK0"
+log_onexit cleanup
+
+#
+# Expand vdev.
+#
+log_must truncate -s $(($MINVDEVSIZE * 2)) $DISK0
+log_must zpool reopen $TESTPOOL
+log_must zpool online -e $TESTPOOL $DISK0
+
+#
+# Fill up the whole vdev.
+#
+dd if=/dev/urandom of=$TESTDIR/$TESTFILE0 bs=8M
+
+#
+# Add another vdev and remove the first vdev creating indirect
+# mappings for nearly all the allocatable space from the first
+# vdev. Wait for removal to finish.
+#
+log_must zpool add $TESTPOOL $DISK1
+log_must zpool remove $TESTPOOL $DISK0
+log_must wait_for_removal $TESTPOOL
+
+#
+# Add a new vdev that will trigger a change in the config.
+# Run sync once to ensure that the config actually changed.
+#
+log_must zpool add $TESTPOOL $DISK2
+log_must sync
+
+#
+# Ensure that zdb does not find any problems with this.
+#
+log_must zdb $TESTPOOL
+
+log_pass "Removal of expanded vdev doesn't cause any problems."
diff --git a/usr/src/uts/common/fs/zfs/spa_config.c b/usr/src/uts/common/fs/zfs/spa_config.c
index 1fe675337f..7d568ffcf8 100644
--- a/usr/src/uts/common/fs/zfs/spa_config.c
+++ b/usr/src/uts/common/fs/zfs/spa_config.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, 2015 by Delphix. All rights reserved.
+ * Copyright (c) 2011, 2018 by Delphix. All rights reserved.
  * Copyright 2017 Joyent, Inc.
  */
 
@@ -510,6 +510,18 @@ spa_config_update(spa_t *spa, int what)
 		 */
 		for (c = 0; c < rvd->vdev_children; c++) {
 			vdev_t *tvd = rvd->vdev_child[c];
+
+			/*
+			 * Explicitly skip vdevs that are indirect or
+			 * log vdevs that are being removed. The reason
+			 * is that both of those can have vdev_ms_array
+			 * set to 0 and we wouldn't want to change their
+			 * metaslab size nor call vdev_expand() on them.
+			 */
+			if (!vdev_is_concrete(tvd) ||
+			    (tvd->vdev_islog && tvd->vdev_removing))
+				continue;
+
 			if (tvd->vdev_ms_array == 0)
 				vdev_metaslab_set_size(tvd);
 			vdev_expand(tvd, txg);
diff --git a/usr/src/uts/common/fs/zfs/vdev.c b/usr/src/uts/common/fs/zfs/vdev.c
index 43c6e4c9a1..f6845f3dcb 100644
--- a/usr/src/uts/common/fs/zfs/vdev.c
+++ b/usr/src/uts/common/fs/zfs/vdev.c
@@ -4086,11 +4086,11 @@ vdev_expand(vdev_t *vd, uint64_t txg)
 {
 	ASSERT(vd->vdev_top == vd);
 	ASSERT(spa_config_held(vd->vdev_spa, SCL_ALL, RW_WRITER) == SCL_ALL);
+	ASSERT(vdev_is_concrete(vd));
 
 	vdev_set_deflate_ratio(vd);
 
-	if ((vd->vdev_asize >> vd->vdev_ms_shift) > vd->vdev_ms_count &&
-	    vdev_is_concrete(vd)) {
+	if ((vd->vdev_asize >> vd->vdev_ms_shift) > vd->vdev_ms_count) {
 		VERIFY(vdev_metaslab_init(vd, txg) == 0);
 		vdev_config_dirty(vd);
 	}