From 969d62812563195404e368969bb74a6ad32969e4 Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Sat, 19 Sep 2026 22:38:20 +0100 Subject: [PATCH] install: Only remove bootloader dirs from the ESP to-existing-root emptied the whole ESP, which can hold content bootc cannot recreate. On Asahi this includes m1n1/boot.bin, vendorfw/ and ubootefi.var; wiping them leaves the machine unbootable. Only remove EFI/ and loader/, which we reinstall. Factor out remove_dir_no_xdev() to share the recursive removal. Generated-by: AI Signed-off-by: Eric Curtin --- crates/lib/src/install.rs | 73 ++++++++++++++++++----- docs/src/bootc-install.md | 5 +- tmt/tests/booted/test-multi-device-esp.nu | 31 +++++++++- 3 files changed, 90 insertions(+), 19 deletions(-) diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index dfa32ad929..9e531dffb4 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -2327,12 +2327,7 @@ fn remove_all_in_dir_no_xdev(d: &Dir, mount_err: bool) -> Result<()> { let name = entry.file_name(); let etype = entry.file_type()?; if etype == FileType::dir() { - if let Some(subdir) = d.open_dir_noxdev(&name)? { - remove_all_in_dir_no_xdev(&subdir, mount_err)?; - d.remove_dir(&name)?; - } else if mount_err { - anyhow::bail!("Found unexpected mount point {name:?}"); - } + remove_dir_no_xdev(d, &name, mount_err)?; } else { d.remove_file_optional(&name)?; } @@ -2340,6 +2335,19 @@ fn remove_all_in_dir_no_xdev(d: &Dir, mount_err: bool) -> Result<()> { anyhow::Ok(()) } +/// Recursively remove the directory `name` in `d`, without crossing devices. +/// A mount point is left alone, or is an error if `mount_err` is true. +fn remove_dir_no_xdev(d: &Dir, name: impl AsRef, mount_err: bool) -> Result<()> { + let name = name.as_ref(); + if let Some(subdir) = d.open_dir_noxdev(name)? { + remove_all_in_dir_no_xdev(&subdir, mount_err)?; + d.remove_dir(name)?; + } else if mount_err { + anyhow::bail!("Found unexpected mount point {name:?}"); + } + Ok(()) +} + #[context("Removing boot directory content except loader dir on ostree")] fn remove_all_except_loader_dirs(bootdir: &Dir, is_ostree: bool) -> Result<()> { let entries = bootdir @@ -2364,12 +2372,8 @@ fn remove_all_except_loader_dirs(bootdir: &Dir, is_ostree: bool) -> Result<()> { let etype = entry.file_type()?; if etype == FileType::dir() { - // Open the directory and remove its contents - if let Some(subdir) = bootdir.open_dir_noxdev(&file_name)? { - remove_all_in_dir_no_xdev(&subdir, false) - .with_context(|| format!("Removing directory contents: {}", file_name))?; - bootdir.remove_dir(&file_name)?; - } + remove_dir_no_xdev(bootdir, file_name, false) + .with_context(|| format!("Removing directory: {file_name}"))?; } else { bootdir .remove_file_optional(&file_name) @@ -2379,6 +2383,22 @@ fn remove_all_except_loader_dirs(bootdir: &Dir, is_ostree: bool) -> Result<()> { Ok(()) } +/// Remove the bootloader dirs (bootupd/grub, systemd-boot) from the ESP. +/// Other content, e.g. Asahi's `m1n1/` and `vendorfw/`, may be firmware +/// or earlier boot stages we cannot recreate, so it is preserved. +// TODO: be more selective, e.g. keep other OSes' `EFI/` and +// non-bootc `loader/` entries, and drop old Type #1 kernels (see #2243). +#[context("Removing bootloader content from EFI system partition")] +fn clean_esp_bootloader_dirs(efidir: &Dir) -> Result<()> { + for name in ["EFI", "loader"] { + if efidir.try_exists(name)? { + remove_dir_no_xdev(efidir, name, false) + .with_context(|| format!("Removing directory: {name}"))?; + } + } + Ok(()) +} + #[context("Removing boot directory content")] fn clean_boot_directories(rootfs: &Dir, rootfs_path: &Utf8Path, is_ostree: bool) -> Result<()> { let bootdir = @@ -2393,13 +2413,12 @@ fn clean_boot_directories(rootfs: &Dir, rootfs_path: &Utf8Path, is_ostree: bool) // This should not remove /boot/efi note. remove_all_except_loader_dirs(&bootdir, is_ostree).context("Emptying /boot")?; - // TODO: we should also support not wiping the ESP. if ARCH_USES_EFI { if let Some(efidir) = bootdir .open_dir_optional(crate::bootloader::EFI_DIR) .context("Opening /boot/efi")? { - remove_all_in_dir_no_xdev(&efidir, false).context("Emptying EFI system partition")?; + clean_esp_bootloader_dirs(&efidir)?; } } @@ -3078,6 +3097,32 @@ mod tests { Ok(()) } + #[test] + fn test_clean_esp_bootloader_dirs() -> Result<()> { + let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + + td.create_dir_all("EFI/BOOT")?; + td.write("EFI/BOOT/BOOTAA64.EFI", b"shim")?; + td.create_dir_all("loader/entries")?; + td.write("loader/entries/foo.conf", b"entry")?; + // Asahi content which must survive + td.create_dir_all("m1n1")?; + td.write("m1n1/boot.bin", b"m1n1")?; + td.write("ubootefi.var", b"efivars")?; + + clean_esp_bootloader_dirs(&td)?; + assert!(!td.exists("EFI")); + assert!(!td.exists("loader")); + assert_eq!(td.read("m1n1/boot.bin")?, b"m1n1"); + assert_eq!(td.read("ubootefi.var")?, b"efivars"); + + // Idempotent + clean_esp_bootloader_dirs(&td)?; + assert_eq!(td.entries()?.count(), 2); + + Ok(()) + } + #[test] fn test_read_boot_fstab_entry() -> Result<()> { let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?; diff --git a/docs/src/bootc-install.md b/docs/src/bootc-install.md index 6849c8adcf..f3e555647b 100644 --- a/docs/src/bootc-install.md +++ b/docs/src/bootc-install.md @@ -205,8 +205,9 @@ Set the environment variable `BOOTC_DIRECT_IO=on` to create the loopback device This is a variant of `install to-filesystem`, which maximizes convenience for using an existing Linux system, converting it into the target container image. Note that -the `/boot` (and `/boot/efi`) partitions *will be reinitialized* - so this is a -somewhat destructive operation for the existing Linux installation. +the `/boot` partition and the `EFI/` and `loader/` directories of the ESP +*will be reinitialized* - so this is a somewhat destructive operation for the +existing Linux installation. Other ESP content (e.g. Asahi's `m1n1/`) is preserved. Also, because the filesystem is reused, it's required that the target system kernel support the root storage setup already initialized. diff --git a/tmt/tests/booted/test-multi-device-esp.nu b/tmt/tests/booted/test-multi-device-esp.nu index 526a54afc1..303743805a 100644 --- a/tmt/tests/booted/test-multi-device-esp.nu +++ b/tmt/tests/booted/test-multi-device-esp.nu @@ -16,7 +16,8 @@ # 3. Three devices, partial ESP: Three disks, ESP on disk1+disk3 only # # Reboot 2: -# 4. Single device (no LVM): ESP + root partition on a single disk +# 4. Single device (no LVM): ESP + root partition on a single disk; +# also checks that non-bootloader ESP content is preserved # 5. No ESP anywhere: Two disks with no ESP; install should fail gracefully # # This validates the fix for https://github.com/bootc-dev/bootc/issues/481 @@ -157,6 +158,17 @@ def validate_esp [esp_partition: string] { } } +# Mount an ESP partition, run a closure on the mountpoint, and unmount it +def with_esp [esp_partition: string, f: closure] { + let esp_mount = "/var/mnt/esp_seed" + mkdir $esp_mount + mount $esp_partition $esp_mount + let r = (do $f $esp_mount) + umount $esp_mount + rmdir $esp_mount + $r +} + # Run bootc install to-existing-root from within the container image under test def run_install [mountpoint: string] { (podman run @@ -356,8 +368,15 @@ def test_single_device_no_lvm [] { mkdir $mountpoint mount $"($loop1)p2" $mountpoint - # Create boot directory - mkdir $"($mountpoint)/boot" + # Create /boot/efi so the ESP gets mounted and cleaned + mkdir $"($mountpoint)/boot/efi" + + # Seed non-bootloader content (as on Asahi) and a stale bootloader dir + with_esp $"($loop1)p1" {|esp| + mkdir $"($esp)/m1n1" $"($esp)/EFI/stale" + "m1n1" | save $"($esp)/m1n1/boot.bin" + "stale" | save $"($esp)/EFI/stale/x.efi" + } # Show block device hierarchy lsblk --pairs --paths --inverse --output NAME,TYPE $"($loop1)p2" @@ -366,6 +385,12 @@ def test_single_device_no_lvm [] { # Validate ESP was installed correctly validate_esp $"($loop1)p1" + let r = (with_esp $"($loop1)p1" {|esp| { + m1n1: ($"($esp)/m1n1/boot.bin" | path exists) + stale: ($"($esp)/EFI/stale" | path exists) + }}) + assert $r.m1n1 "m1n1/boot.bin was removed from the ESP" + assert (not $r.stale) "EFI/stale was not removed from the ESP" } catch {|e| cleanup_simple $loop1 $mountpoint rm -f $disk1