Created attachment 202370 [details] don't check if a dataset is mounted when destroying a snapshot How to reproduce: $ bectl create bootenv $ bectl list -s | grep -A1 bootenv bootenv zroot/ROOT/bootenv - - 8K 2019-02-25 12:45 zroot/ROOT/solid@2019-02-25-12:45:25 - - 0 2019-02-25 12:45 $ bectl destroy -o bootenv cannot destroy mounted boot env unless forced $ bectl list | grep bootenv /* no output*/ $ zfs list -t snapshot | grep solid@2019-02-25-12:45:25 zroot/ROOT/solid@2019-02-25-12:45:25 0 - 21.7G - The bug is in the libbe library, specifically the 'be_destroy' function. 'be_destroy' can destroy a boot environment by name or snapshot, in either case 'be_destroy' acquires a zfs handle internally. When 'be_destroy' is given a boot environment name, the zfs handle will be the boot environment dataset (zroot/ROOT/bootenv). When 'be_destroy' is given a snapshot name, the zfs handle will be the dataset that the snapshot was taken from (zroot/ROOT/solid). After the zfs handle is acquired, a check is made to see if the dataset is mounted and bombs if it is (unless forced). In the above case, 'zroot/ROOT/solid' is the current boot environment. In short - when destroying a snapshot, 'be_destroy' tries to unmount the dataset that the snapshot was taken from. The attached fix checks if the dataset is mounted only when that dataset (i.e. boot environment) will be destroyed. In other words, the patch doesn't check if a dataset is mounted when destroying a snapshot.
*** Bug 236006 has been marked as a duplicate of this bug. ***
Take
A commit references this bug: Author: kevans Date: Mon Apr 1 17:44:21 UTC 2019 New revision: 345769 URL: https://svnweb.freebsd.org/changeset/base/345769 Log: libbe: Fix zfs_is_mounted check w/ snapshots 'be_destroy' can destroy a boot environment (by name) or a given snapshot. If the target to be destroyed is a dataset, check if it's mounted. We don't want to check if the origin dataset is mounted when destroying a snapshot. PR: 236043 Submitted by: Rob Fairbanks <rob.fx907 gmail com> MFC after: 1 week Differential Revision: https://reviews.freebsd.org/D19650 Changes: head/lib/libbe/be.c head/sbin/bectl/tests/bectl_test.sh
Committed, thanks!
A commit references this bug: Author: kevans Date: Mon Apr 8 17:41:40 UTC 2019 New revision: 346034 URL: https://svnweb.freebsd.org/changeset/base/346034 Log: MFC r343335, r343977, r343993-r343994, r344034, r344084, r345302, r345769 r343335: libbe(3): simplify import, allow replication streams Previously, we directly used libzfs_core's lzc_receive to import to a temporary snapshot, then cloned the snapshot and setup the properties. This failed when attempting to import replication streams with questionable error. libzfs's zfs_receive is a much better fit here, so we now use it instead with the destination dataset and let libzfs take care of the dirty details. be_import is greatly simplified as a result. r343977: libbe(3): Add a destroy option for removing the origin Currently origin snapshots are left behind when a BE is destroyed, whether it was an auto-created snapshot or explicitly specified via, for example, `bectl create -e be@mysnap ...`. Removing it automatically could be argued as a POLA violation in some circumstances, so provide a flag to be_destroy for it. An accompanying option will be added to bectl(8) to utilize this. Some minor style/consistency nits in the affected areas also addressed. r343993: bectl(8): Add -o flag to destroy to clean up the origin snapshot of BE We can't predict when destruction of origin is needed, and currently we have a precedent for not prompting for things. Leave the decision up to the user of bectl(8) if they want the origin snapshot to be destroyed or not. Emits a warning when -o isn't used and an origin snapshot is left to be cleaned up, for the time being. This is handy when one drops the -o flag but really did want to clean up the origin. A couple of -e ignore's have been sprinkled around the test suite for places that we don't care that the origin's not been cleaned up. -o functionality tests will be added in the future, but are omitted for now to reduce conflicts with work in flight to fix bits of the tests. r343994: bectl(8): commit missing test modifications from r343993 r344034: libbe(3): Belatedly note the BE_DESTROY_ORIGIN option added in r343977 r344084: libbe(3): Fix be_destroy behavior w.r.t. deep BE snapshots and -o be_destroy is documented to recursively destroy a boot environment. In the case of snapshots, one would take this to mean that these are also recursively destroyed. However, this was previously not the case. be_destroy would descend into the be_destroy callback and attempt to zfs_iter_children on the top-level snapshot, which is bogus. Our alternative approach is to take note of the snapshot name and iterate through all of fs children of the BE to try destruction in the children. The -o option is also fixed to work properly with deep BEs. If the BE was created with `bectl create -e otherDeepBE newDeepBE`, for instance, then a recursive snapshot of otherDeepBE would have been taken for construction of newDeepBE but a subsequent destroy with BE_DESTROY_ORIGIN set would only clean up the snapshot at the root of otherDeepBE: ${BEROOT}/otherDeepBE@... The most recent iteration instead pretends not to know how these things work, verifies that the origin is another BE and then passes that back through be_destroy to DTRT when snapshots and deep BEs may be in play. r345302: bectl(8): change jail command to execute jail(8) The jail(8) command provides a variety of jail pseudo-parameters that are useful to consumers of bectl, mount.devfs being the most-often-requested paramater by bectl users. command, exec.start, nopersist, and persist may not be specified via -o to bectl. The command/exec.start remains passed as it always has at the end of bectl, and persistence is dictated by -b/-U bectl jail arguments. r345769: libbe: Fix zfs_is_mounted check w/ snapshots 'be_destroy' can destroy a boot environment (by name) or a given snapshot. If the target to be destroyed is a dataset, check if it's mounted. We don't want to check if the origin dataset is mounted when destroying a snapshot. PR: 236043 Changes: _U stable/12/ stable/12/lib/libbe/be.c stable/12/lib/libbe/be.h stable/12/lib/libbe/be_error.c stable/12/lib/libbe/libbe.3 stable/12/sbin/bectl/bectl.8 stable/12/sbin/bectl/bectl.c stable/12/sbin/bectl/bectl_jail.c stable/12/sbin/bectl/tests/bectl_test.sh