install: create temporary directory for ESP bls mount

Plus additional review comments:
- Created constant for EFI/LINUX
- Switched from Task to Command
- Create efi_dir as Utf8PathBuf

Signed-off-by: Robert Sturla <[email protected]>

Author: p5

crates/lib/src/install.rs

@@ -98,6 +98,8 @@ use bootc_mount::{inspect_filesystem, Filesystem};
 
 /// The toplevel boot directory
 const BOOT: &str = "boot";
+/// The EFI Linux directory
+const EFI_LINUX: &str = "EFI/Linux";
 /// Directory for transient runtime state
 #[cfg(feature = "install-to-disk")]
 const RUN_BOOTC: &str = "/run/bootc";
@@ -1691,7 +1693,7 @@ pub(crate) fn setup_composefs_bls_boot(
 ) -> Result<String> {
     let id_hex = id.to_hex();
 
-    let (root_path, esp_device, cmdline_refs) = match setup_type {
+    let (esp_device, cmdline_refs) = match setup_type {
         BootSetupType::Setup((root_setup, state)) => {
             // root_setup.kargs has [root=UUID=<UUID>, "rw"]
             let mut cmdline_options = String::from(root_setup.kargs.join(" "));
@@ -1713,11 +1715,7 @@ pub(crate) fn setup_composefs_bls_boot(
                 .find(|p| p.parttype.as_str() == ESP_GUID)
                 .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?;
 
-            (
-                root_setup.physical_root_path.clone(),
-                esp_part.node.clone(),
-                cmdline_options,
-            )
+            (esp_part.node.clone(), cmdline_options)
         }
 
         BootSetupType::Upgrade => {
@@ -1731,7 +1729,6 @@ pub(crate) fn setup_composefs_bls_boot(
             };
 
             (
-                sysroot,
                 get_esp_partition(&parent)?.0,
                 vec![
                     format!("root=UUID={DPS_UUID}"),
@@ -1743,18 +1740,20 @@ pub(crate) fn setup_composefs_bls_boot(
         }
     };
 
-    let mounted_esp: PathBuf = root_path.join("esp").into();
-    let esp_mount_point_existed = mounted_esp.exists();
-
-    create_dir_all(&mounted_esp).context("Failed to create dir {mounted_esp:?}")?;
+    let temp_efi_dir = tempfile::tempdir()
+        .map_err(|e| anyhow::anyhow!("Failed to create temporary directory for EFI mount: {e}"))?;
+    let mounted_efi = temp_efi_dir.path().to_path_buf();
 
-    Task::new("Mounting ESP", "mount")
-        .args([&PathBuf::from(&esp_device), &mounted_esp.clone()])
-        .run()?;
+    Command::new("mount")
+        .args([&PathBuf::from(&esp_device), &mounted_efi])
+        .log_debug()
+        .run_inherited_with_cmd_context()
+        .context("Mounting EFI")?;
 
     let is_upgrade = matches!(setup_type, BootSetupType::Upgrade);
 
-    let efi_dir = mounted_esp.join(format!("EFI/Linux"));
+    let efi_dir = Utf8PathBuf::from_path_buf(mounted_efi.join("EFI/Linux"))
+        .map_err(|_| anyhow::anyhow!("EFI dir is not valid UTF-8"))?;
     let (bls_config, boot_digest) = match &entry {
         ComposefsBootEntry::Type1(..) => unimplemented!(),
         ComposefsBootEntry::Type2(..) => unimplemented!(),
@@ -1768,18 +1767,16 @@ pub(crate) fn setup_composefs_bls_boot(
             bls_config.title = Some(id_hex.clone());
             bls_config.sort_key = Some("1".into());
             bls_config.machine_id = None;
-            bls_config.linux = format!("/EFI/Linux/{id_hex}/vmlinuz");
-            bls_config.initrd = vec![format!("/EFI/Linux/{id_hex}/initrd")];
+            bls_config.linux = format!("/{EFI_LINUX}/{id_hex}/vmlinuz");
+            bls_config.initrd = vec![format!("/{EFI_LINUX}/{id_hex}/initrd")];
             bls_config.options = Some(cmdline_refs);
             bls_config.extra = HashMap::new();
 
             if let Some(symlink_to) = find_vmlinuz_initrd_duplicates(&boot_digest)? {
-                bls_config.linux = format!("/EFI/Linux/{symlink_to}/vmlinuz");
-                bls_config.initrd = vec![format!("/EFI/Linux/{symlink_to}/initrd")];
+                bls_config.linux = format!("/{EFI_LINUX}/{symlink_to}/vmlinuz");
+                bls_config.initrd = vec![format!("/{EFI_LINUX}/{symlink_to}/initrd")];
             } else {
-                let efi_dir_utf8 = Utf8PathBuf::from_path_buf(efi_dir.clone())
-                    .map_err(|_| anyhow::anyhow!("EFI dir is not valid UTF-8"))?;
-                write_bls_boot_entries_to_disk(&efi_dir_utf8, id, usr_lib_modules_vmlinuz, &repo)?;
+                write_bls_boot_entries_to_disk(&efi_dir, id, usr_lib_modules_vmlinuz, &repo)?;
             }
 
             (bls_config, boot_digest)
@@ -1792,12 +1789,12 @@ pub(crate) fn setup_composefs_bls_boot(
 
         // This will be atomically renamed to 'loader/entries' on shutdown/reboot
         (
-            mounted_esp.join(format!("loader/{STAGED_BOOT_LOADER_ENTRIES}")),
+            mounted_efi.join(format!("loader/{STAGED_BOOT_LOADER_ENTRIES}")),
             Some(booted_bls),
         )
     } else {
         (
-            mounted_esp.join(format!("loader/{BOOT_LOADER_ENTRIES}")),
+            mounted_efi.join(format!("loader/{BOOT_LOADER_ENTRIES}")),
             None,
         )
     };
@@ -1836,16 +1833,11 @@ pub(crate) fn setup_composefs_bls_boot(
         rustix::fs::fsync(owned_loader_entries_fd).context("fsync")?;
     }
 
-    Task::new("Unmounting ESP", "umount")
-        .arg(&mounted_esp)
-        .run()?;
-
-    if !esp_mount_point_existed {
-        // This shouldn't be a fatal error
-        if let Err(e) = std::fs::remove_dir(&mounted_esp) {
-            tracing::error!("Failed to remove mount point '{mounted_esp:?}': {e}");
-        }
-    }
+    Command::new("umount")
+        .arg(&mounted_efi)
+        .log_debug()
+        .run_inherited_with_cmd_context()
+        .context("Unmounting EFI")?;
 
     Ok(boot_digest)
 }
@@ -1920,13 +1912,12 @@ pub(crate) fn setup_composefs_uki_boot(
         }
     };
 
-    let mounted_esp: PathBuf = root_path.join("esp").into();
-    let esp_mount_point_existed = mounted_esp.exists();
-
-    create_dir_all(&mounted_esp).context("Failed to create dir {mounted_esp:?}")?;
+    let temp_efi_dir = tempfile::tempdir()
+        .map_err(|e| anyhow::anyhow!("Failed to create temporary directory for EFI mount: {e}"))?;
+    let mounted_efi = temp_efi_dir.path().to_path_buf();
 
     Task::new("Mounting ESP", "mount")
-        .args([&PathBuf::from(&esp_device), &mounted_esp.clone()])
+        .args([&PathBuf::from(&esp_device), &mounted_efi.clone()])
         .run()?;
 
     let boot_label = match entry {
@@ -1968,7 +1959,7 @@ pub(crate) fn setup_composefs_uki_boot(
             }
 
             // Write the UKI to ESP
-            let efi_linux_path = mounted_esp.join("EFI/Linux");
+            let efi_linux_path = mounted_efi.join(EFI_LINUX);
             create_dir_all(&efi_linux_path).context("Creating EFI/Linux")?;
 
             let efi_linux =
@@ -1990,16 +1981,11 @@ pub(crate) fn setup_composefs_uki_boot(
         }
     };
 
-    Task::new("Unmounting ESP", "umount")
-        .arg(&mounted_esp)
-        .run()?;
-
-    if !esp_mount_point_existed {
-        // This shouldn't be a fatal error
-        if let Err(e) = std::fs::remove_dir(&mounted_esp) {
-            tracing::error!("Failed to remove mount point '{mounted_esp:?}': {e}");
-        }
-    }
+    Command::new("umount")
+        .arg(&mounted_efi)
+        .log_debug()
+        .run_inherited_with_cmd_context()
+        .context("Unmounting ESP")?;
 
     let boot_dir = root_path.join("boot");
     create_dir_all(&boot_dir).context("Failed to create boot dir")?;