bootloader: Fix a few things related to systemd-boot installs#2191
bootloader: Fix a few things related to systemd-boot installs#2191cgwalters wants to merge 1 commit intobootc-dev:mainfrom
Conversation
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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.
| .env("KERNEL_INSTALL_CONF_ROOT", &esp_path_in_root) | |
| .env("KERNEL_INSTALL_CONF_ROOT", prepared_root.root_path().join(prepared_root.esp_subdir)) |
|
/packit test |
|
/packit build |
| 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. |
There was a problem hiding this comment.
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.
| "--esp-path", | ||
| esp_path_in_root.as_str(), | ||
| "--boot-path", | ||
| esp_path_in_root.as_str(), |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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()
But you're setting |
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>
See: osbuild/image-builder-cli#506
Right now most of our testing for composefs installs uses bcvk which uses
to-diskin an ephemeral VM. That masks some issues, like the fact thatbootctl installby default touches the ESP variables. Wire things up with--generic-imageso 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 recommendedpodman runinvocations to pass--read-only. But that reveals the next bug, which is that bootctl installs wants to write an/etc/kernel/entry-tokenwhich 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)