Skip to content

bootloader: Fix a few things related to systemd-boot installs#2191

Open
cgwalters wants to merge 1 commit intobootc-dev:mainfrom
cgwalters:bootctl-root
Open

bootloader: Fix a few things related to systemd-boot installs#2191
cgwalters wants to merge 1 commit intobootc-dev:mainfrom
cgwalters:bootctl-root

Conversation

@cgwalters
Copy link
Copy Markdown
Collaborator

See: osbuild/image-builder-cli#506

Right now most of our testing for composefs installs uses bcvk which uses to-disk in an ephemeral VM. That masks some issues, like the fact that bootctl install by default touches the ESP variables. Wire things up with --generic-image so it behaves the same as the bootupd backend.

Now, the next problem is that osbuild actually makes / readonly (which is great!) and in fact we should change our recommended podman run invocations to pass --read-only. But that reveals the next bug, which is that bootctl installs wants to write an /etc/kernel/entry-token which we basically don't use. Redirect those writes to a tempdir for now, though we should probably try to get upstream patches for that.

Also set SYSTEMD_RELAX_ESP_CHECKS=1 so bootctl skips the partition-type GUID check for the ESP - we don't want it to e.g. try to look up things in the udev database, which might not be available.

Assisted-by: OpenCode (claude-sonnet-4-6)

@bootc-bot bootc-bot Bot requested a review from jeckersb May 8, 2026 17:56
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the MountedImageRoot abstraction to manage the mounting of composefs images and the EFI System Partition (ESP) within a temporary directory. This change allows bootctl to be invoked with the --root flag, enabling it to read configuration from the target OS. Feedback identifies a potential issue where the KERNEL_INSTALL_CONF_ROOT environment variable might resolve to the host's root instead of the temporary mount; a suggestion is provided to use the host-absolute path to ensure the kernel entry token is written to the correct location.

.env("SYSTEMD_RELAX_ESP_CHECKS", "1")
// Redirect the entry-token write into the ESP (writable FAT) rather
// than the EROFS composefs root.
.env("KERNEL_INSTALL_CONF_ROOT", &esp_path_in_root)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The KERNEL_INSTALL_CONF_ROOT environment variable is used by bootctl to determine where to read and write the kernel entry token. When bootctl is invoked with --root, it prefixes paths provided via CLI arguments, but it typically does not prefix absolute paths provided via environment variables.

Since esp_path_in_root is an absolute path (e.g., /efi), bootctl will likely attempt to access the host's /efi directory instead of the ESP mounted within the temporary root. To ensure the entry token is correctly redirected to the writable ESP, you should provide the absolute path on the host where the ESP is currently mounted.

Suggested change
.env("KERNEL_INSTALL_CONF_ROOT", &esp_path_in_root)
.env("KERNEL_INSTALL_CONF_ROOT", prepared_root.root_path().join(prepared_root.esp_subdir))

@cgwalters
Copy link
Copy Markdown
Collaborator Author

/packit test

@cgwalters
Copy link
Copy Markdown
Collaborator Author

/packit build

Comment thread crates/lib/src/bootc_composefs/boot.rs Outdated
let composefs = TempMount::mount_fd(composefs_mnt_fd)
.context("Attaching composefs image to temporary directory")?;

// Per BLS: prefer /efi when that directory exists, /boot otherwise.
Copy link
Copy Markdown

@supakeen supakeen May 8, 2026

Choose a reason for hiding this comment

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

This comment isn't entirely clear I feel. The BLS prefers the ESP to be mounted to /boot if there's no XBOOTLDR. Otherwise it prefers the ESP mounted to /efi when there XBOOTLDR at /boot.

It is recommended to mount $BOOT (either XBOOTLDR or the ESP) to /boot/. If both XBOOTLDR and the ESP are present, the ESP should be mounted to /efi/.

For finding the ESP the below is correct (though it omits that /boot/efi is also checked).

Bit of a nitpick so feel free to ignore.

Comment thread crates/lib/src/bootloader.rs Outdated
"--esp-path",
esp_path_in_root.as_str(),
"--boot-path",
esp_path_in_root.as_str(),
Copy link
Copy Markdown

@supakeen supakeen May 8, 2026

Choose a reason for hiding this comment

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

This should be the XBOOTLDR path if it exists, otherwise it should be omitted?

--boot-path=
   Path to the Extended Boot Loader partition, as defined in the UAPI.1 Boot Loader Specification[1]. If not specified, /boot/ is
   checked. It is recommended to mount the Extended Boot Loader partition to /boot/, if possible.

Copy link
Copy Markdown

@supakeen supakeen left a comment

Choose a reason for hiding this comment

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

One nitpick, one thing I think should be changed.

We used the following redirect during our testing and that made things work, it's largely similar to the new arguments passed here:

Note that we had to set the --entry-token as it couldn't find one in /etc/kernel/entry-token and couldn't find /etc/os-release. The behavior is as follows:

If set to auto (the default), the /etc/kernel/entry-token file will be read if it exists, and the stored value used. Otherwise, if the
local machine ID is initialized it is used. Otherwise, IMAGE_ID= from os-release will be used, if set. Otherwise, ID= from os-release
will be used, if s
#!/usr/bin/env python3
import subprocess as sp
import sys


def main():
    args = sys.argv[1:]
    for idx, arg in enumerate(args):
        if arg == "--esp-path":
            args[idx+1] = "/boot/efi"
    args.extend(["--root", "/run/osbuild/mounts"])
    args.extend(["--entry-token", "literal:simonos"])

    cmd = ["/usr/bin/bootctl.orig"] + args
    print(" ".join(cmd), file=sys.stderr)
    sp.run(cmd, check=True)


if __name__ == "__main__":
    main()

@cgwalters
Copy link
Copy Markdown
Collaborator Author

Note that we had to set the --entry-token as it couldn't find one in /etc/kernel/entry-token and couldn't find /etc/os-release. The behavior is as follows:

But you're setting --root there I believe to the physical root, whereas this PR is setting it to the composefs image root.

@supakeen
Copy link
Copy Markdown

supakeen commented May 8, 2026

Note that we had to set the --entry-token as it couldn't find one in /etc/kernel/entry-token and couldn't find /etc/os-release. The behavior is as follows:

But you're setting --root there I believe to the physical root, whereas this PR is setting it to the composefs image root.

That makes sense 🙂.

See: osbuild/image-builder-cli#506

Right now most of our testing for composefs installs uses bcvk which
uses `to-disk` in an ephemeral VM. That masks some issues, like the
fact that `bootctl install` by default touches the ESP variables.
Wire things up with `--generic-image` so it behaves the same as
the bootupd backend.

Now, the next problem is that osbuild actually makes `/` readonly (which
is great!) and in fact we should change our recommended `podman run`
invocations to pass `--read-only`. But that reveals the next bug,
which is that bootctl installs wants to write an `/etc/kernel/entry-token`
which we basically don't use. Redirect those writes to a tempdir
for now, though we should probably try to get upstream patches for that.

Also set SYSTEMD_RELAX_ESP_CHECKS=1 so bootctl skips the partition-type
GUID check for the ESP - we don't want it to e.g. try to look up
things in the udev database, which might not be available.

Assisted-by: OpenCode (claude-sonnet-4-6)
Signed-off-by: Colin Walters <walters@verbum.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/merge Run full CI suite (all OSes) — equivalent to merge queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants