diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index a10d41bf12..7723ed4da8 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -118,7 +118,7 @@ use crate::{ }; use crate::{parsers::grub_menuconfig::MenuEntry, store::BootedComposefs}; -use crate::install::{RootSetup, State}; +use crate::install::{BOOT, RootSetup, State}; /// Contains the EFP's filesystem UUID. Used by grub pub(crate) const EFI_UUID_FILE: &str = "efiuuid.cfg"; @@ -304,7 +304,7 @@ fn read_dumpfile_from_fs( let root = fs.as_dir(); let dumpfile_os = std::ffi::OsStr::new(dumpfile_name); - if let Ok(boot_dir) = root.get_directory_ref("boot".as_ref()) { + if let Ok(boot_dir) = root.get_directory_ref(BOOT.as_ref()) { if let Ok(file) = boot_dir.get_file(dumpfile_os) { return read_regular_file(file, repo); } @@ -320,6 +320,39 @@ pub(crate) enum BootSetupType<'a> { Upgrade((&'a Storage, &'a BootedComposefs, &'a Host)), } +impl BootSetupType<'_> { + /// The directory holding GRUB's configuration and the BLS entries: `boot` + /// in the target root at install time. On a booted system, it's + /// `/sysroot/boot`, or `/boot` if that's a separate boot partition. + fn grub_boot_path(&self) -> Result { + match self { + Self::Setup((root_setup, ..)) => Ok(root_setup.physical_root_path.join(BOOT)), + Self::Upgrade((storage, ..)) => Ok(storage.require_grub_boot_path()?.to_owned()), + } + } +} + +/// The absolute path GRUB resolves the kernels and initramfses in BLS entries +/// against: the root of the filesystem holding `grub_boot_path`. +fn grub_bls_abs_entries_path(grub_boot_path: &Utf8Path) -> Result { + let (Some(parent), Some(name)) = (grub_boot_path.parent(), grub_boot_path.file_name()) else { + anyhow::bail!("Invalid boot path {grub_boot_path}"); + }; + let parent = Dir::open_ambient_dir(parent, ambient_authority()) + .with_context(|| format!("Opening {parent}"))?; + // If "boot" is a partition, we want the paths to be absolute to "/" + let is_mountpoint = parent + .is_mountpoint(name) + .with_context(|| format!("Checking if {grub_boot_path} is a mountpoint"))?; + // We can be fairly sure that the kernels we target support `statx` + let root = Utf8Path::new("/"); + Ok(if is_mountpoint == Some(true) { + root.to_owned() + } else { + root.join(BOOT) + }) +} + #[derive( ValueEnum, Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize, Default, JsonSchema, )] @@ -744,7 +777,7 @@ pub(crate) fn setup_composefs_bls_boot( ) -> Result { let id_hex = id.to_hex(); - let (root_path, esp_device, mut cmdline_refs, bootloader) = match setup_type { + let (esp_device, mut cmdline_refs, bootloader) = match setup_type { BootSetupType::Setup((root_setup, state, postfetch, allow_missing_fsverity)) => { // root_setup.kargs has [root=UUID=, "rw"] let mut cmdline_options = Cmdline::new(); @@ -781,7 +814,6 @@ pub(crate) fn setup_composefs_bls_boot( let esp_part = root_setup.device_info.find_first_colocated_esp()?; ( - root_setup.physical_root_path.clone(), esp_part.path(), cmdline_options, postfetch.detected_bootloader.clone(), @@ -818,12 +850,7 @@ pub(crate) fn setup_composefs_bls_boot( let root_dev = bootc_blockdev::list_dev_by_dir(&storage.physical_root)?; let esp_dev = root_dev.find_first_colocated_esp()?; - ( - Utf8PathBuf::from("/sysroot"), - esp_dev.path(), - cmdline, - bootloader, - ) + (esp_dev.path(), cmdline, bootloader) } }; @@ -839,24 +866,17 @@ pub(crate) fn setup_composefs_bls_boot( let (entry_paths, _tmpdir_guard) = match bootloader.kind()? { BootloaderKind::GRUBClassic => { - let root = Dir::open_ambient_dir(&root_path, ambient_authority()) - .context("Opening root path")?; + let boot_path = setup_type.grub_boot_path()?; // Grub wants the paths to be absolute against the mounted drive that the kernel + // initrd live in - // - // If "boot" is a partition, we want the paths to be absolute to "/" - let entries_path = match root.is_mountpoint("boot")? { - Some(true) => "/", - // We can be fairly sure that the kernels we target support `statx` - Some(false) | None => "/boot", - }; + let abs_entries_path = grub_bls_abs_entries_path(&boot_path)?; ( BLSEntryPath { - entries_path: root_path.join("boot"), - config_path: root_path.join("boot"), - abs_entries_path: entries_path.into(), + entries_path: boot_path.clone(), + config_path: boot_path, + abs_entries_path, }, None, ) @@ -1591,13 +1611,12 @@ fn resolve_boot_image_match( #[context("Writing Grub menuentry")] fn write_grub_uki_menuentry( - root_path: Utf8PathBuf, setup_type: &BootSetupType, boot_label: String, id: &Sha512HashValue, esp_device: &String, ) -> Result<()> { - let boot_dir = root_path.join("boot"); + let boot_dir = setup_type.grub_boot_path()?; create_dir_all(&boot_dir).context("Failed to create boot dir")?; let is_upgrade = matches!(setup_type, BootSetupType::Upgrade(..)); @@ -1756,8 +1775,7 @@ pub(crate) fn setup_composefs_uki_boot( boot_ids: &ExpectedBootImageIds, entries: Vec>, ) -> Result<(String, Sha512HashValue)> { - let (root_path, esp_device, bootloader, missing_fsverity_allowed, uki_addons) = match setup_type - { + let (esp_device, bootloader, missing_fsverity_allowed, uki_addons) = match setup_type { BootSetupType::Setup((root_setup, state, postfetch, allow_missing_fsverity)) => { state.require_no_kargs_for_uki()?; @@ -1765,7 +1783,6 @@ pub(crate) fn setup_composefs_uki_boot( let esp_part = root_setup.device_info.find_first_colocated_esp()?; ( - root_setup.physical_root_path.clone(), esp_part.path(), postfetch.detected_bootloader.clone(), allow_missing_fsverity, @@ -1774,7 +1791,6 @@ pub(crate) fn setup_composefs_uki_boot( } BootSetupType::Upgrade((storage, booted_cfs, host)) => { - let sysroot = Utf8PathBuf::from("/sysroot"); // Still needed for root_path let bootloader = host.require_composefs_booted()?.bootloader.clone(); // Locate ESP partition device by walking up to the root disk(s) @@ -1782,7 +1798,6 @@ pub(crate) fn setup_composefs_uki_boot( let esp_dev = root_dev.find_first_colocated_esp()?; ( - sysroot, esp_dev.path(), bootloader, booted_cfs.cmdline.allow_missing_fsverity, @@ -1866,7 +1881,7 @@ pub(crate) fn setup_composefs_uki_boot( match bootloader.kind()? { BootloaderKind::GRUBClassic => { - write_grub_uki_menuentry(root_path, &setup_type, boot_label, &deploy_id, &esp_device)? + write_grub_uki_menuentry(&setup_type, boot_label, &deploy_id, &esp_device)? } BootloaderKind::BLSCompatible => write_systemd_uki_config( @@ -2134,7 +2149,7 @@ pub(crate) async fn setup_composefs_boot( // which is otherwise mounted at the composefs root's own /boot) and // an empty `boot/efi` directory for its EFI component to discover // and mount the real ESP into, exactly as it would on ostree. - let bind_boot_path = root_setup.physical_root_path.join("boot"); + let bind_boot_path = root_setup.physical_root_path.join(BOOT); crate::bootloader::install_via_bootupd( &root_setup.device_info, &root_setup.physical_root_path, @@ -2275,6 +2290,22 @@ mod tests { use super::*; use composefs::erofs::format::FormatVersion; + #[test] + fn test_grub_bls_abs_entries_path() -> Result<()> { + let td = tempfile::tempdir()?; + let td = Utf8Path::from_path(td.path()).unwrap(); + let boot = td.join(BOOT); + std::fs::create_dir(&boot)?; + // boot on the root filesystem + assert_eq!(grub_bls_abs_entries_path(&boot)?, "/boot"); + // A separate filesystem + assert_eq!(grub_bls_abs_entries_path(Utf8Path::new("/proc"))?, "/"); + // Nothing to check + assert!(grub_bls_abs_entries_path(Utf8Path::new("/")).is_err()); + assert!(grub_bls_abs_entries_path(&td.join("nonexistent/boot")).is_err()); + Ok(()) + } + #[test] fn test_replace_composefs_karg() { let mut cmdline = diff --git a/crates/lib/src/bootc_composefs/gc.rs b/crates/lib/src/bootc_composefs/gc.rs index 5e0e771588..06fc1f090a 100644 --- a/crates/lib/src/bootc_composefs/gc.rs +++ b/crates/lib/src/bootc_composefs/gc.rs @@ -536,7 +536,7 @@ pub(crate) async fn composefs_gc( let boot_dump = composefs_ctl::dump_files( &booted_cfs.repo, &non_bootable_img.id.to_hex(), - &vec![std::path::PathBuf::from("/boot")], + &vec![std::path::Path::new("/").join(crate::install::BOOT)], false, ) .context("Getting dump for /boot")?; diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index dd96976ca7..08a208d281 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -46,7 +46,7 @@ const BOOTCTL_RANDOM_SEED_MIN_VERSION: u32 = 257; /// in place (bootupd will overwrite them during installation). // TODO: clean all ESPs on multi-device setups pub(crate) fn mount_esp_part(root: &Dir, root_path: &Utf8Path, is_ostree: bool) -> Result<()> { - let efi_path = Utf8Path::new("boot").join(crate::bootloader::EFI_DIR); + let efi_path = Utf8Path::new(crate::install::BOOT).join(crate::bootloader::EFI_DIR); let Some(esp_fd) = root .open_dir_optional(&efi_path) .context("Opening /boot/efi")? diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 3ee6660b00..b2654b867c 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -966,8 +966,8 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result } // And also label /boot AKA xbootldr, if it exists - if rootfs_dir.try_exists("boot")? { - crate::lsm::ensure_dir_labeled(rootfs_dir, "boot", None, 0o755.into(), sepolicy)?; + if rootfs_dir.try_exists(BOOT)? { + crate::lsm::ensure_dir_labeled(rootfs_dir, BOOT, None, 0o755.into(), sepolicy)?; } // Build the list of ostree repo config options: defaults + install config @@ -1264,7 +1264,7 @@ async fn install_container( if let Some(policy) = sepolicy { let deployment_root_meta = root.dir_metadata()?; let deployment_root_devino = (deployment_root_meta.dev(), deployment_root_meta.ino()); - for d in ["ostree", "boot"] { + for d in ["ostree", BOOT] { let mut pathbuf = Utf8PathBuf::from(d); crate::lsm::ensure_dir_labeled_recurse( &root_setup.physical_root, @@ -1972,7 +1972,7 @@ async fn install_with_sysroot( .clone() .unwrap_or(rootfs.physical_root_path.clone()); let chroot_target = root_path.join(deployment_path.as_str()); - let bind_boot_path = root_path.join("boot"); + let bind_boot_path = root_path.join(BOOT); crate::bootloader::install_via_bootupd( &rootfs.device_info, &root_path, @@ -2226,7 +2226,7 @@ async fn install_to_filesystem_impl( // Finalize mounted filesystems if !rootfs.skip_finalize { - let bootfs = rootfs.boot.as_ref().map(|_| ("boot", "boot")); + let bootfs = rootfs.boot.as_ref().map(|_| ("boot", BOOT)); for (fsname, fs) in std::iter::once(("root", ".")).chain(bootfs) { finalize_filesystem(fsname, &rootfs.physical_root, fs)?; } @@ -2785,7 +2785,7 @@ pub(crate) async fn install_to_filesystem( if spec.is_empty() { None } else { - Some(MountSpec::new(&spec, "/boot")) + Some(MountSpec::new(&spec, &format!("/{BOOT}"))) } } else { // Read /etc/fstab to get boot entry, but only use it if it's UUID-based @@ -2795,7 +2795,7 @@ pub(crate) async fn install_to_filesystem( .or_else(|| { boot_uuid .as_deref() - .map(|boot_uuid| MountSpec::new_uuid_src(boot_uuid, "/boot")) + .map(|boot_uuid| MountSpec::new_uuid_src(boot_uuid, &format!("/{BOOT}"))) }) }; // Ensure that we mount /boot readonly because it's really owned by bootc/ostree @@ -2921,7 +2921,7 @@ fn read_boot_fstab_entry(root: &Dir) -> Result> { let spec = MountSpec::from_str(line)?; // Check if this is a /boot entry - if spec.target == "/boot" { + if spec.target == format!("/{BOOT}") { return Ok(Some(spec)); } } diff --git a/crates/lib/src/install/baseline.rs b/crates/lib/src/install/baseline.rs index 86061dffbe..53cb1f9f06 100644 --- a/crates/lib/src/install/baseline.rs +++ b/crates/lib/src/install/baseline.rs @@ -25,6 +25,7 @@ use clap::ValueEnum; use fn_error_context::context; use serde::{Deserialize, Serialize}; +use super::BOOT; use super::MountSpec; use super::RUN_BOOTC; use super::RW_KARG; @@ -879,7 +880,7 @@ pub(crate) fn install_create_rootfs( let bootarg = bootsrc.as_deref().map(|bootsrc| format!("boot={bootsrc}")); let boot = bootsrc.map(|bootsrc| MountSpec { source: bootsrc, - target: "/boot".into(), + target: format!("/{BOOT}"), fstype: MountSpec::AUTO.into(), options: Some("ro".into()), }); @@ -919,14 +920,14 @@ pub(crate) fn install_create_rootfs( let target_rootfs = Dir::open_ambient_dir(&physical_root_path, cap_std::ambient_authority())?; crate::lsm::ensure_dir_labeled(&target_rootfs, "", Some("/".into()), 0o755.into(), sepolicy)?; let physical_root = Dir::open_ambient_dir(&physical_root_path, cap_std::ambient_authority())?; - let bootfs = physical_root_path.join("boot"); + let bootfs = physical_root_path.join(BOOT); // Create the underlying mount point directory, which should be labeled - crate::lsm::ensure_dir_labeled(&target_rootfs, "boot", None, 0o755.into(), sepolicy)?; + crate::lsm::ensure_dir_labeled(&target_rootfs, BOOT, None, 0o755.into(), sepolicy)?; if let Some(bootdev) = bootdev { bootc_mount::mount_typed(&bootdev.path(), fstype, &bootfs)?; } // And we want to label the root mount of /boot - crate::lsm::ensure_dir_labeled(&target_rootfs, "boot", None, 0o755.into(), sepolicy)?; + crate::lsm::ensure_dir_labeled(&target_rootfs, BOOT, None, 0o755.into(), sepolicy)?; // Create the EFI system partition, if applicable if let Some(esp_partno) = layout.esp_partno { diff --git a/crates/lib/src/lints.rs b/crates/lib/src/lints.rs index aa41e9cfd4..f3c599d6d2 100644 --- a/crates/lib/src/lints.rs +++ b/crates/lib/src/lints.rs @@ -765,7 +765,7 @@ Any content here in the container image will be masked at runtime. check_boot, ); fn check_boot(root: &Dir, config: &LintExecutionConfig) -> LintResult { - let Some(d) = root.open_dir_optional("boot")? else { + let Some(d) = root.open_dir_optional(crate::install::BOOT)? else { return lint_err("Missing /boot directory"); }; diff --git a/crates/lib/src/store/mod.rs b/crates/lib/src/store/mod.rs index e25ac67c1c..dbda828197 100644 --- a/crates/lib/src/store/mod.rs +++ b/crates/lib/src/store/mod.rs @@ -97,7 +97,7 @@ use std::sync::Arc; use anyhow::{Context, Result}; use bootc_mount::tempmount::TempMount; use bootc_mount::{Filesystem, run_findmnt}; -use camino::Utf8PathBuf; +use camino::{Utf8Path, Utf8PathBuf}; use cap_std_ext::cap_std; use cap_std_ext::cap_std::fs::{ Dir, DirBuilder, DirBuilderExt as _, Permissions, PermissionsExt as _, @@ -119,6 +119,7 @@ use composefs_ctl::composefs; use crate::bootc_composefs::backwards_compat::bcompat_boot::prepend_custom_prefix; use crate::bootc_composefs::boot::{EFI_LINUX, mount_esp_readonly, mount_esp_writable}; use crate::bootc_composefs::status::{ComposefsCmdline, composefs_booted, get_bootloader}; +use crate::install::BOOT; use crate::lsm; use crate::podstorage::CStorage; use crate::spec::{BootloaderKind, ImageStatus}; @@ -371,28 +372,91 @@ fn sysroot_is_read_only(d: &Dir) -> Result { Ok(false) } +/// Where GRUB's configuration and the BLS entries live on a booted composefs +/// system. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum GrubBootLocation { + /// `boot` on the physical root, i.e. `/sysroot/boot` + PhysicalRoot, + /// A separate boot partition, mounted on `/boot`. It isn't mounted on + /// `/sysroot/boot`, which is just an empty directory then. + BootMount, +} + +impl GrubBootLocation { + /// Decide from the partition type and filesystem type of the filesystem + /// mounted on `/boot`, and whether it has a GRUB directory. + fn classify(parttype: Option<&str>, fstype: &str, has_grub_dir: bool) -> Self { + use crate::discoverable_partition_specification as dps; + + // The ESP mounted on /boot: GRUB's configuration is in /sysroot/boot + let is_esp = match parttype { + Some(p) if p.eq_ignore_ascii_case(dps::ESP) => true, + Some(p) if p.eq_ignore_ascii_case(dps::XBOOTLDR) => false, + // No (or another) DPS type, so go by the filesystem + _ => fstype == "vfat", + }; + // Otherwise it's a boot partition (XBOOTLDR or not), which should + // have GRUB's configuration, but check just in case + if !is_esp && has_grub_dir { + Self::BootMount + } else { + Self::PhysicalRoot + } + } + + /// The absolute path of the directory in the booted root + fn path(self) -> Utf8PathBuf { + let parent = match self { + Self::PhysicalRoot => "/sysroot", + Self::BootMount => "/", + }; + Utf8Path::new(parent).join(BOOT) + } +} + +/// Find the directory holding GRUB's configuration and the BLS entries on a +/// booted system, returning it and its absolute path. +/// +/// This is `/sysroot/boot`, unless there's a separate boot partition, which +/// is mounted on `/boot`. How it gets mounted there varies: `bootc install` +/// adds a `systemd.mount-extra` karg, but other tools may use e.g. an fstab +/// entry instead. Everything that reads or writes GRUB's boot entries must go +/// through this, via [`Storage::require_boot_dir`] or +/// [`Storage::require_grub_boot_path`]. #[context("Finding boot for Grub")] -fn get_boot_dir_for_grub(physical_root: &Dir) -> Result { +fn get_boot_dir_for_grub(physical_root: &Dir) -> Result<(Dir, Utf8PathBuf)> { // We have this so systemd's boot.automount shouldn't expire until // this function finishes execution as we have a handle to /boot - let boot = - Dir::open_ambient_dir("/boot", ambient_authority()).context("Failed to open /boot")?; + let boot_mount_path = GrubBootLocation::BootMount.path(); + let boot = Dir::open_ambient_dir(&boot_mount_path, ambient_authority()) + .with_context(|| format!("Opening {boot_mount_path}"))?; + + let open = |location: GrubBootLocation| -> Result<(Dir, Utf8PathBuf)> { + let dir = match location { + GrubBootLocation::PhysicalRoot => physical_root + .open_dir(BOOT) + .context("Opening boot in physical root")?, + // Remount /boot if mounted RO + GrubBootLocation::BootMount => open_dir_remount_rw(&boot, ".".into())?, + }; + Ok((dir, location.path())) + }; let is_boot_mntpnt = boot .is_mountpoint(".") - .context("Checking if /boot is a mountpoint")?; + .with_context(|| format!("Checking if {boot_mount_path} is a mountpoint"))?; // /boot is not a mount point for bootloader Grub, so we have to // have stuff in /sysroot/boot if !matches!(is_boot_mntpnt, Some(true)) { - return physical_root - .open_dir("boot") - .context("Opening boot in physical root"); + return open(GrubBootLocation::PhysicalRoot); } // /boot is a mountpoint - // Figure out if it's the ESP or XBOOTLDR - let mnt_res = run_findmnt(&[], None, Some("/boot")).context("Finding /boot mount info")?; + // Figure out if it's the ESP or a boot partition + let mnt_res = run_findmnt(&[], None, Some(boot_mount_path.as_str())) + .with_context(|| format!("Finding {boot_mount_path} mount info"))?; let mut boot_mount_details: Option = None; @@ -405,7 +469,7 @@ fn get_boot_dir_for_grub(physical_root: &Dir) -> Result { if let Some(already_found) = boot_mount_details { // Really shouldn't happen, but for sanity anyhow::bail!( - "Found multiple mounts on /boot. Found {}, already had {}", + "Found multiple mounts on {boot_mount_path}. Found {}, already had {}", mount.source, already_found.source, ); @@ -415,47 +479,15 @@ fn get_boot_dir_for_grub(physical_root: &Dir) -> Result { } let boot_mount_details = boot_mount_details - .ok_or_else(|| anyhow::anyhow!("Failed to get mount details for /boot"))?; - - let boot_fs = boot_mount_details.fstype; - - let boot_dev_info = bootc_blockdev::list_dev(&Utf8PathBuf::from(boot_mount_details.source))?; - - let boot_is_esp = || { - // /boot is ESP so we have grub configs in /sysroot/boot - return physical_root - .open_dir("boot") - .context("Opening boot in physical root"); - }; - - let boot_is_xbootldr = || { - // XBOOTLDR, so grub configs should hopefully be here - // but check just in case - if boot.is_dir("grub2") { - // Remount /boot if mounted RO - return open_dir_remount_rw(&boot, ".".into()); - } - - return physical_root - .open_dir("boot") - .context("Opening boot in physical root"); - }; - - let return_boot_by_fs = || match boot_fs.as_ref() { - "vfat" => boot_is_esp(), - _ => boot_is_xbootldr(), - }; + .ok_or_else(|| anyhow::anyhow!("Failed to get mount details for {boot_mount_path}"))?; - use crate::discoverable_partition_specification as dps; + let boot_dev_info = bootc_blockdev::list_dev(&Utf8PathBuf::from(&boot_mount_details.source))?; - match boot_dev_info.parttype { - Some(parttype) => match parttype.as_str() { - dps::ESP => boot_is_esp(), - dps::XBOOTLDR => boot_is_xbootldr(), - _ => return_boot_by_fs(), - }, - None => return_boot_by_fs(), - } + open(GrubBootLocation::classify( + boot_dev_info.parttype.as_deref(), + &boot_mount_details.fstype, + boot.is_dir("grub2"), + )) } impl BootedStorage { @@ -486,12 +518,15 @@ impl BootedStorage { EspAccess::ReadWrite => mount_esp_writable(&esp_path)?, }; - let boot_dir = match get_bootloader()?.kind()? { + let (boot_dir, grub_boot_path) = match get_bootloader()?.kind()? { // We can have a separate /boot and not /sysroot/boot - BootloaderKind::GRUBClassic => get_boot_dir_for_grub(&physical_root)?, + BootloaderKind::GRUBClassic => { + let (dir, path) = get_boot_dir_for_grub(&physical_root)?; + (dir, Some(path)) + } // NOTE: Handle XBOOTLDR partitions here if and when we use it BootloaderKind::BLSCompatible => { - esp_mount.fd.try_clone().context("Cloning fd")? + (esp_mount.fd.try_clone().context("Cloning fd")?, None) } }; @@ -501,6 +536,7 @@ impl BootedStorage { is_ro, run, boot_dir: Some(boot_dir), + grub_boot_path, esp: Some(esp_mount), ostree: Default::default(), composefs: OnceCell::from(composefs.clone()), @@ -546,6 +582,7 @@ impl BootedStorage { is_ro, run, boot_dir: None, + grub_boot_path: None, esp: None, ostree: OnceCell::from(sysroot), composefs: Default::default(), @@ -599,10 +636,14 @@ pub(crate) struct Storage { pub(crate) is_ro: bool, /// The 'boot' directory, useful and `Some` only for composefs systems - /// For grub booted systems, this points to `/sysroot/boot` + /// For grub booted systems, this points to `/sysroot/boot`, or to `/boot` + /// if that's a separate boot partition /// For systemd booted systems, this points to the ESP pub boot_dir: Option, + /// The absolute path of `boot_dir` on composefs systems booted with grub + grub_boot_path: Option, + /// The ESP mounted at a tmp location pub esp: Option, @@ -667,6 +708,7 @@ impl Storage { is_ro: false, run, boot_dir: None, + grub_boot_path: None, esp: None, ostree: ostree_cell, composefs: Default::default(), @@ -691,6 +733,15 @@ impl Storage { .ok_or_else(|| anyhow::anyhow!("Boot dir not found")) } + /// Returns the absolute path of `boot_dir` on a composefs system booted + /// with grub, for code that works with paths rather than directory fds: + /// `/boot` if it's a separate boot partition, `/sysroot/boot` otherwise + pub(crate) fn require_grub_boot_path(&self) -> Result<&Utf8Path> { + self.grub_boot_path + .as_deref() + .ok_or_else(|| anyhow::anyhow!("Grub boot path not found")) + } + /// Returns the mounted `esp` if it exists pub(crate) fn require_esp(&self) -> Result<&TempMount> { self.esp @@ -699,7 +750,7 @@ impl Storage { } /// Returns the Directory where the Type1 boot binaries are stored - /// `/sysroot/boot` for Grub, and ESP/EFI/Linux for systemd-boot + /// The grub boot directory (see `require_boot_dir`) for Grub, and ESP/EFI/Linux for systemd-boot pub(crate) fn bls_boot_binaries_dir(&self) -> Result { let boot_dir = self.require_boot_dir()?; @@ -925,4 +976,41 @@ mod tests { Ok(()) } + + #[test] + fn test_grub_boot_location_classify() { + use crate::discoverable_partition_specification as dps; + use GrubBootLocation::*; + + const LINUX_DATA: &str = "0fc63daf-8483-4772-8e79-3d69d8477de4"; + let upper_xbootldr = dps::XBOOTLDR.to_ascii_uppercase(); + + // (parttype, fstype, has grub dir, expected) + let cases = [ + // The ESP on /boot never holds GRUB's configuration + (Some(dps::ESP), "vfat", true, PhysicalRoot), + (Some(dps::ESP), "vfat", false, PhysicalRoot), + // XBOOTLDR, whatever its filesystem, if it has GRUB's configuration + (Some(dps::XBOOTLDR), "ext4", true, BootMount), + (Some(dps::XBOOTLDR), "vfat", true, BootMount), + (Some(dps::XBOOTLDR), "ext4", false, PhysicalRoot), + (Some(upper_xbootldr.as_str()), "xfs", true, BootMount), + // No DPS type: go by the filesystem + (Some(LINUX_DATA), "ext4", true, BootMount), + (Some(LINUX_DATA), "vfat", true, PhysicalRoot), + (None, "xfs", true, BootMount), + (None, "btrfs", false, PhysicalRoot), + (None, "vfat", true, PhysicalRoot), + ]; + for (parttype, fstype, has_grub_dir, expected) in cases { + assert_eq!( + GrubBootLocation::classify(parttype, fstype, has_grub_dir), + expected, + "{parttype:?} {fstype} grub dir: {has_grub_dir}" + ); + } + + assert_eq!(PhysicalRoot.path(), "/sysroot/boot"); + assert_eq!(BootMount.path(), "/boot"); + } } diff --git a/tmt/plans/integration.fmf b/tmt/plans/integration.fmf index f0a9fee02a..cf08422d9e 100644 --- a/tmt/plans/integration.fmf +++ b/tmt/plans/integration.fmf @@ -346,4 +346,13 @@ execute: how: fmf test: - /tmt/tests/tests/test-52-install-repart + +/plan-57-composefs-separate-boot: + summary: Test composefs with GRUB and a separate /boot partition + discover: + how: fmf + test: + - /tmt/tests/tests/test-57-composefs-separate-boot + extra-skip_if_ostree: true + extra-fixme_skip_if_uki: true # END GENERATED PLANS diff --git a/tmt/tests/booted/test-composefs-separate-boot.nu b/tmt/tests/booted/test-composefs-separate-boot.nu new file mode 100644 index 0000000000..7bbe26f337 --- /dev/null +++ b/tmt/tests/booted/test-composefs-separate-boot.nu @@ -0,0 +1,291 @@ +# number: 57 +# tmt: +# summary: Test composefs with GRUB and a separate /boot partition +# duration: 60m +# require: +# - dosfstools +# - e2fsprogs +# extra: +# skip_if_ostree: true +# fixme_skip_if_uki: true +# +# Disk images from e.g. image-builder have a separate /boot partition, which +# the VMs these tests run in don't have. With GRUB, the boot partition holds +# GRUB's configuration, the BLS entries and the kernels, is mounted on /boot +# (by a systemd.mount-extra karg from bootc install, or e.g. an fstab entry), +# and /sysroot/boot is empty. +# +# Booting a disk installed inside the VM isn't possible here, so the VM's own +# disk is given that layout instead, as `bootc install` would have made it: +# +# 1. Split the ESP into a smaller ESP and an ext4 XBOOTLDR partition, and move +# everything in /sysroot/boot there: the BLS entries then refer to the +# kernels relative to the boot partition, get the systemd.mount-extra karg, +# and GRUB finds its configuration there by the filesystem UUID in +# bootuuid.cfg. +# 2. Switch to a derived image with its own initramfs, so that its kernel +# and initramfs are written to the boot partition too. +# 3. Upgrade to a newer build of that image, which goes back to the base +# image's initramfs: GC must keep that one, and drop nothing in use. +# 4. Roll back. +# 5. The rollback worked, and nothing was written to /sysroot/boot. + +use std assert +use tap.nu + +const XBOOTLDR_TYPE = "BC13C2FF-59E6-4262-A352-B275FD6F7172" +const ESP_TYPE = "C12A7328-F81F-11D2-BA4B-00A0C93EC93B" +# The size the VM's ESP is shrunk to; the boot partition gets the rest of it +const NEW_ESP_SIZE_MIB = 256 +const SECTOR_SIZE = 512 +const PHYSICAL_BOOT = "/sysroot/boot" +const STATE_FILE = "/var/separate-boot-test.json" +const DERIVED_IMAGE = "localhost/bootc-separate-boot-derived" +const MARKER = "/usr/share/bootc-separate-boot-marker" +const KERNEL_DIR_PREFIX = "bootc_composefs-" + +def partitions [disk: string] { + (sfdisk --json $disk | from json).partitiontable.partitions +} + +def partition_of_type [disk: string, type: string] { + partitions $disk | where { |p| ($p.type | str upcase) == $type } | first +} + +def partition_number [node: string] { + $node | str replace -r '.*[^0-9]' '' | into int +} + +# The path of an existing mount of `dev`, or a new one. +def find_or_mount [dev: string, name: string] { + let existing = (findmnt -rn -o TARGET -S $dev | lines) + if ($existing | is-not-empty) { + return ($existing | first) + } + let path = $"/var/mnt/($name)" + mkdir $path + mount $dev $path + $path +} + +# The kernel directories the BLS entries refer to, relative to the boot +# partition. +def referenced_kernel_dirs [boot: string] { + glob $"($boot)/loader/entries*/*.conf" + | each { |f| open $f | lines | where { |l| $l | str starts-with "linux " } } + | flatten + | each { |l| $l | str replace -r '^linux\s+' '' | path dirname } + | uniq +} + +# Check that everything is on the boot partition mounted on /boot, that the +# entries refer to kernels there, that GC left no unreferenced kernels, and +# that /sysroot/boot is still empty. +def assert_boot_layout [] { + let state = (open $STATE_FILE) + # systemd-gpt-auto-generator also sets up an automount on /boot for the + # XBOOTLDR partition, under the systemd.mount-extra mount + let boot_src = (findmnt -J -l -o SOURCE,FSTYPE -M /boot | from json).filesystems + | where fstype != "autofs" | get source + assert equal $boot_src [$state.boot] "/boot is not the boot partition" + assert (ls -a $PHYSICAL_BOOT | is-empty) $"($PHYSICAL_BOOT) is not empty" + + let entries = (glob /boot/loader/entries/*.conf) + print $"BLS entries: ($entries)" + assert ($entries | is-not-empty) "No BLS entries on /boot" + + let referenced = (referenced_kernel_dirs /boot) + print $"Referenced kernel directories: ($referenced)" + for d in $referenced { + assert ($d | str starts-with $"/($KERNEL_DIR_PREFIX)") $"Kernel path not relative to the boot partition: ($d)" + } + let on_disk = (glob $"/boot/($KERNEL_DIR_PREFIX)*" | each { |d| $"/($d | path basename)" }) + print $"Kernel directories on /boot: ($on_disk)" + assert equal ($on_disk | sort) ($referenced | sort) "Kernel directories on /boot don't match the BLS entries" +} + +# Move /sysroot/boot to a new boot partition carved out of the ESP. +def make_separate_boot [] { + let root_part = (findmnt -n -o SOURCE /sysroot | str trim) + let disk = $"/dev/(lsblk -n -o PKNAME $root_part | str trim)" + let esp = (partition_of_type $disk $ESP_TYPE) + let esp_nr = (partition_number $esp.node) + let esp_uuid = (blkid -s UUID -o value $esp.node | str trim) + let root_uuid = (blkid -s UUID -o value $root_part | str trim) + print $"Disk ($disk), ESP ($esp.node) \(($esp.size) sectors\), root ($root_part)" + assert (($esp.size * $SECTOR_SIZE) > ($NEW_ESP_SIZE_MIB * 3 * 1024 * 1024)) "ESP too small to split" + + let esp_backup = "/var/tmp/esp-backup" + /usr/bin/cp -a -T (find_or_mount $esp.node "esp") $esp_backup + for unit in [boot.automount efi.automount boot.mount efi.mount] { + do { systemctl stop $unit } | complete | ignore + } + for t in (findmnt -rn -o TARGET -S $esp.node | lines) { + umount $t + } + + let new_esp_sectors = $NEW_ESP_SIZE_MIB * 1024 * 1024 / $SECTOR_SIZE + let boot_start = $esp.start + $new_esp_sectors + let boot_sectors = $esp.size - $new_esp_sectors + let used_nrs = (partitions $disk | each { |p| partition_number $p.node }) + let boot_nr = (1..128 | where { |n| $n not-in $used_nrs } | first) + # The root filesystem is mounted, so the kernel can't reread the whole + # partition table, and partx updates it one partition at a time. But + # systemd-udevd (v259 and newer) also syncs the kernel's partitions with + # BLKPG when a disk it watches is closed after writing, racing with partx: + # hold the disk's BSD lock, which it honors, until the kernel is updated. + let script = ' + set -euo pipefail + disk=$1 esp_nr=$2 esp_spec=$3 boot_nr=$4 boot_spec=$5 + echo "$esp_spec" | sfdisk --lock=no --no-reread -N "$esp_nr" "$disk" + echo "$boot_spec" | sfdisk --lock=no --no-reread -N "$boot_nr" "$disk" + partx -u --nr "$esp_nr" "$disk" + partx -a --nr "$boot_nr" "$disk" + ' + (udevadm lock $"--device=($disk)" bash -c $script bash $disk $esp_nr + $"start=($esp.start), size=($new_esp_sectors), type=($ESP_TYPE)" + $boot_nr $"start=($boot_start), size=($boot_sectors), type=($XBOOTLDR_TYPE)") + udevadm settle + let boot = (partitions $disk | where start == $boot_start | first).node + for p in [[node sectors]; [$esp.node $new_esp_sectors] [$boot $boot_sectors]] { + let sysfs = $"/sys/class/block/($p.node | path basename)/size" + assert ($sysfs | path exists) $"The kernel doesn't know ($p.node)" + assert equal (open --raw $sysfs | str trim | into int) $p.sectors $"The kernel's size of ($p.node)" + } + print $"New ESP ($esp.node), boot ($boot)" + + # Keep the ESP's filesystem UUID, in case something refers to it + mkfs.vfat -F 32 -n EFI-SYSTEM -i ($esp_uuid | str replace "-" "") $esp.node + mkfs.ext4 -q -L boot $boot + udevadm settle + let boot_uuid = (blkid -s UUID -o value $boot | str trim) + + let esp_dir = (find_or_mount $esp.node "esp") + /usr/bin/cp -a -T $esp_backup $esp_dir + rm -rf $esp_backup + + # Move everything in /sysroot/boot to the boot partition + mount -o remount,rw /sysroot + let boot_dir = (find_or_mount $boot "boot") + /usr/bin/cp -a -T $PHYSICAL_BOOT $boot_dir + for f in (ls -a $PHYSICAL_BOOT | get name) { + rm -rf $f + } + + # The kernels are now relative to the boot partition, which systemd + # mounts on /boot + let mount_karg = $"systemd.mount-extra=UUID=($boot_uuid):/boot:ext4:defaults" + for f in (glob $"($boot_dir)/loader/entries/*.conf") { + # Read it all before overwriting it + let entry = (open --raw $f | lines | each { |l| + if ($l | str starts-with "options ") { + $"($l) ($mount_karg)" + } else { + $l | str replace -r '^(linux|initrd)(\s+)/boot/' '$1$2/' + } + } | str join "\n") + print $"($f):\n($entry)" + assert ($entry | str contains $"linux /($KERNEL_DIR_PREFIX)") $"Unexpected entry ($f)" + $"($entry)\n" | save -f $f + } + + # GRUB finds its configuration by filesystem UUID (see bootupd's static + # GRUB configs) + let uuid_files = (glob $"($esp_dir)/EFI/*/bootuuid.cfg" | append (glob $"($boot_dir)/grub2/bootuuid.cfg")) + print $"Pointing ($uuid_files) at ($boot_uuid)" + assert ($uuid_files | is-not-empty) "No bootuuid.cfg found" + for f in $uuid_files { + let content = (open --raw $f) + assert ($content | str contains $root_uuid) $"($f) doesn't refer to the root filesystem: ($content)" + $content | str replace --all $root_uuid $boot_uuid | save -f $f + } + + { boot: $boot } | to json | save -f $STATE_FILE + sync +} + +# A derived image with a marker. With `own_initramfs`, its initramfs differs +# from the base image's (zero padding is ignored when unpacking it), so it +# gets its own copy on the boot partition. +def build_derived [marker: string, --own-initramfs] { + let initramfs_step = if $own_initramfs { + "RUN for f in /usr/lib/modules/*/initramfs.img; do head -c 512 /dev/zero >> $f; done\n" + } else { + "" + } + $"FROM localhost/bootc\n($initramfs_step)RUN echo ($marker) > ($MARKER)\n" | podman build -t $DERIVED_IMAGE -f - . +} + +def assert_marker [expected: string] { + assert ($MARKER | path exists) "Not booted into the derived image" + assert equal (open $MARKER | str trim) $expected +} + +def first_boot [] { + tap begin "composefs with GRUB and a separate /boot" + + let st = bootc status --json | from json + let cfs = $st.status.booted.composefs + let bootloader = ($cfs.bootloader | str downcase) + if $bootloader != "grub" or ($cfs.bootType | str downcase) != "bls" { + print $"Only GRUB with BLS entries is covered, not ($bootloader) ($cfs.bootType); skipping" + tap ok + return + } + if not ("/sys/firmware/efi" | path exists) { + print "Only UEFI is covered; skipping" + tap ok + return + } + + make_separate_boot + tmt-reboot +} + +def second_boot [] { + assert_boot_layout + bootc status + + bootc image copy-to-storage + build_derived v1 --own-initramfs + bootc switch --transport containers-storage $DERIVED_IMAGE + assert (glob /boot/loader/entries.staged/*.conf | is-not-empty) "No staged entries on /boot" + tmt-reboot +} + +def third_boot [] { + assert_marker v1 + assert_boot_layout + + build_derived v2 + bootc upgrade + assert (glob /boot/loader/entries.staged/*.conf | is-not-empty) "No staged entries on /boot" + tmt-reboot +} + +def fourth_boot [] { + assert_marker v2 + assert_boot_layout + + bootc rollback + assert (bootc status --json | from json).status.rollbackQueued + tmt-reboot +} + +def fifth_boot [] { + assert_marker v1 + assert_boot_layout + tap ok +} + +def main [] { + # See https://tmt.readthedocs.io/en/stable/stories/features.html#reboot-during-test + match $env.TMT_REBOOT_COUNT? { + null | "0" => first_boot, + "1" => second_boot, + "2" => third_boot, + "3" => fourth_boot, + "4" => fifth_boot, + $o => { error make { msg: $"Invalid TMT_REBOOT_COUNT ($o)" } }, + } +} diff --git a/tmt/tests/tests.fmf b/tmt/tests/tests.fmf index 2139a2c576..b4a1af34de 100644 --- a/tmt/tests/tests.fmf +++ b/tmt/tests/tests.fmf @@ -216,3 +216,11 @@ check: summary: Test bootc install to-disk with systemd-repart partitioning duration: 30m test: nu booted/test-install-repart.nu + +/test-57-composefs-separate-boot: + summary: Test composefs with GRUB and a separate /boot partition + duration: 60m + require: + - dosfstools + - e2fsprogs + test: nu booted/test-composefs-separate-boot.nu