From 0f9d1aca3533bd6f73e2714b746037eec6552adf Mon Sep 17 00:00:00 2001 From: Huijing Hei Date: Sun, 1 Jun 2025 10:39:08 +0800 Subject: [PATCH 1/5] efi: clarify function purpose by renaming `validate_esp()` to `validate_esp_fstype()` --- src/efi.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/efi.rs b/src/efi.rs index 10e58806..1d08d92b 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -271,7 +271,7 @@ impl Component for Efi { let destdir = &openat::Dir::open(&destpath.join("EFI")) .with_context(|| format!("opening EFI dir {}", destpath.display()))?; - validate_esp(&destdir)?; + validate_esp_fstype(&destdir)?; let updated = rootcxt .sysroot .sub_dir(&component_updatedirname(self)) @@ -312,7 +312,7 @@ impl Component for Efi { let destd = &openat::Dir::open(destpath) .with_context(|| format!("opening dest dir {}", destpath.display()))?; - validate_esp(destd)?; + validate_esp_fstype(destd)?; // TODO - add some sort of API that allows directly setting the working // directory to a file descriptor. @@ -368,7 +368,7 @@ impl Component for Efi { let destdir = &openat::Dir::open(&destpath.join("EFI")) .with_context(|| format!("opening EFI dir {}", destpath.display()))?; - validate_esp(&destdir)?; + validate_esp_fstype(&destdir)?; log::trace!("applying diff: {}", &diff); filetree::apply_diff(&updated, &destdir, &diff, None) .context("applying filesystem changes")?; @@ -492,7 +492,7 @@ impl Drop for Efi { } } -fn validate_esp(dir: &openat::Dir) -> Result<()> { +fn validate_esp_fstype(dir: &openat::Dir) -> Result<()> { let dir = unsafe { BorrowedFd::borrow_raw(dir.as_raw_fd()) }; let stat = rustix::fs::fstatfs(&dir)?; if stat.f_type != libc::MSDOS_SUPER_MAGIC { From 923c594cfd30d3cec87e347a5bd79d19d99faafd Mon Sep 17 00:00:00 2001 From: Huijing Hei Date: Fri, 13 Jun 2025 09:30:03 +0800 Subject: [PATCH 2/5] efi: extend support for `update()`, `adopt_update()`, and `validate()` across all ESPs --- src/efi.rs | 109 ++++++++++++++++++++++++----------------------------- 1 file changed, 50 insertions(+), 59 deletions(-) diff --git a/src/efi.rs b/src/efi.rs index 1d08d92b..578d9b17 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -10,6 +10,7 @@ use std::path::{Path, PathBuf}; use std::process::Command; use anyhow::{bail, Context, Result}; +use bootc_utils::CommandRunExt; use cap_std::fs::Dir; use cap_std_ext::cap_std; use fn_error_context::context; @@ -22,7 +23,7 @@ use widestring::U16CString; use crate::bootupd::RootContext; use crate::model::*; use crate::ostreeutil; -use crate::util::{self, CommandRunExt}; +use crate::util; use crate::{blockdev, filetree}; use crate::{component::*, packagesystem}; @@ -256,32 +257,30 @@ impl Component for Efi { return Ok(None); }; - let esp_devices = esp_devices.unwrap_or_default(); - let mut devices = esp_devices.iter(); - let Some(esp) = devices.next() else { - anyhow::bail!("Failed to find esp device"); - }; - - if let Some(next_esp) = devices.next() { - anyhow::bail!( - "Found multiple esp devices {esp} and {next_esp}; not currently supported" - ); - } - let destpath = &self.ensure_mounted_esp(rootcxt.path.as_ref(), Path::new(&esp))?; - - let destdir = &openat::Dir::open(&destpath.join("EFI")) - .with_context(|| format!("opening EFI dir {}", destpath.display()))?; - validate_esp_fstype(&destdir)?; let updated = rootcxt .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(&destdir)?; - log::trace!("applying adoption diff: {}", &diff); - filetree::apply_diff(&updated, &destdir, &diff, None) - .context("applying filesystem changes")?; + + let esp_devices = esp_devices.unwrap_or_default(); + for esp in esp_devices { + let destpath = &self.ensure_mounted_esp(rootcxt.path.as_ref(), Path::new(&esp))?; + + let efidir = openat::Dir::open(&destpath.join("EFI")).context("opening EFI dir")?; + validate_esp_fstype(&efidir)?; + + // For adoption, we should only touch files that we know about. + let diff = updatef.relative_diff_to(&efidir)?; + log::trace!("applying adoption diff: {}", &diff); + filetree::apply_diff(&updated, &efidir, &diff, None) + .context("applying filesystem changes")?; + + // Do the sync before unmount + efidir.syncfs()?; + drop(efidir); + self.unmount().context("unmount after adopt")?; + } Ok(Some(InstalledContent { meta: updatemeta.clone(), filetree: Some(updatef), @@ -354,24 +353,21 @@ impl Component for Efi { let Some(esp_devices) = blockdev::find_colocated_esps(&rootcxt.devices)? else { anyhow::bail!("Failed to find all esp devices"); }; - let mut devices = esp_devices.iter(); - let Some(esp) = devices.next() else { - anyhow::bail!("Failed to find esp device"); - }; - if let Some(next_esp) = devices.next() { - anyhow::bail!( - "Found multiple esp devices {esp} and {next_esp}; not currently supported" - ); + for esp in esp_devices { + let destpath = &self.ensure_mounted_esp(rootcxt.path.as_ref(), Path::new(&esp))?; + let destdir = openat::Dir::open(&destpath.join("EFI")).context("opening EFI dir")?; + validate_esp_fstype(&destdir)?; + log::trace!("applying diff: {}", &diff); + filetree::apply_diff(&updated, &destdir, &diff, None) + .context("applying filesystem changes")?; + + // Do the sync before unmount + destdir.syncfs()?; + drop(destdir); + self.unmount().context("unmount after update")?; } - let destpath = &self.ensure_mounted_esp(rootcxt.path.as_ref(), Path::new(&esp))?; - - let destdir = &openat::Dir::open(&destpath.join("EFI")) - .with_context(|| format!("opening EFI dir {}", destpath.display()))?; - validate_esp_fstype(&destdir)?; - log::trace!("applying diff: {}", &diff); - filetree::apply_diff(&updated, &destdir, &diff, None) - .context("applying filesystem changes")?; + let adopted_from = None; Ok(InstalledContent { meta: updatemeta, @@ -430,31 +426,26 @@ impl Component for Efi { .as_ref() .ok_or_else(|| anyhow::anyhow!("No filetree for installed EFI found!"))?; + let mut errs = Vec::new(); let esp_devices = esp_devices.unwrap_or_default(); - let mut devices = esp_devices.iter(); - let Some(esp) = devices.next() else { - anyhow::bail!("Failed to find esp device"); - }; + for esp in esp_devices.iter() { + let destpath = &self.ensure_mounted_esp(Path::new("/"), Path::new(&esp))?; - if let Some(next_esp) = devices.next() { - anyhow::bail!( - "Found multiple esp devices {esp} and {next_esp}; not currently supported" - ); - } - let destpath = &self.ensure_mounted_esp(Path::new("/"), Path::new(&esp))?; + let efidir = openat::Dir::open(&destpath.join("EFI")) + .with_context(|| format!("opening EFI dir {}", destpath.display()))?; + let diff = currentf.relative_diff_to(&efidir)?; - let efidir = &openat::Dir::open(&destpath.join("EFI")) - .with_context(|| format!("opening EFI dir {}", destpath.display()))?; - - 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)); + 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); + drop(efidir); + self.unmount().context("unmount after validate")?; } - assert_eq!(diff.additions.len(), 0); + if !errs.is_empty() { Ok(ValidationResult::Errors(errs)) } else { From 4c6802707a2b37a86c2b3768ae77f51725bce4a7 Mon Sep 17 00:00:00 2001 From: Huijing Hei Date: Fri, 13 Jun 2025 09:42:29 +0800 Subject: [PATCH 3/5] BIOS: extend support for `update & adopt_update` on all devices For dual boot, need to extend support for both. Also add kola test to verify that supports updating multiple EFIs. --- src/bios.rs | 30 ++++++----------------- tests/kola/raid1/config.bu | 7 ++++++ tests/kola/raid1/data/libtest.sh | 1 + tests/kola/raid1/test.sh | 42 ++++++++++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 23 deletions(-) create mode 100644 tests/kola/raid1/config.bu create mode 120000 tests/kola/raid1/data/libtest.sh create mode 100755 tests/kola/raid1/test.sh diff --git a/src/bios.rs b/src/bios.rs index 59b57626..83bf828e 100644 --- a/src/bios.rs +++ b/src/bios.rs @@ -167,18 +167,11 @@ impl Component for Bios { return Ok(None); }; - let mut parent_devices = rootcxt.devices.iter(); - let Some(parent) = parent_devices.next() else { - anyhow::bail!("Failed to find parent device"); - }; - - if let Some(next) = parent_devices.next() { - anyhow::bail!( - "Found multiple parent devices {parent} and {next}; not currently supported" - ); + for parent in rootcxt.devices.iter() { + self.run_grub_install(rootcxt.path.as_str(), &parent)?; + log::debug!("Installed grub modules on {parent}"); } - self.run_grub_install(rootcxt.path.as_str(), &parent)?; - log::debug!("Installed grub modules on {parent}"); + Ok(Some(InstalledContent { meta: update.clone(), filetree: None, @@ -195,20 +188,11 @@ impl Component for Bios { .query_update(&rootcxt.sysroot)? .expect("update available"); - let mut parent_devices = rootcxt.devices.iter(); - let Some(parent) = parent_devices.next() else { - anyhow::bail!("Failed to find parent device"); - }; - - if let Some(next) = parent_devices.next() { - anyhow::bail!( - "Found multiple parent devices {parent} and {next}; not currently supported" - ); + for parent in rootcxt.devices.iter() { + self.run_grub_install(rootcxt.path.as_str(), &parent)?; + log::debug!("Installed grub modules on {parent}"); } - self.run_grub_install(rootcxt.path.as_str(), &parent)?; - log::debug!("Install grub modules on {parent}"); - let adopted_from = None; Ok(InstalledContent { meta: updatemeta, diff --git a/tests/kola/raid1/config.bu b/tests/kola/raid1/config.bu new file mode 100644 index 00000000..8a9a598f --- /dev/null +++ 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 new file mode 120000 index 00000000..59532579 --- /dev/null +++ b/tests/kola/raid1/data/libtest.sh @@ -0,0 +1 @@ +../../data/libtest.sh \ No newline at end of file diff --git a/tests/kola/raid1/test.sh b/tests/kola/raid1/test.sh new file mode 100755 index 00000000..2a690ab3 --- /dev/null +++ b/tests/kola/raid1/test.sh @@ -0,0 +1,42 @@ +#!/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 using RAID 1 works. + +set -xeuo pipefail + +# shellcheck disable=SC1091 +. "$KOLA_EXT_DATA/libtest.sh" + +tmpdir=$(mktemp -d) +cd ${tmpdir} + +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 | tee out.txt +assert_file_has_content out.txt "Adopted and updated: BIOS: .*" +assert_file_has_content out.txt "Adopted and updated: EFI: .*" + +bootupctl status | tee out.txt +assert_file_has_content_literal out.txt 'Component BIOS' +assert_file_has_content_literal out.txt 'Component EFI' +ok "bootupctl adopt-and-update supports multiple EFIs on RAID1" From 5222e9ae468e38fb7ccf5694580412422317d09f Mon Sep 17 00:00:00 2001 From: Huijing Hei Date: Fri, 13 Jun 2025 09:42:57 +0800 Subject: [PATCH 4/5] blockdev: Use `/sysroot` as fallback in `get_devices()` if `/boot` is not mounted Inspired by https://github.com/coreos/bootupd/pull/855#discussion_r2045345928 If `/boot` is not mounted, the parent devices of `/sysroot` will be used as a fallback. --- src/blockdev.rs | 36 +++++++++++++++++++++++++----------- src/util.rs | 14 -------------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/blockdev.rs b/src/blockdev.rs index 7154eee7..ff46bb2c 100644 --- a/src/blockdev.rs +++ b/src/blockdev.rs @@ -1,24 +1,38 @@ use camino::Utf8Path; use std::path::Path; -use anyhow::{bail, Context, Result}; +use anyhow::{Context, Result}; use bootc_blockdev::PartitionTable; use fn_error_context::context; -#[context("get parent devices from mount point boot")] +#[context("get parent devices from mount point boot or sysroot")] pub fn get_devices>(target_root: P) -> Result> { let target_root = target_root.as_ref(); - let bootdir = target_root.join("boot"); - if !bootdir.exists() { - bail!("{} does not exist", bootdir.display()); + let mut source = None; + + for path in ["boot", "sysroot"] { + let target_path = target_root.join(path); + if !target_path.exists() { + continue; + } + + let target_dir = openat::Dir::open(&target_path) + .with_context(|| format!("Opening {}", target_path.display()))?; + if let Ok(fsinfo) = crate::filesystem::inspect_filesystem(&target_dir, ".") { + source = Some(fsinfo.source); + break; + } } - let bootdir = openat::Dir::open(&bootdir)?; - // Run findmnt to get the source path of mount point boot - let fsinfo = crate::filesystem::inspect_filesystem(&bootdir, ".")?; + + let source = match source { + Some(s) => s, + None => anyhow::bail!("Failed to inspect filesystem from boot or sysroot"), + }; + // Find the parent devices of the source path - let parent_devices = bootc_blockdev::find_parent_devices(&fsinfo.source) - .with_context(|| format!("while looking for backing devices of {}", fsinfo.source))?; - log::debug!("Find parent devices: {parent_devices:?}"); + let parent_devices = bootc_blockdev::find_parent_devices(&source) + .with_context(|| format!("While looking for backing devices of {}", source))?; + log::debug!("Found parent devices: {parent_devices:?}"); Ok(parent_devices) } diff --git a/src/util.rs b/src/util.rs index 18efc4e7..3d32bc5c 100644 --- a/src/util.rs +++ b/src/util.rs @@ -5,20 +5,6 @@ use std::process::Command; use anyhow::{bail, Context, Result}; use openat_ext::OpenatDirExt; -pub(crate) trait CommandRunExt { - fn run(&mut self) -> Result<()>; -} - -impl CommandRunExt for Command { - fn run(&mut self) -> Result<()> { - let r = self.status()?; - if !r.success() { - bail!("Child [{:?}] exited: {}", self, r); - } - Ok(()) - } -} - /// Parse an environment variable as UTF-8 #[allow(dead_code)] pub(crate) fn getenv_utf8(n: &str) -> Result> { From c9cd73eb4727c721f69b2ae5d3d51f3ff3d63d34 Mon Sep 17 00:00:00 2001 From: Huijing Hei Date: Fri, 13 Jun 2025 18:11:24 +0800 Subject: [PATCH 5/5] Remove unused functions --- src/bios.rs | 15 --------------- src/blockdev.rs | 14 -------------- 2 files changed, 29 deletions(-) diff --git a/src/bios.rs b/src/bios.rs index 83bf828e..bdde0269 100644 --- a/src/bios.rs +++ b/src/bios.rs @@ -95,21 +95,6 @@ impl Bios { } Ok(()) } - - // check bios_boot partition on gpt type disk - #[allow(dead_code)] - fn get_bios_boot_partition(&self) -> Option { - 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), - } - log::debug!("Not found any bios_boot partition"); - None - } } impl Component for Bios { diff --git a/src/blockdev.rs b/src/blockdev.rs index ff46bb2c..3c2402f1 100644 --- a/src/blockdev.rs +++ b/src/blockdev.rs @@ -36,20 +36,6 @@ pub fn get_devices>(target_root: P) -> Result> { Ok(parent_devices) } -// Get single device for the target root -#[allow(dead_code)] -pub fn get_single_device>(target_root: P) -> Result { - let mut devices = get_devices(&target_root)?.into_iter(); - let Some(parent) = devices.next() else { - anyhow::bail!("Failed to find parent device"); - }; - - if let Some(next) = devices.next() { - anyhow::bail!("Found multiple parent devices {parent} and {next}; not currently supported"); - } - Ok(parent) -} - /// Find esp partition on the same device /// using sfdisk to get partitiontable pub fn get_esp_partition(device: &str) -> Result> {