Date: Mon, 8 Apr 2019 17:41:39 +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: r346034 - in stable/12: lib/libbe sbin/bectl sbin/bectl/tests Message-ID: <201904081741.x38HfdTE029036@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: kevans Date: Mon Apr 8 17:41:39 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 Modified: 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 Directory Properties: stable/12/ (props changed) Modified: stable/12/lib/libbe/be.c ============================================================================== --- stable/12/lib/libbe/be.c Mon Apr 8 17:36:24 2019 (r346033) +++ stable/12/lib/libbe/be.c Mon Apr 8 17:41:39 2019 (r346034) @@ -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); @@ -189,12 +194,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); } @@ -202,21 +233,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 path[BE_MAXPATHLEN]; - 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)); @@ -224,37 +260,69 @@ 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); - } else { + 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) + return (set_error(lbh, BE_ERR_NOORIGIN)); + + /* Don't destroy a mounted dataset unless force is specified */ + if ((mounted = zfs_is_mounted(fs, NULL)) != 0) { + if (force) { + zfs_unmount(fs, NULL, 0); + } else { + free(bdd.snapname); + return (set_error(lbh, BE_ERR_DESTROYMNT)); + } + } + } else { 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); + 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)); + } } - if (fs == NULL) - 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) - zfs_unmount(fs, NULL, 0); - else - 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)); } - return (0); -} + if ((options & BE_DESTROY_ORIGIN) == 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)); +} + static void be_setup_snapshot_name(libbe_handle_t *lbh, char *buf, size_t buflen) { @@ -669,32 +737,14 @@ int be_import(libbe_handle_t *lbh, const char *bootenv, int fd) { char buf[BE_MAXPATHLEN]; - time_t rawtime; nvlist_t *props; zfs_handle_t *zfs; - int err, len; - char nbuf[24]; + recvflags_t flags = { .nomount = 1 }; + int err; - /* - * We don't need this to be incredibly random, just unique enough that - * it won't conflict with an existing dataset name. Chopping time - * down to 32 bits is probably good enough for this. - */ - snprintf(nbuf, 24, "tmp%u", - (uint32_t)(time(NULL) & 0xFFFFFFFF)); - if ((err = be_root_concat(lbh, nbuf, buf)) != 0) - /* - * Technically this is our problem, but we try to use short - * enough names that we won't run into problems except in - * worst-case BE root approaching MAXPATHLEN. - */ - return (set_error(lbh, BE_ERR_PATHLEN)); + be_root_concat(lbh, bootenv, buf); - time(&rawtime); - len = strlen(buf); - strftime(buf + len, sizeof(buf) - len, "@%F-%T", localtime(&rawtime)); - - if ((err = lzc_receive(buf, NULL, NULL, false, fd)) != 0) { + if ((err = zfs_receive(lbh->lzh, buf, NULL, &flags, fd, NULL)) != 0) { switch (err) { case EINVAL: return (set_error(lbh, BE_ERR_NOORIGIN)); @@ -707,39 +757,22 @@ be_import(libbe_handle_t *lbh, const char *bootenv, in } } - if ((zfs = zfs_open(lbh->lzh, buf, ZFS_TYPE_SNAPSHOT)) == NULL) + if ((zfs = zfs_open(lbh->lzh, buf, ZFS_TYPE_FILESYSTEM)) == NULL) return (set_error(lbh, BE_ERR_ZFSOPEN)); nvlist_alloc(&props, NV_UNIQUE_NAME, KM_SLEEP); nvlist_add_string(props, "canmount", "noauto"); nvlist_add_string(props, "mountpoint", "/"); - be_root_concat(lbh, bootenv, buf); - - err = zfs_clone(zfs, buf, props); - zfs_close(zfs); + err = zfs_prop_set_list(zfs, props); nvlist_free(props); - if (err != 0) - return (set_error(lbh, BE_ERR_UNKNOWN)); - - /* - * Finally, we open up the dataset we just cloned the snapshot so that - * we may promote it. This is necessary in order to clean up the ghost - * snapshot that doesn't need to be seen after the operation is - * complete. - */ - if ((zfs = zfs_open(lbh->lzh, buf, ZFS_TYPE_DATASET)) == NULL) - return (set_error(lbh, BE_ERR_ZFSOPEN)); - - err = zfs_promote(zfs); zfs_close(zfs); if (err != 0) return (set_error(lbh, BE_ERR_UNKNOWN)); - /* Clean up the temporary snapshot */ - return (be_destroy(lbh, nbuf, 0)); + return (0); } #if SOON Modified: stable/12/lib/libbe/be.h ============================================================================== --- stable/12/lib/libbe/be.h Mon Apr 8 17:36:24 2019 (r346033) +++ stable/12/lib/libbe/be.h Mon Apr 8 17:41:39 2019 (r346034) @@ -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; @@ -93,7 +94,8 @@ int be_rename(libbe_handle_t *, const char *, const ch /* Bootenv removal functions */ typedef enum { - BE_DESTROY_FORCE = 1 << 0, + BE_DESTROY_FORCE = 1 << 0, + BE_DESTROY_ORIGIN = 1 << 1, } be_destroy_opt_t; int be_destroy(libbe_handle_t *, const char *, int); @@ -102,7 +104,7 @@ int be_destroy(libbe_handle_t *, const char *, int); typedef enum { BE_MNT_FORCE = 1 << 0, - BE_MNT_DEEP = 1 << 1, + BE_MNT_DEEP = 1 << 1, } be_mount_opt_t; int be_mount(libbe_handle_t *, char *, char *, int, char *); Modified: stable/12/lib/libbe/be_error.c ============================================================================== --- stable/12/lib/libbe/be_error.c Mon Apr 8 17:36:24 2019 (r346033) +++ stable/12/lib/libbe/be_error.c Mon Apr 8 17:41:39 2019 (r346034) @@ -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: stable/12/lib/libbe/libbe.3 ============================================================================== --- stable/12/lib/libbe/libbe.3 Mon Apr 8 17:36:24 2019 (r346033) +++ stable/12/lib/libbe/libbe.3 Mon Apr 8 17:41:39 2019 (r346034) @@ -28,7 +28,7 @@ .\" .\" $FreeBSD$ .\" -.Dd November 21, 2018 +.Dd February 12, 2019 .Dt LIBBE 3 .Os .Sh NAME @@ -253,6 +253,13 @@ It will not destroy a mounted boot environment unless .Dv BE_DESTROY_FORCE option is set in .Fa options . +If the +.Dv BE_DESTROY_ORIGIN +option is set in +.Fa options , +the +.Fn be_destroy +function will destroy the origin snapshot to this boot environment as well. .Pp The .Fn be_nicenum @@ -482,6 +489,8 @@ BE_ERR_NOPOOL BE_ERR_NOMEM .It BE_ERR_UNKNOWN +.It +BE_ERR_INVORIGIN .El .Sh SEE ALSO .Xr bectl 8 Modified: stable/12/sbin/bectl/bectl.8 ============================================================================== --- stable/12/sbin/bectl/bectl.8 Mon Apr 8 17:36:24 2019 (r346033) +++ stable/12/sbin/bectl/bectl.8 Mon Apr 8 17:41:39 2019 (r346034) @@ -18,7 +18,7 @@ .\" .\" $FreeBSD$ .\" -.Dd December 25, 2018 +.Dd February 10, 2019 .Dt BECTL 8 .Os .Sh NAME @@ -40,7 +40,7 @@ .Ar beName@snapshot .Nm .Cm destroy -.Op Fl F +.Op Fl \&Fo .Brq Ar beName | beName@snapshot .Nm .Cm export @@ -124,7 +124,7 @@ If the flag is given, a recursive boot environment will be made. .It Xo .Cm destroy -.Op Fl F +.Op Fl \&Fo .Brq Ar beName | beName@snapshot .Xc Destroys the given @@ -136,6 +136,14 @@ snapshot without confirmation, unlike in Specifying .Fl F will automatically unmount without confirmation. +.Pp +By default, +.Nm +will warn that it is not destroying the origin of +.Ar beName . +The +.Fl o +flag may be specified to destroy the origin as well. .It Cm export Ar sourceBe Export .Ar sourceBe Modified: stable/12/sbin/bectl/bectl.c ============================================================================== --- stable/12/sbin/bectl/bectl.c Mon Apr 8 17:36:24 2019 (r346033) +++ stable/12/sbin/bectl/bectl.c Mon Apr 8 17:41:39 2019 (r346034) @@ -341,16 +341,19 @@ bectl_cmd_add(int argc, char *argv[]) static int bectl_cmd_destroy(int argc, char *argv[]) { - char *target; - int opt, err; - bool force; + nvlist_t *props; + char *origin, *target, targetds[BE_MAXPATHLEN]; + int err, flags, opt; - force = false; - while ((opt = getopt(argc, argv, "F")) != -1) { + flags = 0; + while ((opt = getopt(argc, argv, "Fo")) != -1) { switch (opt) { case 'F': - force = true; + flags |= BE_DESTROY_FORCE; break; + case 'o': + flags |= BE_DESTROY_ORIGIN; + break; default: fprintf(stderr, "bectl destroy: unknown option '-%c'\n", optopt); @@ -368,7 +371,24 @@ bectl_cmd_destroy(int argc, char *argv[]) target = argv[0]; - err = be_destroy(be, target, force); + /* We'll emit a notice if there's an origin to be cleaned up */ + if ((flags & BE_DESTROY_ORIGIN) == 0 && strchr(target, '@') == NULL) { + if (be_root_concat(be, target, targetds) != 0) + goto destroy; + if (be_prop_list_alloc(&props) != 0) + goto destroy; + if (be_get_dataset_props(be, targetds, props) != 0) { + be_prop_list_free(props); + goto destroy; + } + if (nvlist_lookup_string(props, "origin", &origin) == 0) + fprintf(stderr, "bectl destroy: leaving origin '%s' intact\n", + origin); + be_prop_list_free(props); + } + +destroy: + err = be_destroy(be, target, flags); return (err); } Modified: stable/12/sbin/bectl/bectl_jail.c ============================================================================== --- stable/12/sbin/bectl/bectl_jail.c Mon Apr 8 17:36:24 2019 (r346033) +++ stable/12/sbin/bectl/bectl_jail.c Mon Apr 8 17:41:39 2019 (r346034) @@ -40,10 +40,10 @@ __FBSDID("$FreeBSD$"); #include <unistd.h> #include <be.h> - #include "bectl.h" -static void jailparam_grow(void); +#define MNTTYPE_ZFS 222 + static void jailparam_add(const char *name, const char *val); static int jailparam_del(const char *name); static bool jailparam_addarg(char *arg); @@ -51,84 +51,28 @@ static int jailparam_delarg(char *arg); static int bectl_search_jail_paths(const char *mnt); static int bectl_locate_jail(const char *ident); +static int bectl_jail_cleanup(char *mountpoint, int jid); -/* We'll start with 8 parameters initially and grow as needed. */ -#define INIT_PARAMCOUNT 8 - -static struct jailparam *jp; -static int jpcnt; -static int jpused; static char mnt_loc[BE_MAXPATHLEN]; +static nvlist_t *jailparams; -static void -jailparam_grow(void) -{ +static const char *disabled_params[] = { + "command", "exec.start", "nopersist", "persist", NULL +}; - jpcnt *= 2; - jp = realloc(jp, jpcnt * sizeof(*jp)); - if (jp == NULL) - err(2, "realloc"); -} static void jailparam_add(const char *name, const char *val) { - int i; - for (i = 0; i < jpused; ++i) { - if (strcmp(name, jp[i].jp_name) == 0) - break; - } - - if (i < jpused) - jailparam_free(&jp[i], 1); - else if (jpused == jpcnt) - /* The next slot isn't allocated yet */ - jailparam_grow(); - - if (jailparam_init(&jp[i], name) != 0) - return; - if (jailparam_import(&jp[i], val) != 0) - return; - ++jpused; + nvlist_add_string(jailparams, name, val); } static int jailparam_del(const char *name) { - int i; - char *val; - for (i = 0; i < jpused; ++i) { - if (strcmp(name, jp[i].jp_name) == 0) - break; - } - - if (i == jpused) - return (ENOENT); - - for (; i < jpused - 1; ++i) { - val = jailparam_export(&jp[i + 1]); - - jailparam_free(&jp[i], 1); - /* - * Given the context, the following will really only fail if - * they can't allocate the copy of the name or value. - */ - if (jailparam_init(&jp[i], jp[i + 1].jp_name) != 0) { - free(val); - return (ENOMEM); - } - if (jailparam_import(&jp[i], val) != 0) { - jailparam_free(&jp[i], 1); - free(val); - return (ENOMEM); - } - free(val); - } - - jailparam_free(&jp[i], 1); - --jpused; + nvlist_remove_all(jailparams, name); return (0); } @@ -136,6 +80,7 @@ static bool jailparam_addarg(char *arg) { char *name, *val; + size_t i, len; if (arg == NULL) return (false); @@ -156,6 +101,15 @@ jailparam_addarg(char *arg) } strlcpy(mnt_loc, val, sizeof(mnt_loc)); } + + for (i = 0; disabled_params[i] != NULL; i++) { + len = strlen(disabled_params[i]); + if (strncmp(disabled_params[i], name, len) == 0) { + fprintf(stderr, "invalid jail parameter: %s\n", name); + return (false); + } + } + jailparam_add(name, val); return (true); } @@ -176,22 +130,128 @@ jailparam_delarg(char *arg) return (jailparam_del(name)); } +static int +build_jailcmd(char ***argvp, bool interactive, int argc, char *argv[]) +{ + char *cmd, **jargv, *name, *val; + nvpair_t *nvp; + size_t i, iarg, nargv; + + cmd = NULL; + nvp = NULL; + iarg = i = 0; + if (nvlist_size(jailparams, &nargv, NV_ENCODE_NATIVE) != 0) + return (1); + + /* + * Number of args + "/usr/sbin/jail", "-c", and ending NULL. + * If interactive also include command. + */ + nargv += 3; + if (interactive) { + if (argc == 0) + nargv++; + else + nargv += argc; + } + + jargv = *argvp = calloc(nargv, sizeof(jargv)); + if (jargv == NULL) + err(2, "calloc"); + + jargv[iarg++] = strdup("/usr/sbin/jail"); + jargv[iarg++] = strdup("-c"); + while ((nvp = nvlist_next_nvpair(jailparams, nvp)) != NULL) { + name = nvpair_name(nvp); + if (nvpair_value_string(nvp, &val) != 0) + continue; + + if (asprintf(&jargv[iarg++], "%s=%s", name, val) < 0) + goto error; + } + if (interactive) { + if (argc < 1) + cmd = strdup("/bin/sh"); + else { + cmd = argv[0]; + argc--; + argv++; + } + + if (asprintf(&jargv[iarg++], "command=%s", cmd) < 0) { + goto error; + } + if (argc < 1) { + free(cmd); + cmd = NULL; + } + + for (; argc > 0; argc--) { + if (asprintf(&jargv[iarg++], "%s", argv[0]) < 0) + goto error; + argv++; + } + } + + return (0); + +error: + if (interactive && argc < 1) + free(cmd); + for (; i < iarg - 1; i++) { + free(jargv[i]); + } + free(jargv); + return (1); +} + +/* Remove jail and cleanup any non zfs mounts. */ +static int +bectl_jail_cleanup(char *mountpoint, int jid) +{ + struct statfs *mntbuf; + size_t i, searchlen, mntsize; + + if (jid >= 0 && jail_remove(jid) != 0) { + fprintf(stderr, "unable to remove jail"); + return (1); + } + + searchlen = strnlen(mountpoint, MAXPATHLEN); + mntsize = getmntinfo(&mntbuf, MNT_NOWAIT); + for (i = 0; i < mntsize; i++) { + if (strncmp(mountpoint, mntbuf[i].f_mntonname, searchlen) == 0 && + mntbuf[i].f_type != MNTTYPE_ZFS) { + + if (unmount(mntbuf[i].f_mntonname, 0) != 0) { + fprintf(stderr, "bectl jail: unable to unmount filesystem %s", + mntbuf[i].f_mntonname); + return (1); + } + } + } + + return (0); +} + int bectl_cmd_jail(int argc, char *argv[]) { - char *bootenv, *mountpoint; - int jid, mntflags, opt, ret; + char *bootenv, **jargv, *mountpoint; + int i, jid, mntflags, opt, ret; bool default_hostname, interactive, unjail; pid_t pid; + /* XXX TODO: Allow shallow */ mntflags = BE_MNT_DEEP; default_hostname = interactive = unjail = true; - jpcnt = INIT_PARAMCOUNT; - jp = malloc(jpcnt * sizeof(*jp)); - if (jp == NULL) - err(2, "malloc"); + if ((nvlist_alloc(&jailparams, NV_UNIQUE_NAME, 0)) != 0) { + fprintf(stderr, "nvlist_alloc() failed\n"); + return (1); + } + jailparam_add("persist", "true"); jailparam_add("allow.mount", "true"); jailparam_add("allow.mount.devfs", "true"); @@ -210,6 +270,8 @@ bectl_cmd_jail(int argc, char *argv[]) */ if (strcmp(optarg, "host.hostname") == 0) default_hostname = false; + } else { + return (1); } break; case 'U': @@ -236,13 +298,14 @@ bectl_cmd_jail(int argc, char *argv[]) argc -= optind; argv += optind; - /* struct jail be_jail = { 0 }; */ if (argc < 1) { fprintf(stderr, "bectl jail: missing boot environment name\n"); return (usage(false)); } bootenv = argv[0]; + argc--; + argv++; /* * XXX TODO: if its already mounted, perhaps there should be a flag to @@ -264,45 +327,46 @@ bectl_cmd_jail(int argc, char *argv[]) * This is our indicator that path was not set by the user, so we'll use * the path that libbe generated for us. */ - if (mountpoint == NULL) + if (mountpoint == NULL) { jailparam_add("path", mnt_loc); - /* Create the jail for now, attach later as-needed */ - jid = jailparam_set(jp, jpused, JAIL_CREATE); - if (jid == -1) { - fprintf(stderr, "unable to create jail. error: %d\n", errno); + mountpoint = mnt_loc; + } + + if ((build_jailcmd(&jargv, interactive, argc, argv)) != 0) { + fprintf(stderr, "unable to build argument list for jail command\n"); return (1); } - jailparam_free(jp, jpused); - free(jp); - - /* We're not interactive, nothing more to do here. */ - if (!interactive) - return (0); - pid = fork(); - switch(pid) { + + switch (pid) { case -1: perror("fork"); return (1); case 0: - jail_attach(jid); - /* We're attached within the jail... good bye! */ - chdir("/"); - if (argc > 1) - execve(argv[1], &argv[1], NULL); - else - execl("/bin/sh", "/bin/sh", NULL); - fprintf(stderr, "bectl jail: failed to execute %s\n", - (argc > 1 ? argv[1] : "/bin/sh")); - _exit(1); + execv("/usr/sbin/jail", jargv); + fprintf(stderr, "bectl jail: failed to execute\n"); default: - /* Wait for the child to get back, see if we need to unjail */ waitpid(pid, NULL, 0); } + for (i = 0; jargv[i] != NULL; i++) { + free(jargv[i]); + } + free(jargv); + + if (!interactive) + return (0); + if (unjail) { - jail_remove(jid); + /* + * We're not checking the jail id result here because in the + * case of invalid param, or last command in jail was an error + * the jail will not exist upon exit. bectl_jail_cleanup will + * only jail_remove if the jid is >= 0. + */ + jid = bectl_locate_jail(bootenv); + bectl_jail_cleanup(mountpoint, jid); be_unmount(be, bootenv, 0); } @@ -319,7 +383,6 @@ bectl_search_jail_paths(const char *mnt) /* jail_getv expects name/value strings */ snprintf(lastjid, sizeof(lastjid), "%d", 0); - jid = 0; while ((jid = jail_getv(0, "lastjid", lastjid, "path", &jailpath, NULL)) != -1) { @@ -416,7 +479,7 @@ bectl_cmd_unjail(int argc, char *argv[]) return (1); } - jail_remove(jid); + bectl_jail_cleanup(path, jid); be_unmount(be, target, 0); return (0); Modified: stable/12/sbin/bectl/tests/bectl_test.sh ============================================================================== --- stable/12/sbin/bectl/tests/bectl_test.sh Mon Apr 8 17:36:24 2019 (r346033) +++ stable/12/sbin/bectl/tests/bectl_test.sh Mon Apr 8 17:41:39 2019 (r346034) @@ -123,12 +123,21 @@ bectl_destroy_body() zpool=$(make_zpool_name) disk=${cwd}/disk.img mount=${cwd}/mnt + root=${mount}/root bectl_create_setup ${zpool} ${disk} ${mount} atf_check bectl -r ${zpool}/ROOT create -e default default2 atf_check -o not-empty zfs get mountpoint ${zpool}/ROOT/default2 - atf_check bectl -r ${zpool}/ROOT destroy default2 + atf_check -e ignore bectl -r ${zpool}/ROOT destroy default2 atf_check -e not-empty -s not-exit:0 zfs get mountpoint ${zpool}/ROOT/default2 + + # Test origin snapshot deletion when the snapshot to be destroyed + # belongs to a mounted dataset, see PR 236043. + atf_check mkdir -p ${root} + atf_check -o not-empty bectl -r ${zpool}/ROOT mount default ${root} + 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 } bectl_destroy_cleanup() { @@ -154,7 +163,7 @@ bectl_export_import_body() atf_check -o save:exported bectl -r ${zpool}/ROOT export default atf_check -x "bectl -r ${zpool}/ROOT import default2 < exported" atf_check -o not-empty zfs get mountpoint ${zpool}/ROOT/default2 - atf_check bectl -r ${zpool}/ROOT destroy default2 + atf_check -e ignore bectl -r ${zpool}/ROOT destroy default2 atf_check -e not-empty -s not-exit:0 zfs get mountpoint \ ${zpool}/ROOT/default2 } @@ -188,7 +197,7 @@ bectl_list_body() atf_check bectl -r ${zpool}/ROOT create -e default default2 atf_check -o save:list.out bectl -r ${zpool}/ROOT list atf_check -o not-empty grep 'default2' list.out - atf_check bectl -r ${zpool}/ROOT destroy default2 + atf_check -e ignore bectl -r ${zpool}/ROOT destroy default2 atf_check -o save:list.out bectl -r ${zpool}/ROOT list atf_check -s not-exit:0 grep 'default2' list.out # XXX TODO: Formatting checks
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201904081741.x38HfdTE029036>