svn commit: r346034 - in stable/12: lib/libbe sbin/bectl sbin/bectl/tests

Kyle Evans kevans at FreeBSD.org
Tue Sep 3 14:06:28 UTC 2019


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 at 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 at ...
  
  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 at snapshot
 .Nm
 .Cm destroy
-.Op Fl F
+.Op Fl \&Fo
 .Brq Ar beName | beName at 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 at 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




More information about the svn-src-stable-12 mailing list