Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 26 Jul 2019 01:49:59 +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: r350344 - in stable: 11/lib/libbe 11/sbin/bectl 11/sbin/bectl/tests 12/lib/libbe 12/sbin/bectl 12/sbin/bectl/tests
Message-ID:  <201907260149.x6Q1nx0K081444@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kevans
Date: Fri Jul 26 01:49:58 2019
New Revision: 350344
URL: https://svnweb.freebsd.org/changeset/base/350344

Log:
  MFC r349380, r349383, r349455: bectl(8)/libbe(3) fixes
  
  r349380:
  libbe(3): mount: the BE dataset is mounted at /
  
  Other parts of libbe(3) were fairly strict on the mountpoint property of the
  BE dataset, and be_mount was not much better. It was improved in r347027 to
  allow mountpoint=none for depth==0, but this bit was still sensitive to
  mountpoint != / and mountpoint != none. Given that other parts of libbe(3)
  no longer restrict the mountpoint property here, and the rest of the base
  system is generally OK and will assume that a BE is mounted at /, let's do
  the same.
  
  r349383:
  libbe(3): restructure be_mount, skip canmount check for BE dataset
  
  Further cleanup after r349380; loader and kernel will both ignore canmount
  on the root dataset as well, so we should not be so strict about it when
  mounting it. be_mount is restructured to make it more clear that depth==0 is
  special, and to not try fetching these properties that we won't care about.
  
  r349455:
  bectl(8): create non-recursive boot environments
  
  bectl advertises that it has the ability to create recursive and
  non-recursive boot environments. This patch implements that functionality
  using the be_create_depth API provided by libbe. With this patch, bectl now
  works as bectl(8) describes in regards to creating recursive/non-recursive
  boot environments.

Modified:
  stable/12/lib/libbe/be_access.c
  stable/12/sbin/bectl/bectl.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_access.c
  stable/11/sbin/bectl/bectl.c
  stable/11/sbin/bectl/tests/bectl_test.sh
Directory Properties:
  stable/11/   (props changed)

Modified: stable/12/lib/libbe/be_access.c
==============================================================================
--- stable/12/lib/libbe/be_access.c	Fri Jul 26 01:49:28 2019	(r350343)
+++ stable/12/lib/libbe/be_access.c	Fri Jul 26 01:49:58 2019	(r350344)
@@ -89,25 +89,31 @@ be_mount_iter(zfs_handle_t *zfs_hdl, void *data)
 		return (0);
 	}
 
-	if (zfs_prop_get_int(zfs_hdl, ZFS_PROP_CANMOUNT) == ZFS_CANMOUNT_OFF)
-		return (0);
+	/*
+	 * canmount and mountpoint are both ignored for the BE dataset, because
+	 * the rest of the system (kernel and loader) will effectively do the
+	 * same.
+	 */
+	if (info->depth == 0) {
+		snprintf(tmp, BE_MAXPATHLEN, "%s", info->mountpoint);
+	} else {
+		if (zfs_prop_get_int(zfs_hdl, ZFS_PROP_CANMOUNT) ==
+		    ZFS_CANMOUNT_OFF)
+			return (0);
 
-	if (zfs_prop_get(zfs_hdl, ZFS_PROP_MOUNTPOINT, zfs_mnt, BE_MAXPATHLEN,
-	    NULL, NULL, 0, 1))
-		return (1);
+		if (zfs_prop_get(zfs_hdl, ZFS_PROP_MOUNTPOINT, zfs_mnt,
+		    BE_MAXPATHLEN, NULL, NULL, 0, 1))
+			return (1);
 
-	if (strcmp("none", zfs_mnt) == 0) {
 		/*
-		 * mountpoint=none; we'll mount it at info->mountpoint assuming
-		 * we're at the root.  If we're not at the root, we're likely
-		 * at some intermediate dataset (e.g. zroot/var) that will have
-		 * children that may need to be mounted.
+		 * We've encountered mountpoint=none at some intermediate
+		 * dataset (e.g. zroot/var) that will have children that may
+		 * need to be mounted.  Skip mounting it, but iterate through
+		 * the children.
 		 */
-		if (info->depth > 0)
+		if (strcmp("none", zfs_mnt) == 0)
 			goto skipmount;
 
-		snprintf(tmp, BE_MAXPATHLEN, "%s", info->mountpoint);
-	} else {
 		mountpoint = be_mountpoint_augmented(info->lbh, zfs_mnt);
 		snprintf(tmp, BE_MAXPATHLEN, "%s%s", info->mountpoint,
 		    mountpoint);

Modified: stable/12/sbin/bectl/bectl.c
==============================================================================
--- stable/12/sbin/bectl/bectl.c	Fri Jul 26 01:49:28 2019	(r350343)
+++ stable/12/sbin/bectl/bectl.c	Fri Jul 26 01:49:58 2019	(r350344)
@@ -184,7 +184,8 @@ bectl_cmd_activate(int argc, char *argv[])
 static int
 bectl_cmd_create(int argc, char *argv[])
 {
-	char *atpos, *bootenv, *snapname, *source;
+	char snapshot[BE_MAXPATHLEN];
+	char *atpos, *bootenv, *snapname;
 	int err, opt;
 	bool recursive;
 
@@ -214,6 +215,8 @@ bectl_cmd_create(int argc, char *argv[])
 	}
 
 	bootenv = *argv;
+
+	err = BE_ERR_SUCCESS;
 	if ((atpos = strchr(bootenv, '@')) != NULL) {
 		/*
 		 * This is the "create a snapshot variant". No new boot
@@ -221,24 +224,22 @@ bectl_cmd_create(int argc, char *argv[])
 		 */
 		*atpos++ = '\0';
 		err = be_snapshot(be, bootenv, atpos, recursive, NULL);
-	} else if (snapname != NULL) {
-		if (strchr(snapname, '@') != NULL)
-			err = be_create_from_existing_snap(be, bootenv,
-			    snapname);
-		else
-			err = be_create_from_existing(be, bootenv, snapname);
 	} else {
-		if ((snapname = strchr(bootenv, '@')) != NULL) {
-			*(snapname++) = '\0';
-			if ((err = be_snapshot(be, be_active_path(be),
-			    snapname, true, NULL)) != BE_ERR_SUCCESS)
-				fprintf(stderr, "failed to create snapshot\n");
-			asprintf(&source, "%s@%s", be_active_path(be), snapname);
-			err = be_create_from_existing_snap(be, bootenv,
-			    source);
-			return (err);
-		} else
-			err = be_create(be, bootenv);
+		if (snapname == NULL)
+			/* Create from currently booted BE */
+			err = be_snapshot(be, be_active_path(be), NULL,
+			    recursive, snapshot);
+		else if (strchr(snapname, '@') != NULL)
+			/* Create from given snapshot */
+			strlcpy(snapshot, snapname, sizeof(snapshot));
+		else
+			/* Create from given BE */
+			err = be_snapshot(be, snapname, NULL, recursive,
+			    snapshot);
+
+		if (err == BE_ERR_SUCCESS)
+			err = be_create_depth(be, bootenv, snapshot,
+					      recursive == true ? -1 : 0);
 	}
 
 	switch (err) {

Modified: stable/12/sbin/bectl/tests/bectl_test.sh
==============================================================================
--- stable/12/sbin/bectl/tests/bectl_test.sh	Fri Jul 26 01:49:28 2019	(r350343)
+++ stable/12/sbin/bectl/tests/bectl_test.sh	Fri Jul 26 01:49:58 2019	(r350344)
@@ -99,11 +99,35 @@ bectl_create_body()
 	mount=${cwd}/mnt
 
 	bectl_create_setup ${zpool} ${disk} ${mount}
+
+	# Create a child dataset that will be used to test creation
+	# of recursive and non-recursive boot environments.
+	atf_check zfs create -o mountpoint=/usr -o canmount=noauto \
+	    ${zpool}/ROOT/default/usr
+
 	# Test standard creation, creation of a snapshot, and creation from a
 	# snapshot.
 	atf_check bectl -r ${zpool}/ROOT create -e default default2
 	atf_check bectl -r ${zpool}/ROOT create default2@test_snap
 	atf_check bectl -r ${zpool}/ROOT create -e default2@test_snap default3
+
+	# Test standard creation, creation of a snapshot, and creation from a
+	# snapshot for recursive boot environments.
+	atf_check bectl -r ${zpool}/ROOT create -r -e default recursive
+	atf_check bectl -r ${zpool}/ROOT create -r recursive@test_snap
+	atf_check bectl -r ${zpool}/ROOT create -r -e recursive@test_snap recursive-snap
+
+	# Test that non-recursive boot environments have no child datasets.
+	atf_check -e not-empty -s not-exit:0 \
+		zfs list "${zpool}/ROOT/default2/usr"
+	atf_check -e not-empty -s not-exit:0 \
+		zfs list "${zpool}/ROOT/default3/usr"
+
+	# Test that recursive boot environments have child datasets.
+	atf_check -o not-empty \
+		zfs list "${zpool}/ROOT/recursive/usr"
+	atf_check -o not-empty \
+		zfs list "${zpool}/ROOT/recursive-snap/usr"
 }
 bectl_create_cleanup()
 {



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201907260149.x6Q1nx0K081444>