Use Cmdline for kargs fields in InstallConfigOpts and RootSetup

Plus all of the various places this trickles down.

Signed-off-by: John Eckersberg <[email protected]>

Author: jeckersb

crates/lib/src/bootc_composefs/boot.rs

@@ -5,6 +5,7 @@ use std::path::Path;
 
 use anyhow::{anyhow, Context, Result};
 use bootc_blockdev::find_parent_devices;
+use bootc_kernel_cmdline::utf8::Cmdline;
 use bootc_mount::inspect_filesystem_of_dir;
 use bootc_mount::tempmount::TempMount;
 use camino::{Utf8Path, Utf8PathBuf};
@@ -380,13 +381,17 @@ pub(crate) fn setup_composefs_bls_boot(
     let (root_path, esp_device, cmdline_refs, fs, bootloader) = match setup_type {
         BootSetupType::Setup((root_setup, state, fs)) => {
             // root_setup.kargs has [root=UUID=<UUID>, "rw"]
-            let mut cmdline_options = String::from(root_setup.kargs.join(" "));
+            let mut cmdline_options = Cmdline::new();
 
-            if state.composefs_options.insecure {
-                cmdline_options.push_str(&format!(" {COMPOSEFS_CMDLINE}=?{id_hex}"));
+            cmdline_options.extend(&root_setup.kargs);
+
+            let composefs_cmdline = if state.composefs_options.insecure {
+                format!("{COMPOSEFS_CMDLINE}=?{id_hex}")
             } else {
-                cmdline_options.push_str(&format!(" {COMPOSEFS_CMDLINE}={id_hex}"));
-            }
+                format!("{COMPOSEFS_CMDLINE}={id_hex}")
+            };
+
+            cmdline_options.extend(&Cmdline::from(&composefs_cmdline));
 
             // Locate ESP partition device
             let esp_part = esp_in(&root_setup.device_info)?;
@@ -407,12 +412,10 @@ pub(crate) fn setup_composefs_bls_boot(
             (
                 Utf8PathBuf::from("/sysroot"),
                 get_esp_partition(&sysroot_parent)?.0,
-                [
-                    format!("root=UUID={}", this_arch_root()),
-                    RW_KARG.to_string(),
-                    format!("{COMPOSEFS_CMDLINE}={id_hex}"),
-                ]
-                .join(" "),
+                Cmdline::from(format!(
+                    "root=UUID={} {RW_KARG} {COMPOSEFS_CMDLINE}={id_hex}",
+                    this_arch_root()
+                )),
                 fs,
                 bootloader,
             )

crates/lib/src/install.rs

@@ -196,7 +196,7 @@ pub(crate) struct InstallConfigOpts {
     ///
     /// Example: --karg=nosmt --karg=console=ttyS0,115200n8
     #[clap(long)]
-    pub(crate) karg: Option<Vec<String>>,
+    pub(crate) karg: Option<Vec<CmdlineOwned>>,
 
     /// The path to an `authorized_keys` that will be injected into the `root` account.
     ///
@@ -920,7 +920,6 @@ async fn install_container(
         merged_ostree_root.downcast_ref().unwrap(),
         std::env::consts::ARCH,
     )?;
-    let kargsd = kargsd.iter_str().collect::<Vec<_>>();
 
     // If the target uses aboot, then we need to set that bootloader in the ostree
     // config before deploying the commit
@@ -942,28 +941,36 @@ async fn install_container(
     }
 
     // Keep this in sync with install/completion.rs for the Anaconda fixups
-    let install_config_kargs = state
-        .install_config
-        .as_ref()
-        .and_then(|c| c.kargs.as_ref())
-        .into_iter()
-        .flatten()
-        .map(|s| s.as_str());
+    let install_config_kargs = state.install_config.as_ref().and_then(|c| c.kargs.as_ref());
+
     // Final kargs, in order:
     // - root filesystem kargs
     // - install config kargs
     // - kargs.d from container image
     // - args specified on the CLI
-    let kargs = root_setup
-        .kargs
-        .iter()
-        .map(|v| v.as_str())
-        .chain(install_config_kargs)
-        .chain(kargsd)
-        .chain(state.config_opts.karg.iter().flatten().map(|v| v.as_str()))
-        .collect::<Vec<_>>();
+    let mut kargs = Cmdline::new();
+
+    kargs.extend(&root_setup.kargs);
+
+    if let Some(install_config_kargs) = install_config_kargs {
+        for karg in install_config_kargs {
+            kargs.extend(&Cmdline::from(karg.as_str()));
+        }
+    }
+
+    kargs.extend(&kargsd);
+
+    if let Some(cli_kargs) = state.config_opts.karg.as_ref() {
+        for karg in cli_kargs {
+            kargs.extend(karg);
+        }
+    }
+
+    // Finally map into &[&str] for ostree_container
+    let kargs_strs: Vec<&str> = kargs.iter_str().collect();
+
     let mut options = ostree_container::deploy::DeployOpts::default();
-    options.kargs = Some(kargs.as_slice());
+    options.kargs = Some(kargs_strs.as_slice());
     options.target_imgref = Some(&state.target_imgref);
     options.proxy_cfg = proxy_cfg;
     options.skip_completion = true; // Must be set to avoid recursion!
@@ -1077,7 +1084,7 @@ pub(crate) struct RootSetup {
     /// True if we should skip finalizing
     skip_finalize: bool,
     boot: Option<MountSpec>,
-    pub(crate) kargs: Vec<String>,
+    pub(crate) kargs: CmdlineOwned,
 }
 
 fn require_boot_uuid(spec: &MountSpec) -> Result<&str> {
@@ -1631,7 +1638,7 @@ async fn install_to_filesystem_impl(
     cleanup: Cleanup,
 ) -> Result<()> {
     if matches!(state.selinux_state, SELinuxFinalState::ForceTargetDisabled) {
-        rootfs.kargs.push("selinux=0".to_string());
+        rootfs.kargs.extend(&Cmdline::from("selinux=0"));
     }
     // Drop exclusive ownership since we're done with mutation
     let rootfs = &*rootfs;
@@ -2167,6 +2174,8 @@ pub(crate) async fn install_to_filesystem(
         kargs.push(bootarg);
     }
 
+    let kargs = Cmdline::from(kargs.join(" "));
+
     let skip_finalize =
         matches!(fsopts.replace, Some(ReplaceMode::Alongside)) || fsopts.skip_finalize;
     let mut rootfs = RootSetup {

crates/lib/src/install/baseline.rs

@@ -30,6 +30,7 @@ use super::State;
 use super::RUN_BOOTC;
 use super::RW_KARG;
 use crate::task::Task;
+use bootc_kernel_cmdline::utf8::Cmdline;
 #[cfg(feature = "install-to-disk")]
 use bootc_mount::is_mounted_in_pid1_mountns;
 
@@ -424,13 +425,30 @@ pub(crate) fn install_create_rootfs(
         fstype: MountSpec::AUTO.into(),
         options: Some("ro".into()),
     });
-    let kargs = root_blockdev_kargs
-        .into_iter()
-        .flatten()
-        .chain([rootarg, RW_KARG.to_string()].into_iter())
-        .chain(bootarg)
-        .chain(state.config_opts.karg.clone().into_iter().flatten())
-        .collect::<Vec<_>>();
+
+    let mut kargs = Cmdline::new();
+
+    // Add root blockdev kargs (e.g., LUKS parameters)
+    if let Some(root_blockdev_kargs) = root_blockdev_kargs {
+        for karg in root_blockdev_kargs {
+            kargs.extend(&Cmdline::from(karg.as_str()));
+        }
+    }
+
+    // Add root= and rw argument
+    kargs.extend(&Cmdline::from(format!("{rootarg} {RW_KARG}")));
+
+    // Add boot= argument if present
+    if let Some(bootarg) = bootarg {
+        kargs.extend(&Cmdline::from(bootarg.as_str()));
+    }
+
+    // Add CLI kargs
+    if let Some(cli_kargs) = state.config_opts.karg.as_ref() {
+        for karg in cli_kargs {
+            kargs.extend(karg);
+        }
+    }
 
     bootc_mount::mount(&rootdev, &physical_root_path)?;
     let target_rootfs = Dir::open_ambient_dir(&physical_root_path, cap_std::ambient_authority())?;

crates/lib/src/parsers/bls_config.rs

@@ -5,7 +5,7 @@
 #![allow(dead_code)]
 
 use anyhow::{anyhow, Result};
-use bootc_kernel_cmdline::utf8::Cmdline;
+use bootc_kernel_cmdline::utf8::{Cmdline, CmdlineOwned};
 use camino::Utf8PathBuf;
 use composefs_boot::bootloader::EFI_EXT;
 use core::fmt;
@@ -15,7 +15,7 @@ use uapi_version::Version;
 
 use crate::composefs_consts::COMPOSEFS_CMDLINE;
 
-#[derive(Debug, PartialEq, PartialOrd, Eq, Default)]
+#[derive(Debug, PartialEq, Eq, Default)]
 pub enum BLSConfigType {
     EFI {
         /// The path to the EFI binary, usually a UKI
@@ -27,7 +27,7 @@ pub enum BLSConfigType {
         /// The paths to the initrd images.
         initrd: Vec<Utf8PathBuf>,
         /// Kernel command line options.
-        options: Option<String>,
+        options: Option<CmdlineOwned>,
     },
     #[default]
     Unknown,
@@ -229,7 +229,7 @@ pub(crate) fn parse_bls_config(input: &str) -> Result<BLSConfig> {
                 "version" => version = Some(value),
                 "linux" => linux = Some(Utf8PathBuf::from(value)),
                 "initrd" => initrd.push(Utf8PathBuf::from(value)),
-                "options" => options = Some(value),
+                "options" => options = Some(CmdlineOwned::from(value)),
                 "machine-id" => machine_id = Some(value),
                 "sort-key" => sort_key = Some(value),
                 "efi" => efi = Some(Utf8PathBuf::from(value)),
@@ -301,7 +301,7 @@ mod tests {
         assert_eq!(config.version, "2");
         assert_eq!(linux, "/boot/7e11ac46e3e022053e7226a20104ac656bf72d1a84e3a398b7cce70e9df188b6/vmlinuz-5.14.10");
         assert_eq!(initrd, vec!["/boot/7e11ac46e3e022053e7226a20104ac656bf72d1a84e3a398b7cce70e9df188b6/initramfs-5.14.10.img"]);
-        assert_eq!(options, Some("root=UUID=abc123 rw composefs=7e11ac46e3e022053e7226a20104ac656bf72d1a84e3a398b7cce70e9df188b6".to_string()));
+        assert_eq!(&*options.unwrap(), "root=UUID=abc123 rw composefs=7e11ac46e3e022053e7226a20104ac656bf72d1a84e3a398b7cce70e9df188b6");
         assert_eq!(config.extra.get("custom1"), Some(&"value1".to_string()));
         assert_eq!(config.extra.get("custom2"), Some(&"value2".to_string()));