git: 5d0826017fc9 - main - libbe: promote activated BEs all the way

The branch main has been updated by kevans:

URL: https://cgit.FreeBSD.org/src/commit/?id=5d0826017fc9b84f50ef19b0d452bc0590072f2d

commit 5d0826017fc9b84f50ef19b0d452bc0590072f2d
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2023-07-11 05:43:51 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2023-07-11 05:44:13 +0000

    libbe: promote activated BEs all the way
    
    This matches the beadm behavior; generally, we need to keep promoting
    until the BE is no longer a clone from a snapshot.  This fixes scenarios
    where the dataset associated with a BE's origin is itself a clone,
    activating the BE previously would promote it to a clone of the origin's
    origin.
    
    We could keep using be_get_dataset_props here, except for two
    annoyances:
    
    1.) I couldn't find a clean way to just clear an nvlist rather than
        having to re-alloc it, and I didn't want to just remove the one prop
        we're inspecting out of it.
    
    2.) That's a lot of overhead when all we want to do is fetch the origin
        anyways.
    
    Note that this is not a complete fix, but it does fix the majority of
    cases; deep BE subordinates are still notably broken, pending a patch
    from Christian.
    
    Reported by:    R. Christian McDonald <rcm@rcm.sh>
    Reviewed by:    rew
    Differential Revision:  https://reviews.freebsd.org/D40903
---
 lib/libbe/be.c                 | 30 +++++++++++-----------
 sbin/bectl/tests/bectl_test.sh | 56 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 16 deletions(-)

diff --git a/lib/libbe/be.c b/lib/libbe/be.c
index 753fde746e31..31836c623c4a 100644
--- a/lib/libbe/be.c
+++ b/lib/libbe/be.c
@@ -1269,9 +1269,7 @@ be_deactivate(libbe_handle_t *lbh, const char *ds, bool temporary)
 int
 be_activate(libbe_handle_t *lbh, const char *bootenv, bool temporary)
 {
-	char be_path[BE_MAXPATHLEN];
-	nvlist_t *dsprops;
-	const char *origin;
+	char be_path[BE_MAXPATHLEN], origin[BE_MAXPATHLEN];
 	zfs_handle_t *zhp;
 	int err;
 
@@ -1294,23 +1292,23 @@ be_activate(libbe_handle_t *lbh, const char *bootenv, bool temporary)
 		if (err)
 			return (-1);
 
-		zhp = zfs_open(lbh->lzh, be_path, ZFS_TYPE_FILESYSTEM);
-		if (zhp == NULL)
-			return (-1);
+		for (;;) {
+			zhp = zfs_open(lbh->lzh, be_path, ZFS_TYPE_FILESYSTEM);
+			if (zhp == NULL)
+				return (-1);
 
-		if (be_prop_list_alloc(&dsprops) != 0)
-			return (-1);
-
-		if (be_get_dataset_props(lbh, be_path, dsprops) != 0) {
-			nvlist_free(dsprops);
-			return (-1);
-		}
+			if (zfs_prop_get(zhp, ZFS_PROP_ORIGIN, origin, sizeof(origin),
+				NULL, NULL, 0, 1) != 0) {
+				zfs_close(zhp);
+				break;
+			}
 
-		if (nvlist_lookup_string(dsprops, "origin", &origin) == 0)
 			err = zfs_promote(zhp);
-		nvlist_free(dsprops);
+			zfs_close(zhp);
+			if (err)
+				break;
+		}
 
-		zfs_close(zhp);
 
 		if (err)
 			return (-1);
diff --git a/sbin/bectl/tests/bectl_test.sh b/sbin/bectl/tests/bectl_test.sh
index 5f865b86954e..7ca7af8a1abf 100755
--- a/sbin/bectl/tests/bectl_test.sh
+++ b/sbin/bectl/tests/bectl_test.sh
@@ -525,6 +525,61 @@ bectl_jail_cleanup()
 	bectl_cleanup ${zpool}
 }
 
+atf_test_case bectl_promotion cleanup
+bectl_promotion_head()
+{
+
+	atf_set "descr" "Check bectl promotion upon activation"
+	atf_set "require.user" root
+}
+bectl_promotion_body()
+{
+	if [ "$(atf_config_get ci false)" = "true" ] && \
+		[ "$(uname -p)" = "i386" ]; then
+		atf_skip "https://bugs.freebsd.org/249055"
+	fi
+
+	if [ "$(atf_config_get ci false)" = "true" ] && \
+		[ "$(uname -p)" = "armv7" ]; then
+		atf_skip "https://bugs.freebsd.org/249229"
+	fi
+
+	cwd=$(realpath .)
+	zpool=$(make_zpool_name)
+	disk=${cwd}/disk.img
+	mount=${cwd}/mnt
+	root=${mount}/root
+
+	bectl_create_setup ${zpool} ${disk} ${mount}
+	atf_check mkdir -p ${root}
+
+	# Sleeps interspersed to workaround some naming quirks; notably,
+	# bectl will append a serial if two snapshots were created within
+	# the same second, but it can only do that for the one root it's
+	# operating on.  It won't check that other roots don't have a snapshot
+	# with the same name, and the promotion will fail.
+	atf_check bectl -r ${zpool}/ROOT rename default A
+	sleep 1
+	atf_check bectl -r ${zpool}/ROOT create -e A B
+	sleep 1
+	atf_check bectl -r ${zpool}/ROOT create -e B C
+
+	# C should be a clone of B to start with
+	atf_check -o not-inline:"-" zfs list -H -o origin ${zpool}/ROOT/C
+
+	# Activating it should then promote it all the way out of clone-hood.
+	# This entails two promotes internally, as the first would promote it to
+	# a snapshot of A before finally promoting it the second time out of
+	# clone status.
+	atf_check -o not-empty bectl -r ${zpool}/ROOT activate C
+	atf_check -o inline:"-\n" zfs list -H -o origin ${zpool}/ROOT/C
+}
+bectl_promotion_cleanup()
+{
+
+	bectl_cleanup $(get_zpool_name)
+}
+
 atf_init_test_cases()
 {
 	atf_add_test_case bectl_create
@@ -534,4 +589,5 @@ atf_init_test_cases()
 	atf_add_test_case bectl_mount
 	atf_add_test_case bectl_rename
 	atf_add_test_case bectl_jail
+	atf_add_test_case bectl_promotion
 }