From 6969afcefdfb49fb9c0fcf56240e6eb133a2c4a8 Mon Sep 17 00:00:00 2001 From: Alek P Date: Wed, 6 Jun 2018 13:14:52 -0400 Subject: [PATCH] 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 Reviewed by: Matthew Ahrens Signed-off-by: Alek Pinchuk Closes #7574 --- cmd/zfs/zfs_main.c | 11 ++++++- .../zfs_destroy/zfs_destroy_005_neg.ksh | 32 ++++--------------- 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c index 7d81dcaee..b45e93c3f 100644 --- 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 b58f7b325..2e4a0c3b2 100755 --- 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 #