]> git.proxmox.com Git - mirror_zfs.git/commitdiff
Improve `zpool labelclear`
authorBrian Behlendorf <behlendorf1@llnl.gov>
Thu, 21 Mar 2019 17:13:01 +0000 (10:13 -0700)
committerGitHub <noreply@github.com>
Thu, 21 Mar 2019 17:13:01 +0000 (10:13 -0700)
1) As implemented the `zpool labelclear` command overwrites
the calculated offsets of all four vdev labels even when only a
single valid label is found.  If the device as been re-purposed
but still contains a valid label this can result in space no
longer owned by ZFS being zeroed.  Prevent this by verifying
every label removed is intact before it's overwritten.

2) Address a small bug in zpool_do_labelclear() which prevented
labelclear from working on file vdevs.  Only block devices support
BLKFLSBUF, try the ioctl() but when it's reported as unsupported
this should not be fatal.

3) Fix `zpool labelclear` so it can be run on vdevs which were
removed from the pool with `zpool remove`.  Additionally, allow
intact but partial labels to be cleared as in the case of a failed
`zpool attach` or `zpool replace`.

4) Remove LABELCLEAR and LABELREAD variables for test cases.

Reviewed-by: Matt Ahrens <mahrens@delphix.com>
Reviewed-by: Tim Chase <tim@chase2k.com>
Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #8500
Closes #8373
Closes #6261

12 files changed:
cmd/zpool/zpool_main.c
lib/libzfs/libzfs_import.c
tests/runfiles/linux.run
tests/zfs-tests/tests/functional/cli_root/zpool_attach/attach-o_ashift.ksh
tests/zfs-tests/tests/functional/cli_root/zpool_labelclear/Makefile.am
tests/zfs-tests/tests/functional/cli_root/zpool_labelclear/labelclear.cfg
tests/zfs-tests/tests/functional/cli_root/zpool_labelclear/zpool_labelclear_active.ksh
tests/zfs-tests/tests/functional/cli_root/zpool_labelclear/zpool_labelclear_exported.ksh
tests/zfs-tests/tests/functional/cli_root/zpool_labelclear/zpool_labelclear_removed.ksh [new file with mode: 0755]
tests/zfs-tests/tests/functional/cli_root/zpool_labelclear/zpool_labelclear_valid.ksh [new file with mode: 0755]
tests/zfs-tests/tests/functional/cli_root/zpool_replace/replace-o_ashift.ksh
tests/zfs-tests/tests/functional/cli_root/zpool_replace/replace_prop_ashift.ksh

index 61403a173cbf84786ee0d9300bd7e3e441847400..3607656e0d5fe09bed454b9bfb2b2d481b79ebdd 100644 (file)
@@ -1113,13 +1113,18 @@ zpool_do_labelclear(int argc, char **argv)
                return (1);
        }
 
-       if (ioctl(fd, BLKFLSBUF) != 0)
+       /*
+        * Flush all dirty pages for the block device.  This should not be
+        * fatal when the device does not support BLKFLSBUF as would be the
+        * case for a file vdev.
+        */
+       if ((ioctl(fd, BLKFLSBUF) != 0) && (errno != ENOTTY))
                (void) fprintf(stderr, gettext("failed to invalidate "
                    "cache for %s: %s\n"), vdev, strerror(errno));
 
-       if (zpool_read_label(fd, &config, NULL) != 0 || config == NULL) {
+       if (zpool_read_label(fd, &config, NULL) != 0) {
                (void) fprintf(stderr,
-                   gettext("failed to check state for %s\n"), vdev);
+                   gettext("failed to read label from %s\n"), vdev);
                ret = 1;
                goto errout;
        }
index 44a46d1cea6f740ed6c94aa1c4894657a68f02ff..3d7a0bf12a05582933d624ac9e3c432f0f2cc99b 100644 (file)
@@ -148,23 +148,66 @@ zpool_clear_label(int fd)
        int l;
        vdev_label_t *label;
        uint64_t size;
+       int labels_cleared = 0;
 
        if (fstat64_blk(fd, &statbuf) == -1)
                return (0);
+
        size = P2ALIGN_TYPED(statbuf.st_size, sizeof (vdev_label_t), uint64_t);
 
        if ((label = calloc(1, sizeof (vdev_label_t))) == NULL)
                return (-1);
 
        for (l = 0; l < VDEV_LABELS; l++) {
-               if (pwrite64(fd, label, sizeof (vdev_label_t),
+               uint64_t state, guid;
+               nvlist_t *config;
+
+               if (pread64(fd, label, sizeof (vdev_label_t),
                    label_offset(size, l)) != sizeof (vdev_label_t)) {
-                       free(label);
-                       return (-1);
+                       continue;
+               }
+
+               if (nvlist_unpack(label->vl_vdev_phys.vp_nvlist,
+                   sizeof (label->vl_vdev_phys.vp_nvlist), &config, 0) != 0) {
+                       continue;
+               }
+
+               /* Skip labels which do not have a valid guid. */
+               if (nvlist_lookup_uint64(config, ZPOOL_CONFIG_GUID,
+                   &guid) != 0 || guid == 0) {
+                       nvlist_free(config);
+                       continue;
+               }
+
+               /* Skip labels which are not in a known valid state. */
+               if (nvlist_lookup_uint64(config, ZPOOL_CONFIG_POOL_STATE,
+                   &state) != 0 || state > POOL_STATE_L2CACHE) {
+                       nvlist_free(config);
+                       continue;
+               }
+
+               nvlist_free(config);
+
+               /*
+                * A valid label was found, overwrite this label's nvlist
+                * and uberblocks with zeros on disk.  This is done to prevent
+                * system utilities, like blkid, from incorrectly detecting a
+                * partial label.  The leading pad space is left untouched.
+                */
+               memset(label, 0, sizeof (vdev_label_t));
+               size_t label_size = sizeof (vdev_label_t) - (2 * VDEV_PAD_SIZE);
+
+               if (pwrite64(fd, label, label_size, label_offset(size, l) +
+                   (2 * VDEV_PAD_SIZE)) == label_size) {
+                       labels_cleared++;
                }
        }
 
        free(label);
+
+       if (labels_cleared == 0)
+               return (-1);
+
        return (0);
 }
 
index 93f0c03aacd7a9ab91a03682212eae45d6b58e64..9537798a9269f028ea6844d283ea8d108a81b5cc 100644 (file)
@@ -392,7 +392,8 @@ tests = ['zpool_import_001_pos', 'zpool_import_002_pos',
 tags = ['functional', 'cli_root', 'zpool_import']
 
 [tests/functional/cli_root/zpool_labelclear]
-tests = ['zpool_labelclear_active', 'zpool_labelclear_exported']
+tests = ['zpool_labelclear_active', 'zpool_labelclear_exported',
+    'zpool_labelclear_removed', 'zpool_labelclear_valid']
 pre =
 post =
 tags = ['functional', 'cli_root', 'zpool_labelclear']
index 58ab57db05a669cbfd705624333c60ebaa673b1e..fd33fb9506a9068142dee70afbb4701c86dffa82 100755 (executable)
@@ -51,8 +51,8 @@ log_onexit cleanup
 
 disk1=$TEST_BASE_DIR/$FILEDISK0
 disk2=$TEST_BASE_DIR/$FILEDISK1
-log_must mkfile $SIZE $disk1
-log_must mkfile $SIZE $disk2
+log_must truncate -s $SIZE $disk1
+log_must truncate -s $SIZE $disk2
 
 typeset ashifts=("9" "10" "11" "12" "13" "14" "15" "16")
 for ashift in ${ashifts[@]}
@@ -84,13 +84,7 @@ do
                # clean things for the next run
                log_must zpool destroy $TESTPOOL1
                log_must zpool labelclear $disk1
-               # depending on if we expect to have failed the 'zpool attach'
-               if [[ $cmdval -le $ashift ]]
-               then
-                       log_must zpool labelclear $disk2
-               else
-                       log_mustnot zpool labelclear $disk2
-               fi
+               log_must zpool labelclear $disk2
        done
 done
 
index dc4a0277c6e69817c2c7e9c945a74779d2e9ea67..c258f0c929d4340b48b2e5bb098d370d9091403f 100644 (file)
@@ -1,7 +1,9 @@
 pkgdatadir = $(datadir)/@PACKAGE@/zfs-tests/tests/functional/cli_root/zpool_labelclear
 dist_pkgdata_SCRIPTS = \
        zpool_labelclear_active.ksh \
-       zpool_labelclear_exported.ksh
+       zpool_labelclear_exported.ksh \
+       zpool_labelclear_removed.ksh \
+       zpool_labelclear_valid.ksh
 
 dist_pkgdata_DATA = \
        labelclear.cfg
index 8bf038270399502ccadcc914c8585fa9780df8b1..85148d6e856193decfcbdeefa211c1d8bd13a3ea 100644 (file)
@@ -15,9 +15,6 @@
 
 . $STF_SUITE/include/libtest.shlib
 
-typeset LABELCLEAR="zpool labelclear"
-typeset LABELREAD="zdb -lq"
-
 typeset disks=(${DISKS[*]})
 typeset disk1=${disks[0]}
 typeset disk2=${disks[1]}
index 8a30d1a422959a5948b93fbe1a5ca465ee709a25..dcca2e9335d669e8e56997c594105230dc912f1a 100755 (executable)
@@ -43,26 +43,26 @@ log_assert "zpool labelclear will fail on all vdevs of imported pool"
 log_must zpool create -O mountpoint=none -f $TESTPOOL $disk1 log $disk2
 
 # Check that labelclear [-f] will fail on ACTIVE pool vdevs
-log_mustnot $LABELCLEAR $disk1
-log_must $LABELREAD $disk1
-log_mustnot $LABELCLEAR -f $disk1
-log_must $LABELREAD $disk1
-log_mustnot $LABELCLEAR $disk2
-log_must $LABELREAD $disk2
-log_mustnot $LABELCLEAR -f $disk2
-log_must $LABELREAD $disk2
+log_mustnot zpool labelclear $disk1
+log_must zdb -lq $disk1
+log_mustnot zpool labelclear -f $disk1
+log_must zdb -lq $disk1
+log_mustnot zpool labelclear $disk2
+log_must zdb -lq $disk2
+log_mustnot zpool labelclear -f $disk2
+log_must zdb -lq $disk2
 
 # Add a cache/spare to the pool, check that labelclear [-f] will fail
 # on the vdev and will succeed once it's removed from pool config
 for vdevtype in "cache" "spare"; do
        log_must zpool add $TESTPOOL $vdevtype $disk3
-       log_mustnot $LABELCLEAR $disk3
-       log_must $LABELREAD $disk3
-       log_mustnot $LABELCLEAR -f $disk3
-       log_must $LABELREAD $disk3
+       log_mustnot zpool labelclear $disk3
+       log_must zdb -lq $disk3
+       log_mustnot zpool labelclear -f $disk3
+       log_must zdb -lq $disk3
        log_must zpool remove $TESTPOOL $disk3
-       log_must $LABELCLEAR $disk3
-       log_mustnot $LABELREAD $disk3
+       log_must zpool labelclear $disk3
+       log_mustnot zdb -lq $disk3
 done
 
 log_pass "zpool labelclear will fail on all vdevs of imported pool"
index 98385c9b71e0efa06611a7963df88c3ec82f4056..a5131bdbb78b9a432e3ea08ff4a98a6c1be96f49 100755 (executable)
@@ -52,21 +52,21 @@ for vdevtype in "" "cache" "spare"; do
        log_must zpool export $TESTPOOL
 
        # Check that labelclear will fail without -f
-       log_mustnot $LABELCLEAR $disk1
-       log_must $LABELREAD $disk1
-       log_mustnot $LABELCLEAR $disk2
-       log_must $LABELREAD $disk2
+       log_mustnot zpool labelclear $disk1
+       log_must zdb -lq $disk1
+       log_mustnot zpool labelclear $disk2
+       log_must zdb -lq $disk2
 
        # Check that labelclear will succeed with -f
-       log_must $LABELCLEAR -f $disk1
-       log_mustnot $LABELREAD $disk1
-       log_must $LABELCLEAR -f $disk2
-       log_mustnot $LABELREAD $disk2
+       log_must zpool labelclear -f $disk1
+       log_mustnot zdb -lq $disk1
+       log_must zpool labelclear -f $disk2
+       log_mustnot zdb -lq $disk2
 
        # Check that labelclear on auxilary vdevs will succeed
        if [[ -n $vdevtype ]]; then
-               log_must $LABELCLEAR $disk3
-               log_mustnot $LABELREAD $disk3
+               log_must zpool labelclear $disk3
+               log_mustnot zdb -lq $disk3
        fi
 done
 
diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_labelclear/zpool_labelclear_removed.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_labelclear/zpool_labelclear_removed.ksh
new file mode 100755 (executable)
index 0000000..f93de6e
--- /dev/null
@@ -0,0 +1,62 @@
+#!/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 2016 Nexenta Systems, Inc.
+# Copyright (c) 2019 by Lawrence Livermore National Security, LLC.
+#
+
+. $STF_SUITE/tests/functional/cli_root/zpool_labelclear/labelclear.cfg
+
+# DESCRIPTION:
+#    Check that `zpool labelclear` can clear labels on removed devices.
+#
+# STRATEGY:
+# 1. Create a pool with primary, log, spare and cache devices.
+# 2. Remove a top-level vdev, log, spare, and cache device.
+# 3. Run `zpool labelclear` on the removed device.
+# 4. Verify the label has been removed.
+#
+
+verify_runnable "global"
+
+function cleanup
+{
+       poolexists $TESTPOOL && destroy_pool $TESTPOOL
+       rm -f $DEVICE1 $DEVICE2 $DEVICE3 $DEVICE4 $DEVICE5
+}
+
+log_onexit cleanup
+log_assert "zpool labelclear works for removed devices"
+
+DEVICE1="$TEST_BASE_DIR/device-1"
+DEVICE2="$TEST_BASE_DIR/device-2"
+DEVICE3="$TEST_BASE_DIR/device-3"
+DEVICE4="$TEST_BASE_DIR/device-4"
+DEVICE5="$TEST_BASE_DIR/device-5"
+
+log_must truncate -s $((SPA_MINDEVSIZE * 8)) $DEVICE1
+log_must truncate -s $SPA_MINDEVSIZE $DEVICE2 $DEVICE3 $DEVICE4 $DEVICE5
+
+log_must zpool create -f $TESTPOOL $DEVICE1 $DEVICE2 \
+    log $DEVICE3 cache $DEVICE4 spare $DEVICE5
+log_must zpool sync
+
+# Remove each type of vdev and verify the label can be cleared.
+for dev in $DEVICE5 $DEVICE4 $DEVICE3 $DEVICE2; do
+       log_must zpool remove $TESTPOOL $dev
+       log_must zpool sync $TESTPOOL
+       log_must zpool labelclear $dev
+       log_mustnot zdb -lq $dev
+done
+
+log_pass "zpool labelclear works for removed devices"
diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_labelclear/zpool_labelclear_valid.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_labelclear/zpool_labelclear_valid.ksh
new file mode 100755 (executable)
index 0000000..211829d
--- /dev/null
@@ -0,0 +1,91 @@
+#!/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 2016 Nexenta Systems, Inc.
+# Copyright (c) 2019 by Lawrence Livermore National Security, LLC.
+#
+
+. $STF_SUITE/tests/functional/cli_root/zpool_labelclear/labelclear.cfg
+
+# DESCRIPTION:
+#    Check that `zpool labelclear` only clears valid labels.  Expected
+#    label offsets which do not contain intact labels are left untouched.
+#
+# STRATEGY:
+# 1. Create a pool with primary, log, spare and cache devices.
+# 2. Export the pool.
+# 3. Write a known pattern over the first two device labels.
+# 4. Verify with zdb that only the last two device labels are intact.
+# 5. Verify the pool could be imported using those labels.
+# 6. Run `zpool labelclear` to destroy those last two labels.
+# 7. Verify the pool can no longer be found; let alone imported.
+# 8. Verify the pattern is intact to confirm `zpool labelclear` did
+#    not write to first two label offsets.
+# 9. Verify that no valid label remain.
+#
+
+verify_runnable "global"
+
+function cleanup
+{
+       poolexists $TESTPOOL && destroy_pool $TESTPOOL
+       rm -f $PATTERN_FILE $DEVICE1 $DEVICE2 $DEVICE3 $DEVICE4
+}
+
+log_onexit cleanup
+log_assert "zpool labelclear will only clear valid labels"
+
+PATTERN_FILE=$TEST_BASE_DIR/pattern
+
+DEVICE1="$TEST_BASE_DIR/device-1"
+DEVICE2="$TEST_BASE_DIR/device-2"
+DEVICE3="$TEST_BASE_DIR/device-3"
+DEVICE4="$TEST_BASE_DIR/device-4"
+
+log_must dd if=/dev/urandom of=$PATTERN_FILE bs=1048576 count=4
+
+log_must truncate -s $SPA_MINDEVSIZE $DEVICE1 $DEVICE2 $DEVICE3 $DEVICE4
+
+log_must zpool create -O mountpoint=none -f $TESTPOOL $DEVICE1 \
+     log $DEVICE2 cache $DEVICE3 spare $DEVICE4
+log_must zpool export $TESTPOOL
+
+# Overwrite the first 4M of each device and verify the expected labels.
+for dev in $DEVICE1 $DEVICE2 $DEVICE3 $DEVICE4; do
+       dd if=$PATTERN_FILE of=$dev bs=1048576 conv=notrunc
+       log_must eval "zdb -l $dev | grep 'labels = 2 3'"
+done
+
+# Verify the pool could be imported using those labels.
+log_must eval "zpool import -d $TEST_BASE_DIR | grep $TESTPOOL"
+
+# Verify the last two labels on each vdev can be cleared.
+for dev in $DEVICE1 $DEVICE2 $DEVICE3 $DEVICE4; do
+       log_must zpool labelclear -f $dev
+done
+
+# Verify there is no longer a pool which can be imported.
+log_mustnot eval "zpool import -d $TEST_BASE_DIR | grep $TESTPOOL"
+
+# Verify the original pattern over the first two labels is intact
+for dev in $DEVICE1 $DEVICE2 $DEVICE3 $DEVICE4; do
+       log_must cmp -n $((4 * 1048576)) $dev $PATTERN_FILE
+       log_mustnot zdb -lq $dev
+done
+
+# Verify an error is reported when there are no labels to clear.
+for dev in $DEVICE1 $DEVICE2 $DEVICE3 $DEVICE4; do
+       log_mustnot zpool labelclear -f $dev
+done
+
+log_pass "zpool labelclear will only clear valid labels"
index 77f85c6bea706fa66100f5e5da698f56cbb358d8..ae415487c7fb268f7e755e4b1aa55bee97fe9804 100755 (executable)
@@ -51,8 +51,8 @@ log_onexit cleanup
 
 disk1=$TEST_BASE_DIR/$FILEDISK0
 disk2=$TEST_BASE_DIR/$FILEDISK1
-log_must mkfile $SIZE $disk1
-log_must mkfile $SIZE $disk2
+log_must truncate -s $SIZE $disk1
+log_must truncate -s $SIZE $disk2
 
 typeset ashifts=("9" "10" "11" "12" "13" "14" "15" "16")
 for ashift in ${ashifts[@]}
@@ -84,15 +84,8 @@ do
                fi
                # clean things for the next run
                log_must zpool destroy $TESTPOOL1
-               # depending on if we expect to have failed the 'zpool replace'
-               if [[ $cmdval -le $ashift ]]
-               then
-                       log_mustnot zpool labelclear $disk1
-                       log_must zpool labelclear $disk2
-               else
-                       log_must zpool labelclear $disk1
-                       log_mustnot zpool labelclear $disk2
-               fi
+               log_must zpool labelclear $disk1
+               log_must zpool labelclear $disk2
        done
 done
 
index 714f1180f578ffc4ed7f55f52f1a4fcc0296e6ae..e740de133a50eeb47befa4a273818850f067029a 100755 (executable)
@@ -52,8 +52,8 @@ log_onexit cleanup
 
 disk1=$TEST_BASE_DIR/$FILEDISK0
 disk2=$TEST_BASE_DIR/$FILEDISK1
-log_must mkfile $SIZE $disk1
-log_must mkfile $SIZE $disk2
+log_must truncate -s $SIZE $disk1
+log_must truncate -s $SIZE $disk2
 
 typeset ashifts=("9" "10" "11" "12" "13" "14" "15" "16")
 for ashift in ${ashifts[@]}
@@ -89,7 +89,7 @@ do
                fi
                # clean things for the next run
                log_must zpool destroy $TESTPOOL1
-               log_mustnot zpool labelclear $disk1
+               log_must zpool labelclear $disk1
                log_must zpool labelclear $disk2
        done
 done