zfsbootmenu-core.sh: drop preload_be_cmdline, improve KCL caching

Co-authored-by: Zach Dykstra <dykstra.zachary@gmail.com>
Co-authored-by: Andrew J. Hesford <ajh@sideband.org>

Closes #260.

zfsbootmenu/bin/zfsbootmenu

@@ -275,11 +275,11 @@ while true; do
         def_args="${line}"
       done <<< "${BE_ARGS}"
 
-      echo -e "\nNew kernel command line"
+      echo -e "\nNew kernel command line (root= arguments will be ignored)"
       cmdline="$( /libexec/zfsbootmenu-input "${def_args}" )"
 
       if [ -n "${cmdline}" ] ; then
-        kcl_tokenize <<< "${cmdline}" > "${BASE}/cmdline"
+        kcl_tokenize <<< "${cmdline}" | kcl_suppress root > "${BASE}/cmdline"
       fi
       ;;
     "mod-j")

zfsbootmenu/lib/zfsbootmenu-core.sh

@@ -773,9 +773,6 @@ find_be_kernels() {
 
   zdebug "default kernel set to ${def_kernel}"
   echo "${def_kernel##*/}" > "${def_kernel_file}"
-
-  # Pre-load cmdline arguments, possibly from files in the environment
-  preload_be_cmdline "${fs}"
   return 0
 }
 
@@ -889,90 +886,100 @@ find_root_prefix() {
   echo "root=zfs:"
 }
 
-# arg1: ZFS filesystem
-# prints: nothing
-# returns: 0 on success
+# arg1: ZFS KCL cache to validate
+# returns: 0 if cache is valid, 1 otherwise
 
-preload_be_cmdline() {
-  local fs args args_file
+validate_cmdline_cache() {
+  local cf
+  cf="${1}"
 
-  fs="${1}"
-  if [ -z "${fs}" ]; then
-    zerror "fs is undefined"
-    return 1
-  fi
-  zdebug "fs set to ${fs}"
+  # Cache is trivially invalid when it fails to exist!
+  [ -r "${cf}" ] || return 1
 
-  args_file="${BASE}/${fs}/cmdline"
+  # Otherwise, only the noresume flag can invalidate
+  [ -e "${BASE}/noresume" ] || return 0
 
-  if args="$( read_kcl_prop "${fs}" )" && [ -n "${args}" ]; then
-    zdebug "using org.zfsbootmenu:commandline"
-    kcl_tokenize <<< "${args}" > "${args_file}"
-  fi
+  # Cache is still valid if it was written after noresume flag
+  [ "${cf}" -nt "${BASE}/noresume" ] && return 0
 
-  return 0
+  # By default, cache is invalid
+  return 1
 }
 
 # arg1: ZFS filesystem
 # prints: kernel command line arguments
 # returns: nothing
 
 load_be_cmdline() {
-  local zfsbe_fs zfsbe_args spl_hostid zfsbe_kcl
+  local fs args spl_hostid kcl cache rems adds
 
-  zfsbe_fs="${1}"
-  if [ -z "${zfsbe_fs}" ]; then
-    zerror "zfsbe_fs is undefined"
+  fs="${1}"
+  if [ -z "${fs}" ]; then
+    zerror "filesystem is undefined"
     return 1
   fi
-  zdebug "zfsbe_fs set to ${zfsbe_fs}"
+  zdebug "fs set to ${fs}"
 
-  # Always prefer a user-entered KCL
-  if [ -r "${BASE}/cmdline" ]; then
-    zdebug "using ${BASE}/cmdline as commandline for ${zfsbe_fs}"
+  cache="${BASE}/${fs}/cmdline"
 
-    # root= is ALWAYS controlled by ZFSBootMenu
-    kcl_suppress root < "${BASE}/cmdline" | kcl_assemble
+  if [ -r "${BASE}/cmdline" ]; then
+    # Always prefer a user-entered KCL
+    zdebug "using ${BASE}/cmdline as command line for ${fs}"
+    kcl_assemble < "${BASE}/cmdline"
+    return
+  elif validate_cmdline_cache "${cache}"; then
+    # Otherwise, if the BE has a valid KCL cache, just assemble that
+    zdebug "using cached KCL from ${cache} as command line for ${fs}"
+    kcl_assemble < "${cache}"
     return
   fi
 
-  # Default KCL is very basic
-  zfsbe_args="$( kcl_tokenize <<< "quiet loglevel=4" )"
+  # root= is ALWAYS controlled by ZFSBootMenu
+  rems=( "root" )
 
-  # Use a BE-specific KCL if one is preovided
-  if [ -r "${BASE}/${zfsbe_fs}/cmdline" ]; then
-    zdebug "using ${BASE}/${zfsbe_fs}/cmdline as commandline for ${zfsbe_fs}"
-    zfsbe_args="$( kcl_suppress root < "${BASE}/${zfsbe_fs}/cmdline" )"
-  fi
+  # Nothing is added by default
+  adds=()
+
+  # In all other cases, build and attempt to cache the KCL
+  args="$(read_kcl_prop "${fs}" | kcl_tokenize && exit "${PIPESTATUS[0]}" )" || args=""
+  # Use a very basic default KCL if none is specified
+  [ -n "${args}" ] || args="quiet loglevel=4"
 
   if [ -e "${BASE}/noresume" ]; then
-    zdebug "${BASE}/noresume set, processing: '${zfsbe_args}'"
-    # Must drop resume= arguments and append a noresume
-    zfsbe_args="$( kcl_suppress resume <<< "${zfsbe_args}" | kcl_append noresume )"
+    # Drop resume= arguments and append a noresume
+    zdebug "${BASE}/noresume set, expunging from ${fs}"
+    rems+=( "resume" )
+    adds+=( "noresume" )
   fi
 
   # shellcheck disable=SC2154
   if [ "${zbm_set_hostid:-0}" -eq 1 ] && spl_hostid="$( get_spl_hostid )"; then
-    zdebug "overriding spl_hostid and spl.spl_hostid for ${zfsbe_fs}"
+    zdebug "overriding spl_hostid and spl.spl_hostid for ${fs}"
 
     if [ "${spl_hostid}" = "0x00000000" ]; then
-      # spl.spl_hostid=0 is a no-op; imports fall back to /etc/hostid.
-      # Dracut writes spl_hostid to /etc/hostid. to yield expected results.
-      # Others (initramfs-tools, mkinitcpio) ignore this, but there isn't much
-      # else that can be done with those systems.
+      # spl.spl_hostid=0 is a no-op; imports fall back to /etc/hostid. Dracut
+      # writes spl_hostid to /etc/hostid to yield expected results. Others
+      # (initramfs-tools, mkinitcpio) ignore this, but there isn't much else
+      # that can be done with those systems.
       spl_hostid="spl_hostid=00000000"
     else
-      # Using spl.spl_hostid will set a module parameter which takes precedence
-      # over any /etc/hostid and should produce expected behavior in all systems
+      # Using spl.spl_hostid sets a module parameter which takes precedence
+      # over any /etc/hostid and should produce expected behavior everywhere
       spl_hostid="spl.spl_hostid=${spl_hostid}"
     fi
 
-    zfsbe_args="$( kcl_suppress spl_hostid spl.spl_hostid <<< "${zfsbe_args}" | kcl_append "${spl_hostid}" )"
+    rems+=( "spl_hostid" "spl.spl_hostid" )
+    adds+=( "${spl_hostid}" )
   fi
 
-  zfsbe_kcl="$( kcl_assemble <<< "${zfsbe_args}" )"
-  zdebug "assembled commandline: '${zfsbe_kcl}'"
-  echo "${zfsbe_kcl}"
+  # Write the cached command line, if possible
+  zdebug "caching KCL for ${fs} at ${cache}"
+  args="$( kcl_suppress "${rems[@]}" <<< "${args}" | kcl_append "${adds[@]}" )"
+  printf "%s\n" "${args}" > "${cache}"
+
+  kcl="$( kcl_assemble <<< "${args}" )"
+  zdebug "assembled commandline: '${kcl}'"
+  echo "${kcl}"
 }
 
 # arg1: pool name, empty to import all
@@ -1237,6 +1244,9 @@ resume_prompt() {
 
   # Try to avoid importing writable when a resume device is found
   if has_resume_device; then
+    # If NORESUME was already provided, never allow it to be taken back
+    [ -r "${BASE}/noresume" ] && return 0
+
     # Make sure the warning is prominent
     tput clear
     tput cnorm

zfsbootmenu/lib/zfsbootmenu-kcl.sh

@@ -112,7 +112,7 @@ kcl_suppress() {
     sup=0
     for rem in "$@"; do
       # Arguments match entirely or up to first equal
-      if [ "${arg}" = "${rem}" ] || [ "${arg%%=*}" = "${rem}" ]; then
+      if [[ "${arg}" == "${rem}" || "${arg%%=*}" == "${rem}" ]]; then
         sup=1
         break
       fi
@@ -132,9 +132,9 @@ kcl_append() {
   # Carry forward input KCL
   cat
 
-  # Append one line per argument
+  # Append one line per non-trivial argument
   for arg in "$@"; do
-    echo "$arg"
+    [ -n "${arg}" ] && echo "$arg"
   done
 }
 

zfsbootmenu/lib/zfsbootmenu-lib.sh

@@ -568,6 +568,9 @@ populate_be_list() {
 
   ret=1
   for fs in "${candidates[@]}"; do
+    # Remove any existing cmdline cache
+    rm -f "${BASE}/${fs}/cmdline"
+
     # Unlock if necessary
     load_key "${fs}" || continue