Skip to content

Make update and switch operations idempotent #1797

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Make update and switch operations idempotent #1797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ea4253b
composefs/bls: Get cmdline from usr/lib/bootc/kargs.d
Johan-Liebert1 Nov 19, 2025
0c9b17f
storage: Add `boot_dir` and `esp` fields
Johan-Liebert1 Nov 19, 2025
20eb4e4
kargs: Handle addition/removal of kargs from kargs.d
Johan-Liebert1 Nov 20, 2025
dee046d
composefs/update: Ensure idempotency on update
Johan-Liebert1 Nov 21, 2025
b9ecaac
composefs: Ensure idempotency for switch
Johan-Liebert1 Nov 21, 2025
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
30 changes: 26 additions & 4 deletions crates/lib/src/bootc_composefs/boot.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::path::Path;

use anyhow::{anyhow, Context, Result};
use bootc_blockdev::find_parent_devices;
use bootc_kernel_cmdline::utf8::Cmdline;
use bootc_kernel_cmdline::utf8::{Cmdline, Parameter};
use bootc_mount::inspect_filesystem_of_dir;
use bootc_mount::tempmount::TempMount;
use camino::{Utf8Path, Utf8PathBuf};
Expand Down Expand Up @@ -33,6 +33,7 @@ use rustix::{mount::MountFlags, path::Arg};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

use crate::bootc_kargs::kargs_from_composefs_filesystem;
use crate::composefs_consts::{TYPE1_ENT_PATH, TYPE1_ENT_PATH_STAGED};
use crate::parsers::bls_config::{BLSConfig, BLSConfigType};
use crate::parsers::grub_menuconfig::MenuEntry;
Expand All @@ -51,7 +52,6 @@ use crate::{
BOOT_LOADER_ENTRIES, COMPOSEFS_CMDLINE, ORIGIN_KEY_BOOT, ORIGIN_KEY_BOOT_DIGEST,
STAGED_BOOT_LOADER_ENTRIES, STATE_DIR_ABS, USER_CFG, USER_CFG_STAGED,
},
install::RW_KARG,
spec::{Bootloader, Host},
};

Expand Down Expand Up @@ -384,7 +384,7 @@ pub(crate) fn setup_composefs_bls_boot(
) -> Result<String> {
let id_hex = id.to_hex();

let (root_path, esp_device, cmdline_refs, fs, bootloader) = match setup_type {
let (root_path, esp_device, mut cmdline_refs, fs, bootloader) = match setup_type {
BootSetupType::Setup((root_setup, state, postfetch, fs)) => {
// root_setup.kargs has [root=UUID=<UUID>, "rw"]
let mut cmdline_options = Cmdline::new();
Expand Down Expand Up @@ -415,10 +415,30 @@ pub(crate) fn setup_composefs_bls_boot(
let sysroot_parent = get_sysroot_parent_dev(&storage.physical_root)?;
let bootloader = host.require_composefs_booted()?.bootloader.clone();

let boot_dir = storage.require_boot_dir()?;
let current_cfg = get_booted_bls(&boot_dir)?;

let mut cmdline = match current_cfg.cfg_type {
BLSConfigType::NonEFI { options, .. } => {
let options = options
.ok_or_else(|| anyhow::anyhow!("No 'options' found in BLS Config"))?;

Cmdline::from(options)
}

_ => anyhow::bail!("Found NonEFI config"),
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 21, 2025

Choose a reason for hiding this comment

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

medium

The error message here is confusing. It says "Found NonEFI config", but it's in the _ arm of a match where the only other arm is BLSConfigType::NonEFI. This means it triggers for anything that is not NonEFI. A better error message would clarify that a NonEFI config was expected.

Suggested change
_ => anyhow::bail!("Found NonEFI config"),
_ => anyhow::bail!("Unsupported BLS config type: expected a NonEFI config"),

};

// Copy all cmdline args, replacing only `composefs=`
let param = format!("{COMPOSEFS_CMDLINE}={id_hex}");
let param =
Parameter::parse(&param).context("Failed to create 'composefs=' parameter")?;
cmdline.add_or_modify(&param);

(
Utf8PathBuf::from("/sysroot"),
get_esp_partition(&sysroot_parent)?.0,
Cmdline::from(format!("{RW_KARG} {COMPOSEFS_CMDLINE}={id_hex}")),
cmdline,
fs,
bootloader,
)
Expand All @@ -427,6 +447,8 @@ pub(crate) fn setup_composefs_bls_boot(

let is_upgrade = matches!(setup_type, BootSetupType::Upgrade(..));

kargs_from_composefs_filesystem(fs, &repo, &mut cmdline_refs, !is_upgrade)?;

let (entry_paths, _tmpdir_guard) = match bootloader {
Bootloader::Grub => {
let root = Dir::open_ambient_dir(&root_path, ambient_authority())
Expand Down
49 changes: 20 additions & 29 deletions crates/lib/src/bootc_composefs/delete.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@ use composefs_boot::bootloader::{EFI_ADDON_DIR_EXT, EFI_EXT};

use crate::{
bootc_composefs::{
boot::{
find_vmlinuz_initrd_duplicates, get_efi_uuid_source, get_esp_partition,
get_sysroot_parent_dev, mount_esp, BootType, SYSTEMD_UKI_DIR,
},
boot::{find_vmlinuz_initrd_duplicates, get_efi_uuid_source, BootType, SYSTEMD_UKI_DIR},
gc::composefs_gc,
repo::open_composefs_repo,
rollback::{composefs_rollback, rename_exchange_user_cfg},
Expand Down Expand Up @@ -215,40 +212,34 @@ fn remove_grub_menucfg_entry(id: &str, boot_dir: &Dir, deleting_staged: bool) ->
#[fn_error_context::context("Deleting boot entries for deployment {}", deployment.deployment.verity)]
fn delete_depl_boot_entries(
deployment: &DeploymentEntry,
physical_root: &Dir,
storage: &Storage,
deleting_staged: bool,
) -> Result<()> {
match deployment.deployment.bootloader {
Bootloader::Grub => {
let boot_dir = physical_root.open_dir("boot").context("Opening boot dir")?;
let boot_dir = storage.require_boot_dir()?;

match deployment.deployment.boot_type {
BootType::Bls => delete_type1_entry(deployment, &boot_dir, deleting_staged),
match deployment.deployment.bootloader {
Bootloader::Grub => match deployment.deployment.boot_type {
BootType::Bls => delete_type1_entry(deployment, boot_dir, deleting_staged),

BootType::Uki => {
let device = get_sysroot_parent_dev(physical_root)?;
let (esp_part, ..) = get_esp_partition(&device)?;
let esp_mount = mount_esp(&esp_part)?;
BootType::Uki => {
let esp = storage
.esp
.as_ref()
.ok_or_else(|| anyhow::anyhow!("ESP not found"))?;

remove_grub_menucfg_entry(
&deployment.deployment.verity,
&boot_dir,
deleting_staged,
)?;
remove_grub_menucfg_entry(
&deployment.deployment.verity,
boot_dir,
deleting_staged,
)?;

delete_uki(&deployment.deployment.verity, &esp_mount.fd)
}
delete_uki(&deployment.deployment.verity, &esp.fd)
}
}
},

Bootloader::Systemd => {
let device = get_sysroot_parent_dev(physical_root)?;
let (esp_part, ..) = get_esp_partition(&device)?;

let esp_mount = mount_esp(&esp_part)?;

// For Systemd UKI as well, we use .conf files
delete_type1_entry(deployment, &esp_mount.fd, deleting_staged)
delete_type1_entry(deployment, boot_dir, deleting_staged)
}
}
}
Expand Down Expand Up @@ -362,7 +353,7 @@ pub(crate) async fn delete_composefs_deployment(

tracing::info!("Deleting {kind}deployment '{deployment_id}'");

delete_depl_boot_entries(&depl_to_del, &storage.physical_root, deleting_staged)?;
delete_depl_boot_entries(&depl_to_del, &storage, deleting_staged)?;

composefs_gc(storage, booted_cfs).await?;

Expand Down
35 changes: 13 additions & 22 deletions crates/lib/src/bootc_composefs/finalize.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use std::path::Path;

use crate::bootc_composefs::boot::{
get_esp_partition, get_sysroot_parent_dev, mount_esp, BootType,
};
use crate::bootc_composefs::boot::BootType;
use crate::bootc_composefs::rollback::{rename_exchange_bls_entries, rename_exchange_user_cfg};
use crate::bootc_composefs::status::get_composefs_status;
use crate::composefs_consts::STATE_DIR_ABS;
Expand Down Expand Up @@ -86,15 +84,12 @@ pub(crate) async fn composefs_backend_finalize(
// Unmount EROFS
drop(erofs_tmp_mnt);

let sysroot_parent = get_sysroot_parent_dev(&storage.physical_root)?;
// NOTE: Assumption here that ESP will always be present
let (esp_part, ..) = get_esp_partition(&sysroot_parent)?;
let boot_dir = storage.require_boot_dir()?;

let esp_mount = mount_esp(&esp_part)?;
let boot_dir = storage
.physical_root
.open_dir("boot")
.context("Opening boot")?;
let esp_mount = storage
.esp
.as_ref()
.ok_or_else(|| anyhow::anyhow!("ESP not found"))?;

// NOTE: Assuming here we won't have two bootloaders at the same time
match booted_composefs.bootloader {
Expand All @@ -103,21 +98,17 @@ pub(crate) async fn composefs_backend_finalize(
let entries_dir = boot_dir.open_dir("loader")?;
rename_exchange_bls_entries(&entries_dir)?;
}
BootType::Uki => finalize_staged_grub_uki(&esp_mount.fd, &boot_dir)?,
BootType::Uki => finalize_staged_grub_uki(&esp_mount.fd, boot_dir)?,
},

Bootloader::Systemd => match staged_composefs.boot_type {
BootType::Bls => {
let entries_dir = esp_mount.fd.open_dir("loader")?;
rename_exchange_bls_entries(&entries_dir)?;
}
BootType::Uki => {
Bootloader::Systemd => {
if matches!(staged_composefs.boot_type, BootType::Uki) {
rename_staged_uki_entries(&esp_mount.fd)?;

let entries_dir = esp_mount.fd.open_dir("loader")?;
rename_exchange_bls_entries(&entries_dir)?;
}
},

let entries_dir = boot_dir.open_dir("loader")?;
rename_exchange_bls_entries(&entries_dir)?;
}
};

Ok(())
Expand Down
18 changes: 6 additions & 12 deletions crates/lib/src/bootc_composefs/gc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use composefs::fsverity::{FsVerityHashValue, Sha512HashValue};

use crate::{
bootc_composefs::{
boot::{get_esp_partition, get_sysroot_parent_dev, mount_esp},
delete::{delete_image, delete_staged, delete_state_dir, get_image_objects},
status::{
get_bootloader, get_composefs_status, get_sorted_grub_uki_boot_entries,
Expand Down Expand Up @@ -44,28 +43,27 @@ fn list_erofs_images(sysroot: &Dir) -> Result<Vec<String>> {
/// # Returns
/// The fsverity of EROFS images corresponding to boot entries
#[fn_error_context::context("Listing bootloader entries")]
fn list_bootloader_entries(physical_root: &Dir) -> Result<Vec<String>> {
fn list_bootloader_entries(storage: &Storage) -> Result<Vec<String>> {
let bootloader = get_bootloader()?;
let boot_dir = storage.require_boot_dir()?;

let entries = match bootloader {
Bootloader::Grub => {
let boot_dir = physical_root.open_dir("boot").context("Opening boot dir")?;

// Grub entries are always in boot
let grub_dir = boot_dir.open_dir("grub2").context("Opening grub dir")?;

if grub_dir.exists(USER_CFG) {
// Grub UKI
let mut s = String::new();
let boot_entries = get_sorted_grub_uki_boot_entries(&boot_dir, &mut s)?;
let boot_entries = get_sorted_grub_uki_boot_entries(boot_dir, &mut s)?;

boot_entries
.into_iter()
.map(|entry| entry.get_verity())
.collect::<Result<Vec<_>, _>>()?
} else {
// Type1 Entry
let boot_entries = get_sorted_type1_boot_entries(&boot_dir, true)?;
let boot_entries = get_sorted_type1_boot_entries(boot_dir, true)?;

boot_entries
.into_iter()
Expand All @@ -75,11 +73,7 @@ fn list_bootloader_entries(physical_root: &Dir) -> Result<Vec<String>> {
}

Bootloader::Systemd => {
let device = get_sysroot_parent_dev(physical_root)?;
let (esp_part, ..) = get_esp_partition(&device)?;
let esp_mount = mount_esp(&esp_part)?;

let boot_entries = get_sorted_type1_boot_entries(&esp_mount.fd, true)?;
let boot_entries = get_sorted_type1_boot_entries(boot_dir, true)?;

boot_entries
.into_iter()
Expand Down Expand Up @@ -175,7 +169,7 @@ pub(crate) async fn composefs_gc(storage: &Storage, booted_cfs: &BootedComposefs

let sysroot = &storage.physical_root;

let bootloader_entries = list_bootloader_entries(&storage.physical_root)?;
let bootloader_entries = list_bootloader_entries(&storage)?;
let images = list_erofs_images(&sysroot)?;

// Collect the deployments that have an image but no bootloader entry
Expand Down
34 changes: 11 additions & 23 deletions crates/lib/src/bootc_composefs/rollback.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ use cap_std_ext::dirext::CapStdExtDirExt;
use fn_error_context::context;
use rustix::fs::{fsync, renameat_with, AtFlags, RenameFlags};

use crate::bootc_composefs::boot::{
get_esp_partition, get_sysroot_parent_dev, mount_esp, type1_entry_conf_file_name, BootType,
};
use crate::bootc_composefs::boot::{type1_entry_conf_file_name, BootType};
use crate::bootc_composefs::status::{get_composefs_status, get_sorted_type1_boot_entries};
use crate::composefs_consts::TYPE1_ENT_PATH_STAGED;
use crate::spec::Bootloader;
Expand Down Expand Up @@ -196,31 +194,21 @@ pub(crate) async fn composefs_rollback(
anyhow::bail!("Rollback deployment not a composefs deployment")
};

let boot_dir = storage.require_boot_dir()?;

match &rollback_entry.bootloader {
Bootloader::Grub => {
let boot_dir = storage
.physical_root
.open_dir("boot")
.context("Opening boot dir")?;

match rollback_entry.boot_type {
BootType::Bls => {
rollback_composefs_entries(&boot_dir, rollback_entry.bootloader.clone())?;
}

BootType::Uki => {
rollback_grub_uki_entries(&boot_dir)?;
}
Bootloader::Grub => match rollback_entry.boot_type {
BootType::Bls => {
rollback_composefs_entries(boot_dir, rollback_entry.bootloader.clone())?;
}
}
BootType::Uki => {
rollback_grub_uki_entries(boot_dir)?;
}
},

Bootloader::Systemd => {
let parent = get_sysroot_parent_dev(&storage.physical_root)?;
let (esp_part, ..) = get_esp_partition(&parent)?;
let esp_mount = mount_esp(&esp_part)?;

// We use BLS entries for systemd UKI as well
rollback_composefs_entries(&esp_mount.fd, rollback_entry.bootloader.clone())?;
rollback_composefs_entries(boot_dir, rollback_entry.bootloader.clone())?;
}
}

Expand Down
Loading
Loading