svn commit: r356279 - in head: lib/libbe sbin/bectl/tests

Kyle Evans kevans at FreeBSD.org
Thu Jan 2 18:46:35 UTC 2020


Author: kevans
Date: Thu Jan  2 18:46:33 2020
New Revision: 356279
URL: https://svnweb.freebsd.org/changeset/base/356279

Log:
  libbe(3): promote dependent clones when destroying an environment
  
  When removing a boot environment iterate over the dependents and process the
  snapshots by grabbing any clones. Promote the clones we found and then
  remove the target environment.
  
  This fixes the ability to destroy a boot environment when it has been used
  to spawn one or more other boot environments.
  
  PR:		242592
  Submitted by:	Wes Maag <jwmaag gmail com> (with changes by myself)
  MFC after:	1 week
  Differential Revision:	https://reviews.freebsd.org/D22953

Modified:
  head/lib/libbe/be.c
  head/lib/libbe/be.h
  head/lib/libbe/be_error.c
  head/sbin/bectl/tests/bectl_test.sh

Modified: head/lib/libbe/be.c
==============================================================================
--- head/lib/libbe/be.c	Thu Jan  2 17:51:49 2020	(r356278)
+++ head/lib/libbe/be.c	Thu Jan  2 18:46:33 2020	(r356279)
@@ -33,7 +33,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/mount.h>
 #include <sys/stat.h>
 #include <sys/ucred.h>
-
+#include <sys/queue.h>
 #include <sys/zfs_context.h>
 #include <sys/mntent.h>
 
@@ -48,9 +48,16 @@ __FBSDID("$FreeBSD$");
 #include "be.h"
 #include "be_impl.h"
 
+struct promote_entry {
+	char				name[BE_MAXPATHLEN];
+	SLIST_ENTRY(promote_entry)	link;
+};
+
 struct be_destroy_data {
-	libbe_handle_t		*lbh;
-	char			*snapname;
+	libbe_handle_t			*lbh;
+	char				target_name[BE_MAXPATHLEN];
+	char				*snapname;
+	SLIST_HEAD(, promote_entry)	promotelist;
 };
 
 #if SOON
@@ -194,7 +201,153 @@ be_nicenum(uint64_t num, char *buf, size_t buflen)
 	zfs_nicenum(num, buf, buflen);
 }
 
+static bool
+be_should_promote_clones(zfs_handle_t *zfs_hdl, struct be_destroy_data *bdd)
+{
+	char *atpos;
+
+	if (zfs_get_type(zfs_hdl) != ZFS_TYPE_SNAPSHOT)
+		return (false);
+
+	/*
+	 * If we're deleting a snapshot, we need to make sure we only promote
+	 * clones that are derived from one of the snapshots we're deleting,
+	 * rather than that of a snapshot we're not touching.  This keeps stuff
+	 * in a consistent state, making sure that we don't error out unless
+	 * we really need to.
+	 */
+	if (bdd->snapname == NULL)
+		return (true);
+
+	atpos = strchr(zfs_get_name(zfs_hdl), '@');
+	return (strcmp(atpos + 1, bdd->snapname) == 0);
+}
+
+/*
+ * This is executed from be_promote_dependent_clones via zfs_iter_dependents,
+ * It checks if the dependent type is a snapshot then attempts to find any
+ * clones associated with it. Any clones not related to the destroy target are
+ * added to the promote list.
+ */
 static int
+be_dependent_clone_cb(zfs_handle_t *zfs_hdl, void *data)
+{
+	int err;
+	bool found;
+	char *name;
+	struct nvlist *nvl;
+	struct nvpair *nvp;
+	struct be_destroy_data *bdd;
+	struct promote_entry *entry, *newentry;
+
+	nvp = NULL;
+	err = 0;
+	bdd = (struct be_destroy_data *)data;
+
+	if (be_should_promote_clones(zfs_hdl, bdd) &&
+	    (nvl = zfs_get_clones_nvl(zfs_hdl)) != NULL) {
+		while ((nvp = nvlist_next_nvpair(nvl, nvp)) != NULL) {
+			name = nvpair_name(nvp);
+
+			/*
+			 * Skip if the clone is equal to, or a child of, the
+			 * destroy target.
+			 */
+			if (strncmp(name, bdd->target_name,
+			    strlen(bdd->target_name)) == 0 ||
+			    strstr(name, bdd->target_name) == name) {
+				continue;
+			}
+
+			found = false;
+			SLIST_FOREACH(entry, &bdd->promotelist, link) {
+				if (strcmp(entry->name, name) == 0) {
+					found = true;
+					break;
+				}
+			}
+
+			if (found)
+				continue;
+
+			newentry = malloc(sizeof(struct promote_entry));
+			if (newentry == NULL) {
+				err = ENOMEM;
+				break;
+			}
+
+#define	BE_COPY_NAME(entry, src)	\
+	strlcpy((entry)->name, (src), sizeof((entry)->name))
+			if (BE_COPY_NAME(newentry, name) >=
+			    sizeof(newentry->name)) {
+				/* Shouldn't happen. */
+				free(newentry);
+				err = ENAMETOOLONG;
+				break;
+			}
+#undef BE_COPY_NAME
+
+			/*
+			 * We're building up a SLIST here to make sure both that
+			 * we get the order right and so that we don't
+			 * inadvertently observe the wrong state by promoting
+			 * datasets while we're still walking the tree.  The
+			 * latter can lead to situations where we promote a BE
+			 * then effectively demote it again.
+			 */
+			SLIST_INSERT_HEAD(&bdd->promotelist, newentry, link);
+		}
+		nvlist_free(nvl);
+	}
+	zfs_close(zfs_hdl);
+	return (err);
+}
+
+/*
+ * This is called before a destroy, so that any datasets(environments) that are
+ * dependent on this one get promoted before destroying the target.
+ */
+static int
+be_promote_dependent_clones(zfs_handle_t *zfs_hdl, struct be_destroy_data *bdd)
+{
+	int err;
+	zfs_handle_t *clone;
+	struct promote_entry *entry;
+
+	snprintf(bdd->target_name, BE_MAXPATHLEN, "%s/", zfs_get_name(zfs_hdl));
+	err = zfs_iter_dependents(zfs_hdl, true, be_dependent_clone_cb, bdd);
+
+	/*
+	 * Drain the list and walk away from it if we're only deleting a
+	 * snapshot.
+	 */
+	if (bdd->snapname != NULL && !SLIST_EMPTY(&bdd->promotelist))
+		err = BE_ERR_HASCLONES;
+	while (!SLIST_EMPTY(&bdd->promotelist)) {
+		entry = SLIST_FIRST(&bdd->promotelist);
+		SLIST_REMOVE_HEAD(&bdd->promotelist, link);
+
+#define	ZFS_GRAB_CLONE()	\
+	zfs_open(bdd->lbh->lzh, entry->name, ZFS_TYPE_FILESYSTEM)
+		/*
+		 * Just skip this part on error, we still want to clean up the
+		 * promotion list after the first error.  We'll then preserve it
+		 * all the way back.
+		 */
+		if (err == 0 && (clone = ZFS_GRAB_CLONE()) != NULL) {
+			err = zfs_promote(clone);
+			if (err != 0)
+				err = BE_ERR_DESTROYMNT;
+			zfs_close(clone);
+		}
+#undef ZFS_GRAB_CLONE
+		free(entry);
+	}
+
+	return (err);
+}
+
+static int
 be_destroy_cb(zfs_handle_t *zfs_hdl, void *data)
 {
 	char path[BE_MAXPATHLEN];
@@ -239,8 +392,9 @@ be_destroy_cb(zfs_handle_t *zfs_hdl, void *data)
  * BE_DESTROY_FORCE : forces operation on mounted datasets
  * BE_DESTROY_ORIGIN: destroy the origin snapshot as well
  */
-int
-be_destroy(libbe_handle_t *lbh, const char *name, int options)
+static int
+be_destroy_internal(libbe_handle_t *lbh, const char *name, int options,
+    bool odestroyer)
 {
 	struct be_destroy_data bdd;
 	char origin[BE_MAXPATHLEN], path[BE_MAXPATHLEN];
@@ -251,6 +405,7 @@ be_destroy(libbe_handle_t *lbh, const char *name, int 
 
 	bdd.lbh = lbh;
 	bdd.snapname = NULL;
+	SLIST_INIT(&bdd.promotelist);
 	force = options & BE_DESTROY_FORCE;
 	*origin = '\0';
 
@@ -268,25 +423,6 @@ be_destroy(libbe_handle_t *lbh, const char *name, int 
 		if (fs == NULL)
 			return (set_error(lbh, BE_ERR_ZFSOPEN));
 
-		if ((options & BE_DESTROY_WANTORIGIN) != 0 &&
-		    zfs_prop_get(fs, ZFS_PROP_ORIGIN, origin, sizeof(origin),
-		    NULL, NULL, 0, 1) != 0 &&
-		    (options & BE_DESTROY_ORIGIN) != 0)
-			return (set_error(lbh, BE_ERR_NOORIGIN));
-
-		/*
-		 * If the caller wants auto-origin destruction and the origin
-		 * name matches one of our automatically created snapshot names
-		 * (i.e. strftime("%F-%T") with a serial at the end), then
-		 * we'll set the DESTROY_ORIGIN flag and nuke it
-		 * be_is_auto_snapshot_name is exported from libbe(3) so that
-		 * the caller can determine if it needs to warn about the origin
-		 * not being destroyed or not.
-		 */
-		if ((options & BE_DESTROY_AUTOORIGIN) != 0 && *origin != '\0' &&
-		    be_is_auto_snapshot_name(lbh, origin))
-			options |= BE_DESTROY_ORIGIN;
-
 		/* Don't destroy a mounted dataset unless force is specified */
 		if ((mounted = zfs_is_mounted(fs, NULL)) != 0) {
 			if (force) {
@@ -297,6 +433,14 @@ be_destroy(libbe_handle_t *lbh, const char *name, int 
 			}
 		}
 	} else {
+		/*
+		 * If we're initially destroying a snapshot, origin options do
+		 * not make sense.  If we're destroying the origin snapshot of
+		 * a BE, we want to maintain the options in case we need to
+		 * fake success after failing to promote.
+		 */
+		if (!odestroyer)
+			options &= ~BE_DESTROY_WANTORIGIN;
 		if (!zfs_dataset_exists(lbh->lzh, path, ZFS_TYPE_SNAPSHOT))
 			return (set_error(lbh, BE_ERR_NOENT));
 
@@ -311,6 +455,49 @@ be_destroy(libbe_handle_t *lbh, const char *name, int 
 		}
 	}
 
+	/*
+	 * Whether we're destroying a BE or a single snapshot, we need to walk
+	 * the tree of what we're going to destroy and promote everything in our
+	 * path so that we can make it happen.
+	 */
+	if ((err = be_promote_dependent_clones(fs, &bdd)) != 0) {
+		free(bdd.snapname);
+
+		/*
+		 * If we're just destroying the origin of some other dataset
+		 * we were invoked to destroy, then we just ignore
+		 * BE_ERR_HASCLONES and return success unless the caller wanted
+		 * to force the issue.
+		 */
+		if (odestroyer && err == BE_ERR_HASCLONES &&
+		    (options & BE_DESTROY_AUTOORIGIN) != 0)
+			return (0);
+		return (set_error(lbh, err));
+	}
+
+	/*
+	 * This was deferred until after we promote all of the derivatives so
+	 * that we grab the new origin after everything's settled down.
+	 */
+	if ((options & BE_DESTROY_WANTORIGIN) != 0 &&
+	    zfs_prop_get(fs, ZFS_PROP_ORIGIN, origin, sizeof(origin),
+	    NULL, NULL, 0, 1) != 0 &&
+	    (options & BE_DESTROY_ORIGIN) != 0)
+		return (set_error(lbh, BE_ERR_NOORIGIN));
+
+	/*
+	 * If the caller wants auto-origin destruction and the origin
+	 * name matches one of our automatically created snapshot names
+	 * (i.e. strftime("%F-%T") with a serial at the end), then
+	 * we'll set the DESTROY_ORIGIN flag and nuke it
+	 * be_is_auto_snapshot_name is exported from libbe(3) so that
+	 * the caller can determine if it needs to warn about the origin
+	 * not being destroyed or not.
+	 */
+	if ((options & BE_DESTROY_AUTOORIGIN) != 0 && *origin != '\0' &&
+	    be_is_auto_snapshot_name(lbh, origin))
+		options |= BE_DESTROY_ORIGIN;
+
 	err = be_destroy_cb(fs, &bdd);
 	zfs_close(fs);
 	free(bdd.snapname);
@@ -337,8 +524,23 @@ be_destroy(libbe_handle_t *lbh, const char *name, int 
 	if (strncmp(origin, lbh->root, rootlen) != 0 || origin[rootlen] != '/')
 		return (0);
 
-	return (be_destroy(lbh, origin + rootlen + 1,
-	    options & ~BE_DESTROY_ORIGIN));
+	return (be_destroy_internal(lbh, origin + rootlen + 1,
+	    options & ~BE_DESTROY_ORIGIN, true));
+}
+
+int
+be_destroy(libbe_handle_t *lbh, const char *name, int options)
+{
+
+	/*
+	 * The consumer must not set both BE_DESTROY_AUTOORIGIN and
+	 * BE_DESTROY_ORIGIN.  Internally, we'll set the latter from the former.
+	 * The latter should imply that we must succeed at destroying the
+	 * origin, or complain otherwise.
+	 */
+	if ((options & BE_DESTROY_WANTORIGIN) == BE_DESTROY_WANTORIGIN)
+		return (set_error(lbh, BE_ERR_UNKNOWN));
+	return (be_destroy_internal(lbh, name, options, false));
 }
 
 static void

Modified: head/lib/libbe/be.h
==============================================================================
--- head/lib/libbe/be.h	Thu Jan  2 17:51:49 2020	(r356278)
+++ head/lib/libbe/be.h	Thu Jan  2 18:46:33 2020	(r356279)
@@ -60,6 +60,7 @@ typedef enum be_error {
 	BE_ERR_NOMEM,		/* insufficient memory */
 	BE_ERR_UNKNOWN,         /* unknown error */
 	BE_ERR_INVORIGIN,       /* invalid origin */
+	BE_ERR_HASCLONES,	/* snapshot has clones */
 } be_error_t;
 
 

Modified: head/lib/libbe/be_error.c
==============================================================================
--- head/lib/libbe/be_error.c	Thu Jan  2 17:51:49 2020	(r356278)
+++ head/lib/libbe/be_error.c	Thu Jan  2 18:46:33 2020	(r356279)
@@ -108,6 +108,9 @@ libbe_error_description(libbe_handle_t *lbh)
 	case BE_ERR_INVORIGIN:
 		return ("invalid origin");
 
+	case BE_ERR_HASCLONES:
+		return ("snapshot has clones");
+
 	default:
 		assert(lbh->error == BE_ERR_SUCCESS);
 		return ("no error");

Modified: head/sbin/bectl/tests/bectl_test.sh
==============================================================================
--- head/sbin/bectl/tests/bectl_test.sh	Thu Jan  2 17:51:49 2020	(r356278)
+++ head/sbin/bectl/tests/bectl_test.sh	Thu Jan  2 18:46:33 2020	(r356279)
@@ -162,6 +162,51 @@ bectl_destroy_body()
 	atf_check bectl -r ${zpool}/ROOT create -e default default3
 	atf_check bectl -r ${zpool}/ROOT destroy -o default3
 	atf_check bectl -r ${zpool}/ROOT unmount default
+
+	# create two be from the same parent and destroy the parent
+	atf_check bectl -r ${zpool}/ROOT create -e default default2
+	atf_check bectl -r ${zpool}/ROOT create -e default default3
+	atf_check bectl -r ${zpool}/ROOT destroy default
+	atf_check bectl -r ${zpool}/ROOT destroy default2
+	atf_check bectl -r ${zpool}/ROOT rename default3 default
+
+	# Create a BE, have it be the parent for another and repeat, then start
+	# deleting environments.  Arbitrarily chose default3 as the first.
+	# Sleeps are required to prevent conflicting snapshots- libbe will
+	# use the time with a serial at the end as needed to prevent collisions,
+	# but as BEs get promoted the snapshot names will convert and conflict
+	# anyways.  libbe should perhaps consider adding something extra to the
+	# default name to prevent collisions like this, but the default name
+	# includes down to the second and creating BEs this rapidly is perhaps
+	# uncommon enough.
+	atf_check bectl -r ${zpool}/ROOT create -e default default2
+	sleep 1
+	atf_check bectl -r ${zpool}/ROOT create -e default2 default3
+	sleep 1
+	atf_check bectl -r ${zpool}/ROOT create -e default3 default4
+	atf_check bectl -r ${zpool}/ROOT destroy default3
+	atf_check bectl -r ${zpool}/ROOT destroy default2
+	atf_check bectl -r ${zpool}/ROOT destroy default4
+
+	# Create two BEs, then create an unrelated snapshot on the originating
+	# BE and destroy it.  We shouldn't have promoted the second BE, and it's
+	# only possible to tell if we promoted it by making sure we didn't
+	# demote the first BE at some point -- if we did, it's origin will no
+	# longer be empty.
+	atf_check bectl -r ${zpool}/ROOT create -e default default2
+	atf_check bectl -r ${zpool}/ROOT create default at test
+
+	atf_check bectl -r ${zpool}/ROOT destroy default at test
+	atf_check -o inline:"-\n" zfs get -Ho value origin ${zpool}/ROOT/default
+	atf_check bectl -r ${zpool}/ROOT destroy default2
+
+	# As observed by beadm, if we explicitly try to destroy a snapshot that
+	# leads to clones, we shouldn't have allowed it.
+	atf_check bectl -r ${zpool}/ROOT create default at test
+	atf_check bectl -r ${zpool}/ROOT create -e default at test default2
+
+	atf_check -e  not-empty -s not-exit:0 bectl -r ${zpool}/ROOT destroy \
+	    default at test
 }
 bectl_destroy_cleanup()
 {


More information about the svn-src-head mailing list