kargs: Handle addition/removal of kargs from kargs.d

Signed-off-by: Pragyan Poudyal <[email protected]>

Author: Johan-Liebert1

crates/lib/src/bootc_composefs/boot.rs

@@ -445,10 +445,10 @@ pub(crate) fn setup_composefs_bls_boot(
         }
     };
 
-    kargs_from_composefs_filesystem(fs, &repo, &mut cmdline_refs)?;
-
     let is_upgrade = matches!(setup_type, BootSetupType::Upgrade(..));
 
+    kargs_from_composefs_filesystem(fs, &repo, &mut cmdline_refs, !is_upgrade)?;
+
     let (entry_paths, _tmpdir_guard) = match bootloader {
         Bootloader::Grub => {
             let root = Dir::open_ambient_dir(&root_path, ambient_authority())

crates/lib/src/bootc_kargs.rs

@@ -2,6 +2,7 @@
 use anyhow::{Context, Result};
 use bootc_kernel_cmdline::utf8::{Cmdline, CmdlineOwned};
 use camino::Utf8Path;
+use cap_std_ext::cap_std::ambient_authority;
 use cap_std_ext::cap_std::fs::Dir;
 use cap_std_ext::cap_std::fs_utf8::Dir as DirUtf8;
 use cap_std_ext::dirext::CapStdExtDirExt;
@@ -49,12 +50,52 @@ impl Config {
     }
 }
 
+/// Compute the diff between existing and remote kargs
+/// Apply the diff to the new kargs
+fn compute_apply_kargs_diff(
+    existing_kargs: &Cmdline,
+    remote_kargs: &Cmdline,
+    new_kargs: &mut Cmdline,
+) {
+    // Calculate the diff between the existing and remote kargs
+    let added_kargs: Vec<_> = remote_kargs
+        .iter()
+        .filter(|item| !existing_kargs.iter().any(|existing| *item == existing))
+        .collect();
+    let removed_kargs: Vec<_> = existing_kargs
+        .iter()
+        .filter(|item| !remote_kargs.iter().any(|remote| *item == remote))
+        .collect();
+
+    tracing::debug!("kargs: added={:?} removed={:?}", added_kargs, removed_kargs);
+
+    // Apply the diff to the system kargs
+    for arg in &removed_kargs {
+        new_kargs.remove_exact(arg);
+    }
+    for arg in &added_kargs {
+        new_kargs.add(arg);
+    }
+}
+
 /// Looks for files in usr/lib/bootc/kargs.d and parses cmdline agruments
 pub(crate) fn kargs_from_composefs_filesystem(
     fs: &ComposefsFilesystem,
     repo: &ComposefsRepository,
-    cmdline: &mut Cmdline,
+    new_kargs: &mut Cmdline,
+    fresh_install: bool,
 ) -> Result<()> {
+    let existing_kargs = match fresh_install {
+        // If we are freshly installing, we won't have any existing kargs
+        true => Cmdline::new(),
+
+        false => {
+            let root = Dir::open_ambient_dir("/", ambient_authority()).context("Opening root")?;
+            let existing_kargs = get_kargs_in_root(&root, std::env::consts::ARCH)?;
+            existing_kargs
+        }
+    };
+
     let kargs_d = fs
         .root
         .get_directory_opt(OsStr::new(KARGS_PATH))
@@ -64,12 +105,18 @@ pub(crate) fn kargs_from_composefs_filesystem(
         return Ok(());
     };
 
+    let mut remote_kargs = Cmdline::new();
+
     for (fname, inode) in kargs_d.sorted_entries() {
         let Inode::Leaf(..) = inode else { continue };
 
-        let fname_str = fname
-            .to_str()
-            .ok_or_else(|| anyhow::anyhow!("Failed to get filename as string"))?;
+        let Some(fname_str) = fname.to_str() else {
+            tracing::warn!(
+                "Failed to convert filename to UTF-8, will skip parsing: {:?}",
+                fname
+            );
+            continue;
+        };
 
         if !Config::filename_matches(fname_str) {
             continue;
@@ -84,10 +131,12 @@ pub(crate) fn kargs_from_composefs_filesystem(
             .with_context(|| format!("File {fname_str} is not a valid UTF-8"))?;
 
         if let Some(kargs) = parse_kargs_toml(contents, std::env::consts::ARCH)? {
-            cmdline.extend(&kargs);
+            remote_kargs.extend(&kargs);
         };
     }
 
+    compute_apply_kargs_diff(&existing_kargs, &remote_kargs, new_kargs);
+
     Ok(())
 }
 
@@ -221,29 +270,7 @@ pub(crate) fn get_kargs(
     // Fetch the kernel arguments from the new root
     let remote_kargs = get_kargs_from_ostree(repo, &fetched_tree, sys_arch)?;
 
-    // Calculate the diff between the existing and remote kargs
-    let added_kargs: Vec<_> = remote_kargs
-        .iter()
-        .filter(|item| !existing_kargs.iter().any(|existing| *item == existing))
-        .collect();
-    let removed_kargs: Vec<_> = existing_kargs
-        .iter()
-        .filter(|item| !remote_kargs.iter().any(|remote| *item == remote))
-        .collect();
-
-    tracing::debug!(
-        "kargs: added={:?} removed={:?}",
-        &added_kargs,
-        removed_kargs
-    );
-
-    // Apply the diff to the system kargs
-    for arg in &removed_kargs {
-        kargs.remove_exact(arg);
-    }
-    for arg in &added_kargs {
-        kargs.add(arg);
-    }
+    compute_apply_kargs_diff(&existing_kargs, &remote_kargs, &mut kargs);
 
     Ok(kargs)
 }