Date: Fri, 10 Jan 2020 03:37:53 +0000 (UTC) From: Kyle Evans <kevans@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r356593 - in stable: 11/lib/libbe 11/sbin/bectl/tests 12/lib/libbe 12/sbin/bectl/tests Message-ID: <202001100337.00A3brcY020405@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: kevans Date: Fri Jan 10 03:37:52 2020 New Revision: 356593 URL: https://svnweb.freebsd.org/changeset/base/356593 Log: MFC r356279: libbe(3): promote dependent clones when destroying a BE 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 Modified: stable/12/lib/libbe/be.c stable/12/lib/libbe/be.h stable/12/lib/libbe/be_error.c stable/12/sbin/bectl/tests/bectl_test.sh Directory Properties: stable/12/ (props changed) Changes in other areas also in this revision: Modified: stable/11/lib/libbe/be.c stable/11/lib/libbe/be.h stable/11/lib/libbe/be_error.c stable/11/sbin/bectl/tests/bectl_test.sh Directory Properties: stable/11/ (props changed) Modified: stable/12/lib/libbe/be.c ============================================================================== --- stable/12/lib/libbe/be.c Fri Jan 10 03:17:28 2020 (r356592) +++ stable/12/lib/libbe/be.c Fri Jan 10 03:37:52 2020 (r356593) @@ -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: stable/12/lib/libbe/be.h ============================================================================== --- stable/12/lib/libbe/be.h Fri Jan 10 03:17:28 2020 (r356592) +++ stable/12/lib/libbe/be.h Fri Jan 10 03:37:52 2020 (r356593) @@ -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: stable/12/lib/libbe/be_error.c ============================================================================== --- stable/12/lib/libbe/be_error.c Fri Jan 10 03:17:28 2020 (r356592) +++ stable/12/lib/libbe/be_error.c Fri Jan 10 03:37:52 2020 (r356593) @@ -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: stable/12/sbin/bectl/tests/bectl_test.sh ============================================================================== --- stable/12/sbin/bectl/tests/bectl_test.sh Fri Jan 10 03:17:28 2020 (r356592) +++ stable/12/sbin/bectl/tests/bectl_test.sh Fri Jan 10 03:37:52 2020 (r356593) @@ -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@test + + atf_check bectl -r ${zpool}/ROOT destroy default@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@test + atf_check bectl -r ${zpool}/ROOT create -e default@test default2 + + atf_check -e not-empty -s not-exit:0 bectl -r ${zpool}/ROOT destroy \ + default@test } bectl_destroy_cleanup() {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202001100337.00A3brcY020405>