Skip to content
Open
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
7 changes: 6 additions & 1 deletion crates/lib/src/bootloader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,12 @@ pub(crate) fn install_via_bootupd(
println!("Installing bootloader via bootupd");

// Build the bootupctl arguments
let mut bootupd_args: Vec<&str> = vec!["backend", "install", "--write-uuid"];
let mut bootupd_args: Vec<&str> = vec!["backend", "install"];
if configopts.skip_boot_uuid_stamp {
bootupd_args.push("--with-static-configs")
} else {
bootupd_args.push("--write-uuid");
}
if let Some(v) = verbose {
bootupd_args.push(v);
}
Expand Down
16 changes: 14 additions & 2 deletions crates/lib/src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,11 @@ pub(crate) struct InstallConfigOpts {
/// The stateroot name to use. Defaults to `default`.
#[clap(long)]
pub(crate) stateroot: Option<String>,

/// Don't pass --write-uuid to bootupd during bootloader installation.
#[clap(long)]
#[serde(default)]
pub(crate) skip_boot_uuid_stamp: bool,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also per below how about --bootupd-skip-boot-uuid

}

#[derive(Debug, Default, Clone, clap::Parser, Serialize, Deserialize, PartialEq, Eq)]
Expand Down Expand Up @@ -1512,7 +1517,7 @@ async fn verify_target_fetch(

/// Preparation for an install; validates and prepares some (thereafter immutable) global state.
async fn prepare_install(
config_opts: InstallConfigOpts,
mut config_opts: InstallConfigOpts,
source_opts: InstallSourceOpts,
target_opts: InstallTargetOpts,
mut composefs_options: InstallComposefsOpts,
Expand Down Expand Up @@ -1637,8 +1642,15 @@ async fn prepare_install(
}

let install_config = config::load_config()?;
if install_config.is_some() {
if let Some(ref config) = install_config {
tracing::debug!("Loaded install configuration");
// Merge config file values into config_opts (CLI takes precedence)
// Only apply config file value if CLI didn't explicitly set it
if !config_opts.skip_boot_uuid_stamp {
if let Some(skip) = config.skip_boot_uuid_stamp {
config_opts.skip_boot_uuid_stamp = skip;
}
}
Comment on lines +1649 to +1653
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This logic for merging the skip_boot_uuid_stamp option can be simplified. The nested if let can be replaced with unwrap_or to make the code more concise while preserving the same behavior.

        if !config_opts.skip_boot_uuid_stamp {
            config_opts.skip_boot_uuid_stamp = config.skip_boot_uuid_stamp.unwrap_or(false);
        }

} else {
tracing::debug!("No install configuration found");
}
Expand Down
58 changes: 58 additions & 0 deletions crates/lib/src/install/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ pub(crate) struct InstallConfiguration {
pub(crate) root_mount_spec: Option<String>,
/// Mount specification for the /boot filesystem.
pub(crate) boot_mount_spec: Option<String>,
/// Wether to skip writing the boot partition UUID to the bootloader configuration.
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

There's a typo in the comment. "Wether" should be "Whether".

Suggested change
/// Wether to skip writing the boot partition UUID to the bootloader configuration.
/// Whether to skip writing the boot partition UUID to the bootloader configuration.

/// When true, uses --with-static-configs instead of --write-uuid for bootupd.
/// Defaults to false if not specified (the boot UUID is written by default).
pub(crate) skip_boot_uuid_stamp: Option<bool>,
}

fn merge_basic<T>(s: &mut Option<T>, o: Option<T>, _env: &EnvProperties) {
Expand Down Expand Up @@ -160,6 +164,11 @@ impl Mergeable for InstallConfiguration {
merge_basic(&mut self.stateroot, other.stateroot, env);
merge_basic(&mut self.root_mount_spec, other.root_mount_spec, env);
merge_basic(&mut self.boot_mount_spec, other.boot_mount_spec, env);
merge_basic(
&mut self.skip_boot_uuid_stamp,
other.skip_boot_uuid_stamp,
env,
);
if let Some(other_kargs) = other.kargs {
self.kargs
.get_or_insert_with(Default::default)
Expand Down Expand Up @@ -731,4 +740,53 @@ boot-mount-spec = ""
assert_eq!(install.root_mount_spec.as_deref().unwrap(), "");
assert_eq!(install.boot_mount_spec.as_deref().unwrap(), "");
}

#[test]
fn test_parse_skip_boot_uuid_stamp() {
// Test parsing true
let c: InstallConfigurationToplevel = toml::from_str(
r#"[install]
skip-boot-uuid-stamp = true
"#,
)
.unwrap();
assert_eq!(c.install.unwrap().skip_boot_uuid_stamp.unwrap(), true);

// Test parsing false
let c: InstallConfigurationToplevel = toml::from_str(
r#"[install]
skip-boot-uuid-stamp = false
"#,
)
.unwrap();
assert_eq!(c.install.unwrap().skip_boot_uuid_stamp.unwrap(), false);

// Test default (not specified) is None
let c: InstallConfigurationToplevel = toml::from_str(
r#"[install]
root-fs-type = "xfs"
"#,
)
.unwrap();
assert!(c.install.unwrap().skip_boot_uuid_stamp.is_none());
}

#[test]
fn test_merge_skip_boot_uuid_stamp() {
let env = EnvProperties {
sys_arch: "x86_64".to_string(),
};
let mut install: InstallConfiguration = toml::from_str(
r#"skip-boot-uuid-stamp = false
"#,
)
.unwrap();
let other = InstallConfiguration {
skip_boot_uuid_stamp: Some(true),
..Default::default()
};
install.merge(other, &env);
// skip_boot_uuid_stamp should be overridden to true
assert_eq!(install.skip_boot_uuid_stamp.unwrap(), true);
}
}
3 changes: 3 additions & 0 deletions docs/src/man/bootc-install-config.5.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ The `install` section supports these subfields:
- `boot-mount-spec`: A string specifying the /boot filesystem mount specification.
If not provided and /boot is a separate mount, its UUID will be used.
An empty string signals to omit boot mount kargs entirely.
- `skip-boot-uuid-stamp`: A boolean that controls whether to skip writing partition UUIDs
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is actually specific to bootupd and does not apply with e.g. sdboot and DPS in general. I think it's also meaningless with s390x. I wonder if we should have a dedicated bootupd section with this as the first option.

Now...then the next question is if we change things so that we error if bootupd isn't used. That would force cases like the FCOS image builds to conditionalize the install config per-arch/layout, but it does feel more right to me instead of silently ignoring it here, which is less compatible if we ever aded any other knobs

Copy link
Contributor Author

@jbtrystram jbtrystram Feb 4, 2026

Choose a reason for hiding this comment

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

Another approach suggested by @jlebon was to hide that logic behind the boot-mount-spec="" special case, since this is effectively signaling that the user want to own that process. But the kernel mount spec and a bootloader mount spec are still technically different stages, so I am not sure it's a better approach.

Now about having it fail when not applicable.. I am not really opposed to it. It make the pipeline slightly more complex, but we already have different manifests for each arch so I guess that isn't too much more work.

Final question: you said "bootupd", by that you mean grub ? Isn't bootupd support sd-boot at some point ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Final question: you said "bootupd", by that you mean grub ? Isn't bootupd support sd-boot at some point ?

Yes well the main argument is to support the shim + sdboot case actually but the uuid is really IMO a legacy for GRUB systems and newer things should use the DPS which we enforce with the sealeduki-composefs case.

(I have a WIP to allow DPS with ostree+grub but it requires newer Fedora)

to the bootloader configuration. When `true`, bootupd is invoked with `--with-static-configs`
instead of `--write-uuid`. Defaults to `false` (UUIDs are written by default).

# filesystem

Expand Down
Loading