From 57b55727366435159bb6f891a32e3c80c865aaa1 Mon Sep 17 00:00:00 2001
From: Zach Dykstra <dykstra.zachary@gmail.com>
Date: Tue, 3 Oct 2023 16:42:57 -0500
Subject: [PATCH] zfsbootmenu: remove legacy root= usage/design

ZFSBootMenu has danced around `$root` for a few years now, as a holdover from
very early design decisions picked up from Dracut. We don't actually _need_ to
tie ourselves to that variable - and in fact, a lot of the early startup process
can be simplified by no longer trying to stuff so much logic into it.

To that end, the legacy but undocumented support for
`root=zfsbootmenu:POOL=pool` has been completely removed. `root` on the ZBM KCL
is no longer honored. `zbm.prefer` with the documented syntax is now the only
way to tell ZFSBootMenu which pool should be preferentially used in the boot
process. Accordingly, the global `zbm_prefer_pool` variable is now set when a
pool should be preferred, and the accompanying `zbm_require_pool` variable is
used to indicate how firm that pool requirement is. One quirk of this is that a
pool name of `zfsbootmenu` can now actually be a value that `zbm.prefer` will
respect/honor.
---
 zfsbootmenu/lib/zfsbootmenu-core.sh           |  2 +-
 zfsbootmenu/libexec/zfsbootmenu-init          | 30 +++++------
 .../pre-init/zfsbootmenu-parse-commandline.sh | 50 ++++++-------------
 zfsbootmenu/pre-init/zfsbootmenu-preinit.sh   |  4 +-
 4 files changed, 32 insertions(+), 54 deletions(-)

diff --git a/zfsbootmenu/lib/zfsbootmenu-core.sh b/zfsbootmenu/lib/zfsbootmenu-core.sh
index e165bf216..08b1de5bd 100755
--- a/zfsbootmenu/lib/zfsbootmenu-core.sh
+++ b/zfsbootmenu/lib/zfsbootmenu-core.sh
@@ -154,7 +154,7 @@ match_hostid() {
         state*)
           state="${line#state: }"
           # shellcheck disable=SC2154
-          if [ "${state}" == "ONLINE" ] && [ -n "${pool}" ] && [ "${pool}" != "${root}" ]; then
+          if [ "${state}" == "ONLINE" ] && [ -n "${pool}" ] && [ "${pool}" != "${zbm_prefer_pool}" ]; then
             importable+=("${pool}")
             pool=""
           fi
diff --git a/zfsbootmenu/libexec/zfsbootmenu-init b/zfsbootmenu/libexec/zfsbootmenu-init
index 7b5f9814f..5631d280a 100755
--- a/zfsbootmenu/libexec/zfsbootmenu-init
+++ b/zfsbootmenu/libexec/zfsbootmenu-init
@@ -105,16 +105,9 @@ fi
 tput clear
 /libexec/zfsbootmenu-run-hooks "early-setup.d"
 
-# Prefer a specific pool when checking for a bootfs value
-# shellcheck disable=SC2154
-if [ "${root}" = "zfsbootmenu" ]; then
-  boot_pool=
-else
-  boot_pool="${root}"
-fi
-
 # If a boot pool is specified, that will be tried first
-try_pool="${boot_pool}"
+# shellcheck disable=SC2154
+try_pool="${zbm_prefer_pool}"
 zbm_import_attempt=0
 
 while true; do
@@ -126,7 +119,7 @@ while true; do
 
   # shellcheck disable=SC2154
   if check_for_pools; then
-    if [ "${zbm_require_bpool}" = "only" ]; then
+    if [ "${zbm_require_pool}" = "only" ]; then
       zdebug "only importing ${try_pool}"
       break
     elif [ -n "${try_pool}" ]; then
@@ -148,15 +141,15 @@ while true; do
     echo -n "$spl_hostid" > "${BASE}/spl_hostid"
 
     # If match_hostid succeeds, it has imported *a* pool...
-    if [ -n "${try_pool}" ] && [ "${zbm_require_bpool}" = "only" ]; then
-      # In "only" bpool mode, the import was the sole pool desired; nothing more to do
+    if [ -n "${try_pool}" ] && [ "${zbm_require_pool}" = "only" ]; then
+      # In "only" pool mode, the import was the sole pool desired; nothing more to do
       break
     else
       # Otherwise, try one more pass to pick up other pools matching this hostid
       try_pool=""
       continue
     fi
-  elif [ -n "${try_pool}" ] && [ -z "${zbm_require_bpool}" ]; then
+  elif [ -n "${try_pool}" ] && [ -z "${zbm_require_pool}" ]; then
     # If a specific pool was tried unsuccessfully but is not a requirement,
     # allow another pass to try any other importable pools
     try_pool=""
@@ -186,6 +179,7 @@ while IFS=$'\t' read -r _pool _health; do
     zerror "prohibiting read/write operations on ${_pool}"
   fi
 done <<<"$( zpool list -H -o name,health )"
+unset _pool _health
 
 zdebug && zdebug "$( zreport )"
 
@@ -205,17 +199,19 @@ if [ "${unsupported}" -ne 0 ]; then
   timed_prompt -m "$( colorize red 'Unsupported features detected')" \
     -m "$( colorize red 'Upgrade ZFS modules in ZFSBootMenu with generate-zbm')"
 fi
+unset unsupported
 
 # Attempt to find the bootfs property
 # shellcheck disable=SC2086
-while read -r line; do
-  if [ "${line}" = "-" ]; then
+while read -r _bootfs; do
+  if [ "${_bootfs}" = "-" ]; then
     BOOTFS=
   else
-    BOOTFS="${line}"
+    BOOTFS="${_bootfs}"
     break
   fi
-done <<<"$( zpool list -H -o bootfs ${boot_pool} )"
+done <<<"$( zpool list -H -o bootfs "${zbm_prefer_pool:---}" )"
+unset _bootfs
 
 if [ -n "${BOOTFS}" ]; then
   export BOOTFS
diff --git a/zfsbootmenu/pre-init/zfsbootmenu-parse-commandline.sh b/zfsbootmenu/pre-init/zfsbootmenu-parse-commandline.sh
index 9daddfda2..1954c67d8 100755
--- a/zfsbootmenu/pre-init/zfsbootmenu-parse-commandline.sh
+++ b/zfsbootmenu/pre-init/zfsbootmenu-parse-commandline.sh
@@ -199,11 +199,6 @@ else
   zinfo "disabling automatic replacement of spl_hostid"
 fi
 
-# rewrite root=
-if prefer=$( get_zbm_arg zbm.prefer ) ; then
-  root="zfsbootmenu:POOL=${prefer}"
-fi
-
 if kcl_override=$( get_zbm_arg zbm.kcl_override ) ; then
   # Remove the leading /  trailing quote to "unpack" this argument
   kcl_override="${kcl_override#\"}"
@@ -224,46 +219,33 @@ if kcl_override=$( get_zbm_arg zbm.kcl_override ) ; then
   zinfo "overriding all BE KCLs with: '$( kcl_assemble < "${BASE}/cmdline" )'"
 fi
 
-wait_for_zfs=0
-case "${root}" in
-  zfsbootmenu:POOL=*)
-    # Prefer a specific pool for bootfs value, root=zfsbootmenu:POOL=zroot
-    root="${root#zfsbootmenu:POOL=}"
-    # shellcheck disable=SC2034
-    rootok=1
-    wait_for_zfs=1
-
-    zinfo "preferring ${root} for bootfs"
-    ;;
-  *)
-    root="zfsbootmenu"
-    # shellcheck disable=SC2034
-    rootok=1
-    wait_for_zfs=1
-
-    zinfo "enabling menu after udev settles"
-  ;;
-esac
+zbm_prefer_pool=
+if zbm_prefer_pool=$( get_zbm_arg zbm.prefer ) ; then
+  # shellcheck disable=SC2034
+  zbm_prefer_pool="${zbm_prefer_pool%%/*}"
+  zinfo "preferring ${zbm_prefer_pool} for bootfs"
+fi
 
 # pool! : this pool must be imported before all others
 # pool!!: this pool, and no others, must be imported
 
 # shellcheck disable=SC2034
-case "${root}" in
+case "${zbm_prefer_pool}" in
   *!!)
-    zbm_require_bpool="only"
-    root="${root%!!}"
+    zbm_require_pool="only"
+    zbm_prefer_pool="${zbm_prefer_pool%!!}"
     ;;
   *!)
-    zbm_require_bpool="yes"
-    root="${root%!}"
+    zbm_require_pool="yes"
+    zbm_prefer_pool="${zbm_prefer_pool%!}"
     ;;
   *)
-    zbm_require_bpool=""
+    zbm_require_pool=""
     ;;
 esac
 
 # Make sure Dracut is happy that we have a root
-if [ ${wait_for_zfs} -eq 1 ]; then
-  ln -s /dev/null /dev/root 2>/dev/null
-fi
+
+# shellcheck disable=SC2034
+rootok=1
+ln -s /dev/null /dev/root 2>/dev/null
diff --git a/zfsbootmenu/pre-init/zfsbootmenu-preinit.sh b/zfsbootmenu/pre-init/zfsbootmenu-preinit.sh
index 79394b261..e6c0f3ebf 100755
--- a/zfsbootmenu/pre-init/zfsbootmenu-preinit.sh
+++ b/zfsbootmenu/pre-init/zfsbootmenu-preinit.sh
@@ -26,8 +26,8 @@ export spl_hostid="${spl_hostid}"
 export import_policy="${import_policy}"
 export menu_timeout="${menu_timeout}"
 export loglevel="${loglevel}"
-export root="${root}"
-export zbm_require_bpool="${zbm_require_bpool}"
+export zbm_prefer_pool="${zbm_prefer_pool}"
+export zbm_require_pool="${zbm_require_pool}"
 export default_hostid=00bab10c
 export zbm_sort="${zbm_sort}"
 export zbm_set_hostid="${zbm_set_hostid}"