Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 37 additions & 27 deletions crates/lib/src/bootc_composefs/boot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ pub(crate) const EFI_LINUX: &str = "EFI/Linux";
const SYSTEMD_TIMEOUT: &str = "timeout 5";
const SYSTEMD_LOADER_CONF_PATH: &str = "loader/loader.conf";

const INITRD: &str = "initrd";
const VMLINUZ: &str = "vmlinuz";

/// We want to be able to control the ordering of UKIs so we put them in a directory that's not the
/// directory specified by the BLS spec. We do this because we want systemd-boot to only look at
/// our config files and not show the actual UKIs in the bootloader menu
Expand Down Expand Up @@ -280,7 +283,7 @@ fn write_bls_boot_entries_to_disk(

entries_dir
.atomic_write(
"vmlinuz",
VMLINUZ,
read_file(&entry.vmlinuz, &repo).context("Reading vmlinuz")?,
)
.context("Writing vmlinuz to path")?;
Expand All @@ -291,7 +294,7 @@ fn write_bls_boot_entries_to_disk(

entries_dir
.atomic_write(
"initrd",
INITRD,
read_file(initramfs, &repo).context("Reading initrd")?,
)
.context("Writing initrd to path")?;
Expand Down Expand Up @@ -351,12 +354,11 @@ fn osrel_title_and_version(
Ok(Some((title, version)))
}

struct BLSEntryPath<'a> {
struct BLSEntryPath {
/// Where to write vmlinuz/initrd
entries_path: Utf8PathBuf,
/// The absolute path, with reference to the partition's root, where the vmlinuz/initrd are written to
/// We need this as when installing, the mounted path will not
abs_entries_path: &'a str,
abs_entries_path: Utf8PathBuf,
/// Where to write the .conf files
config_path: Utf8PathBuf,
}
Expand Down Expand Up @@ -419,14 +421,29 @@ pub(crate) fn setup_composefs_bls_boot(
let is_upgrade = matches!(setup_type, BootSetupType::Upgrade(..));

let (entry_paths, _tmpdir_guard) = match bootloader {
Bootloader::Grub => (
BLSEntryPath {
entries_path: root_path.join("boot"),
config_path: root_path.join("boot"),
abs_entries_path: "boot",
},
None,
),
Bootloader::Grub => {
let root = Dir::open_ambient_dir(&root_path, ambient_authority())
.context("Opening root 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",
};

(
BLSEntryPath {
entries_path: root_path.join("boot"),
config_path: root_path.join("boot"),
abs_entries_path: entries_path.into(),
},
None,
)
}
Comment on lines +424 to +446
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For clarity and to avoid confusion, it would be better to rename the local variable entries_path. It currently has the same name as the entries_path field in the BLSEntryPath struct, but they represent different concepts. The local variable is a path prefix for the bootloader configuration, while the struct field is the directory where kernel/initrd files are written. Renaming it to something like abs_path_prefix would make the code easier to understand.

        Bootloader::Grub => {
            let root = Dir::open_ambient_dir(&root_path, ambient_authority())
                .context("Opening root 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 abs_path_prefix = match root.is_mountpoint("boot")? {
                Some(true) => "/",
                // We can be fairly sure that the kernels we target support `statx`
                Some(false) | None => "/boot",
            };

            (
                BLSEntryPath {
                    entries_path: root_path.join("boot"),
                    config_path: root_path.join("boot"),
                    abs_entries_path: abs_path_prefix.into(),
                },
                None,
            )
        }


Bootloader::Systemd => {
let efi_mount = mount_esp(&esp_device).context("Mounting ESP")?;
Expand All @@ -438,7 +455,7 @@ pub(crate) fn setup_composefs_bls_boot(
BLSEntryPath {
entries_path: efi_linux_dir,
config_path: mounted_efi.clone(),
abs_entries_path: EFI_LINUX,
abs_entries_path: Utf8PathBuf::from("/").join(EFI_LINUX),
},
Some(efi_mount),
)
Expand Down Expand Up @@ -474,10 +491,8 @@ pub(crate) fn setup_composefs_bls_boot(
.with_sort_key(default_sort_key.into())
.with_version(version)
.with_cfg(BLSConfigType::NonEFI {
linux: format!("/{}/{id_hex}/vmlinuz", entry_paths.abs_entries_path).into(),
initrd: vec![
format!("/{}/{id_hex}/initrd", entry_paths.abs_entries_path).into()
],
linux: entry_paths.abs_entries_path.join(&id_hex).join(VMLINUZ),
initrd: vec![entry_paths.abs_entries_path.join(&id_hex).join(INITRD)],
options: Some(cmdline_refs),
});

Expand All @@ -491,15 +506,10 @@ pub(crate) fn setup_composefs_bls_boot(
ref mut initrd,
..
} => {
*linux =
format!("/{}/{symlink_to}/vmlinuz", entry_paths.abs_entries_path)
.into();

*initrd = vec![format!(
"/{}/{symlink_to}/initrd",
entry_paths.abs_entries_path
)
.into()];
*linux = entry_paths.abs_entries_path.join(&symlink_to).join(VMLINUZ);

*initrd =
vec![entry_paths.abs_entries_path.join(&symlink_to).join(INITRD)];
}

_ => unreachable!(),
Expand Down
1 change: 1 addition & 0 deletions crates/system-reinstall-bootc/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const CONFIG_VAR: &str = "BOOTC_REINSTALL_CONFIG";
pub(crate) struct ReinstallConfig {
/// The bootc image to install on the system.
pub(crate) bootc_image: String,
pub(crate) composefs_backend: bool,
}

impl ReinstallConfig {
Expand Down
11 changes: 7 additions & 4 deletions crates/system-reinstall-bootc/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,26 @@ const ROOT_KEY_MOUNT_POINT: &str = "/bootc_authorized_ssh_keys/root";
/// file with a single member `bootc_image` that specifies the image to install.
/// This will take precedence over the CLI.
#[derive(clap::Parser)]
struct Opts {
pub(crate) struct ReinstallOpts {
/// The bootc image to install
pub(crate) image: String,
// Note if we ever add any other options here,
#[arg(long)]
pub(crate) composefs_backend: bool,
}

#[context("run")]
fn run() -> Result<()> {
// We historically supported an environment variable providing a config to override the image, so
// keep supporting that. I'm considering deprecating that though.
let opts = if let Some(config) = config::ReinstallConfig::load().context("loading config")? {
Opts {
ReinstallOpts {
image: config.bootc_image,
composefs_backend: config.composefs_backend,
}
} else {
// Otherwise an image is required.
Opts::parse()
ReinstallOpts::parse()
};

bootc_utils::initialize_tracing();
Expand Down Expand Up @@ -68,7 +71,7 @@ fn run() -> Result<()> {

prompt::mount_warning()?;

let mut reinstall_podman_command = podman::reinstall_command(&opts.image, ssh_key_file_path)?;
let mut reinstall_podman_command = podman::reinstall_command(&opts, ssh_key_file_path)?;

println!();
println!("Going to run command:");
Expand Down
12 changes: 8 additions & 4 deletions crates/system-reinstall-bootc/src/podman.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::prompt;
use crate::{prompt, ReinstallOpts};

use super::ROOT_KEY_MOUNT_POINT;
use anyhow::{ensure, Context, Result};
Expand All @@ -25,7 +25,7 @@ fn bootc_has_clean(image: &str) -> Result<bool> {
}

#[context("reinstall_command")]
pub(crate) fn reinstall_command(image: &str, ssh_key_file: &str) -> Result<Command> {
pub(crate) fn reinstall_command(opts: &ReinstallOpts, ssh_key_file: &str) -> Result<Command> {
let mut podman_command_and_args = [
// We use podman to run the bootc container. This might change in the future to remove the
// podman dependency.
Expand Down Expand Up @@ -72,11 +72,15 @@ pub(crate) fn reinstall_command(image: &str, ssh_key_file: &str) -> Result<Comma
.map(String::from)
.to_vec();

if opts.composefs_backend {
bootc_command_and_args.push("--composefs-backend".into());
}

// Enable the systemd service to cleanup the previous install after booting into the
// bootc system for the first time.
// This only happens if the bootc version in the image >= 1.1.8 (this is when the cleanup
// feature was introduced)
if bootc_has_clean(image)? {
if bootc_has_clean(&opts.image)? {
bootc_command_and_args.push("--cleanup".to_string());
}

Expand All @@ -88,7 +92,7 @@ pub(crate) fn reinstall_command(image: &str, ssh_key_file: &str) -> Result<Comma

let all_args = [
podman_command_and_args,
vec![image.to_string()],
vec![opts.image.to_string()],
bootc_command_and_args,
]
.concat();
Expand Down