From owner-svn-src-head@freebsd.org Wed Feb 13 04:19:10 2019 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 5928314E2E0C; Wed, 13 Feb 2019 04:19:10 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id E49546FA5E; Wed, 13 Feb 2019 04:19:09 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id CF448220B8; Wed, 13 Feb 2019 04:19:09 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x1D4J9tX012815; Wed, 13 Feb 2019 04:19:09 GMT (envelope-from kevans@FreeBSD.org) Received: (from kevans@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x1D4J9VA012812; Wed, 13 Feb 2019 04:19:09 GMT (envelope-from kevans@FreeBSD.org) Message-Id: <201902130419.x1D4J9VA012812@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kevans set sender to kevans@FreeBSD.org using -f From: Kyle Evans Date: Wed, 13 Feb 2019 04:19:09 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r344084 - head/lib/libbe X-SVN-Group: head X-SVN-Commit-Author: kevans X-SVN-Commit-Paths: head/lib/libbe X-SVN-Commit-Revision: 344084 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: E49546FA5E X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.96 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-1.00)[-0.996,0]; NEURAL_HAM_SHORT(-0.97)[-0.969,0]; NEURAL_HAM_LONG(-1.00)[-0.999,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Feb 2019 04:19:10 -0000 Author: kevans Date: Wed Feb 13 04:19:08 2019 New Revision: 344084 URL: https://svnweb.freebsd.org/changeset/base/344084 Log: 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. MFC after: 1 week Modified: head/lib/libbe/be.c head/lib/libbe/be.h head/lib/libbe/be_error.c head/lib/libbe/libbe.3 Modified: head/lib/libbe/be.c ============================================================================== --- head/lib/libbe/be.c Wed Feb 13 03:11:12 2019 (r344083) +++ head/lib/libbe/be.c Wed Feb 13 04:19:08 2019 (r344084) @@ -45,6 +45,11 @@ __FBSDID("$FreeBSD$"); #include "be.h" #include "be_impl.h" +struct be_destroy_data { + libbe_handle_t *lbh; + char *snapname; +}; + #if SOON static int be_create_child_noent(libbe_handle_t *lbh, const char *active, const char *child_path); @@ -186,12 +191,38 @@ be_nicenum(uint64_t num, char *buf, size_t buflen) static int be_destroy_cb(zfs_handle_t *zfs_hdl, void *data) { + char path[BE_MAXPATHLEN]; + struct be_destroy_data *bdd; + zfs_handle_t *snap; int err; - if ((err = zfs_iter_children(zfs_hdl, be_destroy_cb, data)) != 0) + bdd = (struct be_destroy_data *)data; + if (bdd->snapname == NULL) { + err = zfs_iter_children(zfs_hdl, be_destroy_cb, data); + if (err != 0) + return (err); + return (zfs_destroy(zfs_hdl, false)); + } + /* If we're dealing with snapshots instead, delete that one alone */ + err = zfs_iter_filesystems(zfs_hdl, be_destroy_cb, data); + if (err != 0) return (err); - if ((err = zfs_destroy(zfs_hdl, false)) != 0) - return (err); + /* + * This part is intentionally glossing over any potential errors, + * because there's a lot less potential for errors when we're cleaning + * up snapshots rather than a full deep BE. The primary error case + * here being if the snapshot doesn't exist in the first place, which + * the caller will likely deem insignificant as long as it doesn't + * exist after the call. Thus, such a missing snapshot shouldn't jam + * up the destruction. + */ + snprintf(path, sizeof(path), "%s@%s", zfs_get_name(zfs_hdl), + bdd->snapname); + if (!zfs_dataset_exists(bdd->lbh->lzh, path, ZFS_TYPE_SNAPSHOT)) + return (0); + snap = zfs_open(bdd->lbh->lzh, path, ZFS_TYPE_SNAPSHOT); + if (snap != NULL) + zfs_destroy(snap, false); return (0); } @@ -199,22 +230,26 @@ be_destroy_cb(zfs_handle_t *zfs_hdl, void *data) * Destroy the boot environment or snapshot specified by the name * parameter. Options are or'd together with the possible values: * 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) { + struct be_destroy_data bdd; char origin[BE_MAXPATHLEN], path[BE_MAXPATHLEN]; zfs_handle_t *fs; - char *p; + char *snapdelim; int err, force, mounted; + size_t rootlen; - p = path; + bdd.lbh = lbh; + bdd.snapname = NULL; force = options & BE_DESTROY_FORCE; *origin = '\0'; be_root_concat(lbh, name, path); - if (strchr(name, '@') == NULL) { + if ((snapdelim = strchr(path, '@')) == NULL) { if (!zfs_dataset_exists(lbh->lzh, path, ZFS_TYPE_FILESYSTEM)) return (set_error(lbh, BE_ERR_NOENT)); @@ -222,9 +257,10 @@ be_destroy(libbe_handle_t *lbh, const char *name, int strcmp(path, lbh->bootfs) == 0) return (set_error(lbh, BE_ERR_DESTROYACT)); - fs = zfs_open(lbh->lzh, p, ZFS_TYPE_FILESYSTEM); + fs = zfs_open(lbh->lzh, path, ZFS_TYPE_FILESYSTEM); if (fs == NULL) return (set_error(lbh, BE_ERR_ZFSOPEN)); + if ((options & BE_DESTROY_ORIGIN) != 0 && zfs_prop_get(fs, ZFS_PROP_ORIGIN, origin, sizeof(origin), NULL, NULL, 0, 1) != 0) @@ -233,40 +269,56 @@ be_destroy(libbe_handle_t *lbh, const char *name, int if (!zfs_dataset_exists(lbh->lzh, path, ZFS_TYPE_SNAPSHOT)) return (set_error(lbh, BE_ERR_NOENT)); - fs = zfs_open(lbh->lzh, p, ZFS_TYPE_SNAPSHOT); - if (fs == NULL) + bdd.snapname = strdup(snapdelim + 1); + if (bdd.snapname == NULL) + return (set_error(lbh, BE_ERR_NOMEM)); + *snapdelim = '\0'; + fs = zfs_open(lbh->lzh, path, ZFS_TYPE_DATASET); + if (fs == NULL) { + free(bdd.snapname); return (set_error(lbh, BE_ERR_ZFSOPEN)); + } } /* Check if mounted, unmount if force is specified */ if ((mounted = zfs_is_mounted(fs, NULL)) != 0) { - if (force) + if (force) { zfs_unmount(fs, NULL, 0); - else + } else { + free(bdd.snapname); return (set_error(lbh, BE_ERR_DESTROYMNT)); + } } - if ((err = be_destroy_cb(fs, NULL)) != 0) { + err = be_destroy_cb(fs, &bdd); + zfs_close(fs); + free(bdd.snapname); + if (err != 0) { /* Children are still present or the mount is referenced */ if (err == EBUSY) return (set_error(lbh, BE_ERR_DESTROYMNT)); return (set_error(lbh, BE_ERR_UNKNOWN)); } - if (*origin != '\0') { - fs = zfs_open(lbh->lzh, origin, ZFS_TYPE_SNAPSHOT); - if (fs == NULL) - return (set_error(lbh, BE_ERR_ZFSOPEN)); - err = zfs_destroy(fs, false); - if (err == EBUSY) - return (set_error(lbh, BE_ERR_DESTROYMNT)); - else if (err != 0) - return (set_error(lbh, BE_ERR_UNKNOWN)); - } + if ((options & BE_DESTROY_ORIGIN) == 0) + return (0); - return (0); -} + /* The origin can't possibly be shorter than the BE root */ + rootlen = strlen(lbh->root); + if (*origin == '\0' || strlen(origin) <= rootlen + 1) + return (set_error(lbh, BE_ERR_INVORIGIN)); + /* + * We'll be chopping off the BE root and running this back through + * be_destroy, so that we properly handle the origin snapshot whether + * it be that of a deep BE or not. + */ + if (strncmp(origin, lbh->root, rootlen) != 0 || origin[rootlen] != '/') + return (0); + + return (be_destroy(lbh, origin + rootlen + 1, + options & ~BE_DESTROY_ORIGIN)); +} int be_snapshot(libbe_handle_t *lbh, const char *source, const char *snap_name, Modified: head/lib/libbe/be.h ============================================================================== --- head/lib/libbe/be.h Wed Feb 13 03:11:12 2019 (r344083) +++ head/lib/libbe/be.h Wed Feb 13 04:19:08 2019 (r344084) @@ -59,6 +59,7 @@ typedef enum be_error { BE_ERR_NOPOOL, /* operation not supported on this pool */ BE_ERR_NOMEM, /* insufficient memory */ BE_ERR_UNKNOWN, /* unknown error */ + BE_ERR_INVORIGIN, /* invalid origin */ } be_error_t; Modified: head/lib/libbe/be_error.c ============================================================================== --- head/lib/libbe/be_error.c Wed Feb 13 03:11:12 2019 (r344083) +++ head/lib/libbe/be_error.c Wed Feb 13 04:19:08 2019 (r344084) @@ -105,6 +105,9 @@ libbe_error_description(libbe_handle_t *lbh) case BE_ERR_UNKNOWN: return ("unknown error"); + case BE_ERR_INVORIGIN: + return ("invalid origin"); + default: assert(lbh->error == BE_ERR_SUCCESS); return ("no error"); Modified: head/lib/libbe/libbe.3 ============================================================================== --- head/lib/libbe/libbe.3 Wed Feb 13 03:11:12 2019 (r344083) +++ head/lib/libbe/libbe.3 Wed Feb 13 04:19:08 2019 (r344084) @@ -28,7 +28,7 @@ .\" .\" $FreeBSD$ .\" -.Dd February 11, 2019 +.Dd February 12, 2019 .Dt LIBBE 3 .Os .Sh NAME @@ -489,6 +489,8 @@ BE_ERR_NOPOOL BE_ERR_NOMEM .It BE_ERR_UNKNOWN +.It +BE_ERR_INVORIGIN .El .Sh SEE ALSO .Xr bectl 8