Skip to content

install: Do not clean boot directories on ostree systems #1193

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

Merged
merged 1 commit into from
Mar 13, 2025
Merged

install: Do not clean boot directories on ostree systems #1193

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 24 additions & 13 deletions lib/src/install.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1596,21 +1596,31 @@ fn remove_all_in_dir_no_xdev(d: &Dir, mount_err: bool) -> Result<()> {
}

#[context("Removing boot directory content")]
fn clean_boot_directories(rootfs: &Dir) -> Result<()> {
fn clean_boot_directories(rootfs: &Dir, is_ostree: bool) -> Result<()> {
let bootdir =
crate::utils::open_dir_remount_rw(rootfs, BOOT.into()).context("Opening /boot")?;
// This should not remove /boot/efi note.
remove_all_in_dir_no_xdev(&bootdir, false)?;
// TODO: Discover the ESP the same way bootupd does it; we should also
// support not wiping the ESP.
if ARCH_USES_EFI {
if let Some(efidir) = bootdir
.open_dir_optional(crate::bootloader::EFI_DIR)
.context("Opening /boot/efi")?
{
remove_all_in_dir_no_xdev(&efidir, false)?;

if is_ostree {
// On ostree systems, the boot directory already has our desired format, we should only
// remove the bootupd-state.json file to avoid bootupctl complaining it already exists.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm though I think this is going to work when staying within a single OS major usually, in the general case I think we can get some skew here when we don't clear the ESP (as is done below for the non-ostree/bootupd case).

Related to this on the flip side, ostree and bootupd are separate, orthogonal projects and we were actually talking about using bootupd for package-based Fedora too. In that scenario, we would still need to blow away the bootupd state.

But then again really the right fix in the medium term here is again to basically treat this as an upgrade, which bootupd already supports. So, eh.

bootdir
.remove_file_optional("bootupd-state.json")
.context("removing bootupd-state.json")?;
} else {
// This should not remove /boot/efi note.
remove_all_in_dir_no_xdev(&bootdir, false)?;
// TODO: Discover the ESP the same way bootupd does it; we should also
// support not wiping the ESP.
if ARCH_USES_EFI {
if let Some(efidir) = bootdir
.open_dir_optional(crate::bootloader::EFI_DIR)
.context("Opening /boot/efi")?
{
remove_all_in_dir_no_xdev(&efidir, false)?;
}
}
}

Ok(())
}

Expand Down Expand Up @@ -1731,7 +1741,8 @@ pub(crate) async fn install_to_filesystem(
// the deployment root.
let possible_physical_root = fsopts.root_path.join("sysroot");
let possible_ostree_dir = possible_physical_root.join("ostree");
if possible_ostree_dir.exists() {
let is_already_ostree = possible_ostree_dir.exists();
if is_already_ostree {
tracing::debug!(
"ostree detected in {possible_ostree_dir}, assuming target is a deployment root and using {possible_physical_root}"
);
Expand Down Expand Up @@ -1759,7 +1770,7 @@ pub(crate) async fn install_to_filesystem(
tokio::task::spawn_blocking(move || remove_all_in_dir_no_xdev(&rootfs_fd, true))
.await??;
}
Some(ReplaceMode::Alongside) => clean_boot_directories(&rootfs_fd)?,
Some(ReplaceMode::Alongside) => clean_boot_directories(&rootfs_fd, is_already_ostree)?,
None => require_empty_rootdir(&rootfs_fd)?,
}

Expand Down