Always continue recursive destroy after error

Currently, during a recursive zfs destroy the first error that is
encountered will stop the destruction of the datasets. Errors may
happen for a variety of reasons including competing deletions
and busy datasets.
This patch switches recursive destroy to always do a best-effort
recursive dataset destroy.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Signed-off-by: Alek Pinchuk <apinchuk@datto.com>
Closes #7574

diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c
index 7d81dcaee235b1e7d4f02606c28c82410e088de6..b45e93c3fb92a87c037483f717a5d905f2c761c5 100644 (file)
--- a/cmd/zfs/zfs_main.c
+++ b/cmd/zfs/zfs_main.c
@@ -1189,6 +1189,15 @@ destroy_callback(zfs_handle_t *zhp, void *data)
                    zfs_unmount(zhp, NULL, cb->cb_force ? MS_FORCE : 0) != 0 ||
                    zfs_destroy(zhp, cb->cb_defer_destroy) != 0) {
                        zfs_close(zhp);
+                       /*
+                        * When performing a recursive destroy we ignore errors
+                        * so that the recursive destroy could continue
+                        * destroying past problem datasets
+                        */
+                       if (cb->cb_recurse) {
+                               cb->cb_error = B_TRUE;
+                               return (0);
+                       }
                        return (-1);
                }
        }
@@ -1558,7 +1567,7 @@ zfs_do_destroy(int argc, char **argv)
                        err = zfs_destroy_snaps_nvl(g_zfs,
                            cb.cb_batchedsnaps, cb.cb_defer_destroy);
                }
-               if (err != 0)
+               if (err != 0 || cb.cb_error == B_TRUE)
                        rv = 1;
        }
 

diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_destroy/zfs_destroy_005_neg.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_destroy/zfs_destroy_005_neg.ksh
index b58f7b325c79b96a1b04aa93de8cd0de13ba91c7..2e4a0c3b2bb5fd3c1405b89db8210a6a8d32fc5b 100755 (executable)
--- a/tests/zfs-tests/tests/functional/cli_root/zfs_destroy/zfs_destroy_005_neg.ksh
+++ b/tests/zfs-tests/tests/functional/cli_root/zfs_destroy/zfs_destroy_005_neg.ksh
@@ -27,6 +27,7 @@
 
 #
 # Copyright (c) 2012, 2016 by Delphix. All rights reserved.
+# Copyright (c) 2018 Datto Inc.
 #
 
 . $STF_SUITE/include/libtest.shlib
@@ -106,24 +107,11 @@ log_note "mkbusy $mntpt/$TESTFILE0 (pidlist: $pidlist)"
 [[ -z $pidlist ]] && log_fail "Failure from mkbusy"
 negative_test "-R -rR" $CTR
 
-#
-# Checking the outcome of the test above is tricky, because the order in
-# which datasets are destroyed is not deterministic. Both $FS and $VOL are
-# busy, and the remaining datasets will be different depending on whether we
-# tried (and failed) to delete $FS or $VOL first.
-
-# The following datasets will exist independent of the order
+# The following busy datasets will still exist
 check_dataset datasetexists $CTR $FS $VOL
 
-if datasetexists $VOLSNAP && datasetnonexists $FSSNAP; then
-       # The recursive destroy failed on $FS
-       check_dataset datasetnonexists $FSSNAP $FSCLONE
-       check_dataset datasetexists $VOLSNAP $VOLCLONE
-elif datasetexists $FSSNAP && datasetnonexists $VOLSNAP; then
-       # The recursive destroy failed on $VOL
-       check_dataset datasetnonexists $VOLSNAP $VOLCLONE
-       check_dataset datasetexists $FSSNAP $FSCLONE
-else
+# The following datasets will not exist because of best-effort recursive destroy
+if datasetexists $VOLSNAP || datasetexists $FSSNAP; then
        log_must zfs list -rtall
        log_fail "Unexpected datasets remaining"
 fi
@@ -157,15 +145,9 @@ if is_global_zone; then
        check_dataset datasetexists $CTR $VOL
        check_dataset datasetnonexists $VOLSNAP $VOLCLONE
 
-       # Here again, the non-determinism of destroy order is a factor. $FS,
-       # $FSSNAP and $FSCLONE will still exist here iff we attempted to destroy
-       # $VOL (and failed) first. So check that either all of the datasets are
-       # present, or they're all gone.
-       if datasetexists $FS; then
-               check_dataset datasetexists $FS $FSSNAP $FSCLONE
-       else
-               check_dataset datasetnonexists $FS $FSSNAP $FSCLONE
-       fi
+       # Due to recusive destroy being a best-effort operation,
+       # all of the non-busy datasets bellow should be gone now.
+       check_dataset datasetnonexists $FS $FSSNAP $FSCLONE
 fi
 
 #