efi: support updating multiple EFIs in mirrored setups (RAID1)

HuijingHei · HuijingHei · commit d4bdbe1b6fa7 · 2025-02-19T16:47:57.000+08:00
The prep PR is #824 Fixes #132
diff --git a/src/bios.rs b/src/bios.rs
@@ -97,16 +97,16 @@ impl Bios {
     }
 
     // check bios_boot partition on gpt type disk
+    #[cfg(target_arch = "x86_64")]
     fn get_bios_boot_partition(&self) -> Option<String> {
         match blockdev::get_single_device("/") {
             Ok(device) => {
                 let bios_boot_part =
                     blockdev::get_bios_boot_partition(&device).expect("get bios_boot part");
                 return bios_boot_part;
             }
-            Err(e) => log::warn!("Get error: {}", e),
+            Err(e) => log::warn!("Get single device: {}", e),
         }
-        log::debug!("Not found any bios_boot partition");
         None
     }
 }
@@ -149,7 +149,7 @@ impl Component for Bios {
 
     fn query_adopt(&self) -> Result<Option<Adoptable>> {
         #[cfg(target_arch = "x86_64")]
-        if crate::efi::is_efi_booted()? && self.get_bios_boot_partition().is_none() {
+        if self.get_bios_boot_partition().is_none() {
             log::debug!("Skip BIOS adopt");
             return Ok(None);
         }
diff --git a/src/blockdev.rs b/src/blockdev.rs
@@ -37,7 +37,6 @@ pub fn get_single_device<P: AsRef<Path>>(target_root: P) -> Result<String> {
 
 /// Find esp partition on the same device
 /// using sfdisk to get partitiontable
-#[allow(dead_code)]
 pub fn get_esp_partition(device: &str) -> Result<Option<String>> {
     const ESP_TYPE_GUID: &str = "C12A7328-F81F-11D2-BA4B-00A0C93EC93B";
     let device_info: PartitionTable = bootc_blockdev::partitions_of(Utf8Path::new(device))?;
@@ -52,7 +51,6 @@ pub fn get_esp_partition(device: &str) -> Result<Option<String>> {
 }
 
 /// Find all ESP partitions on the devices with mountpoint boot
-#[allow(dead_code)]
 pub fn find_colocated_esps<P: AsRef<Path>>(target_root: P) -> Result<Vec<String>> {
     // first, get the parent device
     let devices = get_devices(&target_root).with_context(|| "while looking for colocated ESPs")?;
diff --git a/src/efi.rs b/src/efi.rs
@@ -19,6 +19,7 @@ use rustix::fd::BorrowedFd;
 use walkdir::WalkDir;
 use widestring::U16CString;
 
+use crate::blockdev;
 use crate::filetree;
 use crate::model::*;
 use crate::ostreeutil;
@@ -57,28 +58,7 @@ pub(crate) struct Efi {
 }
 
 impl Efi {
-    fn esp_path(&self) -> Result<PathBuf> {
-        self.ensure_mounted_esp(Path::new("/"))
-            .map(|v| v.join("EFI"))
-    }
-
-    fn open_esp_optional(&self) -> Result<Option<openat::Dir>> {
-        if !is_efi_booted()? && self.get_esp_device().is_none() {
-            log::debug!("Skip EFI");
-            return Ok(None);
-        }
-        let sysroot = openat::Dir::open("/")?;
-        let esp = sysroot.sub_dir_optional(&self.esp_path()?)?;
-        Ok(esp)
-    }
-
-    fn open_esp(&self) -> Result<openat::Dir> {
-        self.ensure_mounted_esp(Path::new("/"))?;
-        let sysroot = openat::Dir::open("/")?;
-        let esp = sysroot.sub_dir(&self.esp_path()?)?;
-        Ok(esp)
-    }
-
+    // Get esp device via legacy
     fn get_esp_device(&self) -> Option<PathBuf> {
         let esp_devices = [COREOS_ESP_PART_LABEL, ANACONDA_ESP_PART_LABEL]
             .into_iter()
@@ -93,11 +73,26 @@ impl Efi {
         return esp_device;
     }
 
-    pub(crate) fn ensure_mounted_esp(&self, root: &Path) -> Result<PathBuf> {
-        let mut mountpoint = self.mountpoint.borrow_mut();
+    // Get esp device list on all devices
+    fn get_esp_devices(&self) -> Option<Vec<String>> {
+        let mut esp_devices = vec![];
+        if let Some(esp_device) = self.get_esp_device() {
+            esp_devices.push(esp_device.to_string_lossy().into_owned());
+        } else {
+            esp_devices = blockdev::find_colocated_esps("/").expect("get esp devices");
+        };
+        if !esp_devices.is_empty() {
+            return Some(esp_devices);
+        }
+        return None;
+    }
+
+    fn check_mounted_esp<P: AsRef<Path>>(&self, root: P) -> Result<Option<PathBuf>> {
+        let mountpoint = self.mountpoint.borrow_mut();
         if let Some(mountpoint) = mountpoint.as_deref() {
-            return Ok(mountpoint.to_owned());
+            return Ok(Some(mountpoint.to_owned()));
         }
+        let root = root.as_ref();
         for &mnt in ESP_MOUNTS {
             let mnt = root.join(mnt);
             if !mnt.exists() {
@@ -109,13 +104,23 @@ impl Efi {
                 continue;
             }
             util::ensure_writable_mount(&mnt)?;
-            log::debug!("Reusing existing {mnt:?}");
-            return Ok(mnt);
+            log::debug!("Reusing existing mount point {mnt:?}");
+            return Ok(Some(mnt));
+        }
+        Ok(None)
+    }
+
+    pub(crate) fn ensure_mounted_esp<P: AsRef<Path>>(
+        &self,
+        root: P,
+        esp_device: &str,
+    ) -> Result<PathBuf> {
+        let mut mountpoint = self.mountpoint.borrow_mut();
+        if let Some(mountpoint) = mountpoint.as_deref() {
+            return Ok(mountpoint.to_owned());
         }
 
-        let esp_device = self
-            .get_esp_device()
-            .ok_or_else(|| anyhow::anyhow!("Failed to find ESP device"))?;
+        let root = root.as_ref();
         for &mnt in ESP_MOUNTS.iter() {
             let mnt = root.join(mnt);
             if !mnt.exists() {
@@ -137,6 +142,7 @@ impl Efi {
         if let Some(mount) = self.mountpoint.borrow_mut().take() {
             Command::new("umount")
                 .arg(&mount)
+                .arg("-l")
                 .run()
                 .with_context(|| format!("Failed to unmount {mount:?}"))?;
             log::trace!("Unmounted");
@@ -243,8 +249,7 @@ impl Component for Efi {
     }
 
     fn query_adopt(&self) -> Result<Option<Adoptable>> {
-        let esp = self.open_esp_optional()?;
-        if esp.is_none() {
+        if self.get_esp_devices().is_none() {
             log::trace!("No ESP detected");
             return Ok(None);
         };
@@ -267,16 +272,32 @@ impl Component for Efi {
             anyhow::bail!("Failed to find adoptable system")
         };
 
-        let esp = self.open_esp()?;
-        validate_esp(&esp)?;
         let updated = sysroot
             .sub_dir(&component_updatedirname(self))
             .context("opening update dir")?;
         let updatef = filetree::FileTree::new_from_dir(&updated).context("reading update dir")?;
-        // For adoption, we should only touch files that we know about.
-        let diff = updatef.relative_diff_to(&esp)?;
-        log::trace!("applying adoption diff: {}", &diff);
-        filetree::apply_diff(&updated, &esp, &diff, None).context("applying filesystem changes")?;
+        let esp_devices = self
+            .get_esp_devices()
+            .expect("get esp devices before adopt");
+        let sysroot = sysroot.recover_path()?;
+
+        for esp_dev in esp_devices {
+            let dest_path = if let Some(dest_path) = self.check_mounted_esp(&sysroot)? {
+                dest_path.join("EFI")
+            } else {
+                self.ensure_mounted_esp(&sysroot, &esp_dev)?.join("EFI")
+            };
+
+            let esp = openat::Dir::open(&dest_path).context("opening EFI dir")?;
+            validate_esp(&esp)?;
+
+            // For adoption, we should only touch files that we know about.
+            let diff = updatef.relative_diff_to(&esp)?;
+            log::trace!("applying adoption diff: {}", &diff);
+            filetree::apply_diff(&updated, &esp, &diff, None)
+                .context("applying filesystem changes")?;
+            self.unmount().context("unmount after adopt")?;
+        }
         Ok(InstalledContent {
             meta: updatemeta.clone(),
             filetree: Some(updatef),
@@ -298,9 +319,17 @@ impl Component for Efi {
         log::debug!("Found metadata {}", meta.version);
         let srcdir_name = component_updatedirname(self);
         let ft = crate::filetree::FileTree::new_from_dir(&src_root.sub_dir(&srcdir_name)?)?;
-        let destdir = &self.ensure_mounted_esp(Path::new(dest_root))?;
+        let destdir = if let Some(destdir) = self.check_mounted_esp(dest_root)? {
+            destdir
+        } else {
+            let esp_device = self
+                .get_esp_device()
+                .ok_or_else(|| anyhow::anyhow!("Failed to find ESP device"))?;
+            let esp_device = esp_device.to_str().unwrap();
+            self.ensure_mounted_esp(dest_root, esp_device)?
+        };
 
-        let destd = &openat::Dir::open(destdir)
+        let destd = &openat::Dir::open(&destdir)
             .with_context(|| format!("opening dest dir {}", destdir.display()))?;
         validate_esp(destd)?;
 
@@ -339,12 +368,25 @@ impl Component for Efi {
             .context("opening update dir")?;
         let updatef = filetree::FileTree::new_from_dir(&updated).context("reading update dir")?;
         let diff = currentf.diff(&updatef)?;
-        self.ensure_mounted_esp(Path::new("/"))?;
-        let destdir = self.open_esp().context("opening EFI dir")?;
-        validate_esp(&destdir)?;
-        log::trace!("applying diff: {}", &diff);
-        filetree::apply_diff(&updated, &destdir, &diff, None)
-            .context("applying filesystem changes")?;
+        let esp_devices = self
+            .get_esp_devices()
+            .context("get esp devices when running update")?;
+        let sysroot = sysroot.recover_path()?;
+
+        for esp in esp_devices {
+            let dest_path = if let Some(dest_path) = self.check_mounted_esp(&sysroot)? {
+                dest_path.join("EFI")
+            } else {
+                self.ensure_mounted_esp(&sysroot, &esp)?.join("EFI")
+            };
+
+            let destdir = openat::Dir::open(&dest_path).context("opening EFI dir")?;
+            validate_esp(&destdir)?;
+            log::trace!("applying diff: {}", &diff);
+            filetree::apply_diff(&updated, &destdir, &diff, None)
+                .context("applying filesystem changes")?;
+            self.unmount().context("unmount after update")?;
+        }
         let adopted_from = None;
         Ok(InstalledContent {
             meta: updatemeta,
@@ -392,24 +434,37 @@ impl Component for Efi {
     }
 
     fn validate(&self, current: &InstalledContent) -> Result<ValidationResult> {
-        if !is_efi_booted()? && self.get_esp_device().is_none() {
+        let esp_devices = self.get_esp_devices();
+        if !is_efi_booted()? && esp_devices.is_none() {
             return Ok(ValidationResult::Skip);
         }
         let currentf = current
             .filetree
             .as_ref()
             .ok_or_else(|| anyhow::anyhow!("No filetree for installed EFI found!"))?;
-        self.ensure_mounted_esp(Path::new("/"))?;
-        let efidir = self.open_esp()?;
-        let diff = currentf.relative_diff_to(&efidir)?;
+
         let mut errs = Vec::new();
-        for f in diff.changes.iter() {
-            errs.push(format!("Changed: {}", f));
-        }
-        for f in diff.removals.iter() {
-            errs.push(format!("Removed: {}", f));
+        let esps = esp_devices.ok_or_else(|| anyhow::anyhow!("No esp device found!"))?;
+        let dest_root = Path::new("/");
+        for esp_dev in esps.iter() {
+            let dest_path = if let Some(dest_path) = self.check_mounted_esp(dest_root)? {
+                dest_path.join("EFI")
+            } else {
+                self.ensure_mounted_esp(dest_root, &esp_dev)?.join("EFI")
+            };
+
+            let efidir = openat::Dir::open(dest_path.as_path())?;
+            let diff = currentf.relative_diff_to(&efidir)?;
+
+            for f in diff.changes.iter() {
+                errs.push(format!("Changed: {}", f));
+            }
+            for f in diff.removals.iter() {
+                errs.push(format!("Removed: {}", f));
+            }
+            assert_eq!(diff.additions.len(), 0);
+            self.unmount().context("unmount after validate")?;
         }
-        assert_eq!(diff.additions.len(), 0);
         if !errs.is_empty() {
             Ok(ValidationResult::Errors(errs))
         } else {
diff --git a/tests/kola/raid1/config.bu b/tests/kola/raid1/config.bu
@@ -0,0 +1,7 @@
+variant: fcos
+version: 1.5.0
+boot_device:
+  mirror:
+    devices:
+      - /dev/vda
+      - /dev/vdb
diff --git a/tests/kola/raid1/data/libtest.sh b/tests/kola/raid1/data/libtest.sh
@@ -0,0 +1 @@
+../../data/libtest.sh
diff --git a/tests/kola/raid1/test.sh b/tests/kola/raid1/test.sh
@@ -0,0 +1,37 @@
+#!/bin/bash
+## kola:
+##   # additionalDisks is only supported on qemu.
+##   platforms: qemu
+##   # Root reprovisioning requires at least 4GiB of memory.
+##   minMemory: 4096
+##   # Linear RAID is setup on these disks.
+##   additionalDisks: ["10G"]
+##   # This test includes a lot of disk I/O and needs a higher
+##   # timeout value than the default.
+##   timeoutMin: 15
+##   description: Verify updating multiple EFIs with RAID 1 works.
+
+set -xeuo pipefail
+
+# shellcheck disable=SC1091
+. "$KOLA_EXT_DATA/libtest.sh"
+
+srcdev=$(findmnt -nvr /sysroot -o SOURCE)
+[[ ${srcdev} == "/dev/md126" ]]
+
+blktype=$(lsblk -o TYPE "${srcdev}" --noheadings)
+[[ ${blktype} == "raid1" ]]
+
+fstype=$(findmnt -nvr /sysroot -o FSTYPE)
+[[ ${fstype} == "xfs" ]]
+ok "source is XFS on RAID1 device"
+
+
+mount -o remount,rw /boot
+
+rm -f -v /boot/bootupd-state.json
+
+bootupctl adopt-and-update | grep "Adopted and updated: EFI"
+
+bootupctl status | grep "Component EFI"
+ok "bootupctl adopt-and-update supports multiple EFIs on RAID1"

Original file line number	Diff line number	Diff line change
`@@ -97,16 +97,16 @@ impl Bios {`
`97`	`97`	`}`
`98`	`98`
`99`	`99`	`// check bios_boot partition on gpt type disk`
	`100`	`+ #[cfg(target_arch = "x86_64")]`
`100`	`101`	`fn get_bios_boot_partition(&self) -> Option<String> {`
`101`	`102`	`match blockdev::get_single_device("/") {`
`102`	`103`	`Ok(device) => {`
`103`	`104`	`let bios_boot_part =`
`104`	`105`	`blockdev::get_bios_boot_partition(&device).expect("get bios_boot part");`
`105`	`106`	`return bios_boot_part;`
`106`	`107`	`}`
`107`		`- Err(e) => log::warn!("Get error: {}", e),`
	`108`	`+ Err(e) => log::warn!("Get single device: {}", e),`
`108`	`109`	`}`
`109`		`- log::debug!("Not found any bios_boot partition");`
`110`	`110`	`None`
`111`	`111`	`}`
`112`	`112`	`}`
`@@ -149,7 +149,7 @@ impl Component for Bios {`
`149`	`149`
`150`	`150`	`fn query_adopt(&self) -> Result<Option<Adoptable>> {`
`151`	`151`	`#[cfg(target_arch = "x86_64")]`
`152`		`- if crate::efi::is_efi_booted()? && self.get_bios_boot_partition().is_none() {`
	`152`	`+ if self.get_bios_boot_partition().is_none() {`
`153`	`153`	`log::debug!("Skip BIOS adopt");`
`154`	`154`	`return Ok(None);`
`155`	`155`	`}`