efi: Support managing non-EFI components at the ESP root#1073
efi: Support managing non-EFI components at the ESP root#1073martinezjavier wants to merge 5 commits intocoreos:mainfrom
Conversation
|
Hi @martinezjavier. Thanks for your PR. I'm waiting for a coreos member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
0badf42 to
b3432cd
Compare
There was a problem hiding this comment.
Code Review
This pull request extends bootupd to manage non-EFI components at the ESP root, which is useful for platforms like Raspberry Pi. It also fixes issues with version selection and RPM name handling. The changes are logical and well-tested. I've identified a potential bug in version comparison and a couple of areas for code quality improvement to enhance maintainability.
681bcc5 to
66ab5cb
Compare
|
I had a quick look at that (not an expert on bootupd, so not a thorough review), but it looks good, and exactly what we need for automotive (for rpi support). |
a458fc7 to
db97662
Compare
I've addressed these since the code review bot was correct about both issues. |
db97662 to
8ed56bf
Compare
|
/ok-to-test |
8ed56bf to
d3e0ee4
Compare
bootupd stores update payloads under /usr/lib/efi/ in a <component>/<evr>/ directory structure. When building the file tree used to compute an update diff, it strips the <component>/<evr>/ prefix to obtain the destination path on the ESP. For example, /usr/lib/efi/shim/15.8/EFI/BOOT/shimaa64.efi is translated to the EFI/BOOT/shimaa64.efi destination path. However, the diff records the source paths directly and re-derives the destination at apply time, coupling the diff representation to the source directory layout. When multiple versions of a component are present (e.g. both shim/15.8/ and shim/15.9/), several source files map to the same destination path. Since the iteration order is non-deterministic, the file tree may end up with stale metadata from the wrong version. This causes updates to be silently skipped when the old hash wins, and prevents detection of files removed in the newer version. To avoid this issue, let's record a source_map in the diff that maps each destination path back to its original source path. The diff now always works in terms of destination paths, and only consults the map when it needs to locate the source file for copying. This prepares for building per-version file trees in a following commit, making the update payloads iteration to be deterministic. Assisted-by: Cursor (Claude Opus 4)
The atomic swap operations on the ESP are performed in arbitrary iteration order. If a system loses power mid-update, the ESP may be left with a mix of old and new content in an unpredictable combination, making failures harder to diagnose and reproduce. Sort the swap operations in a stable order: EFI vendor directories first, then other directories at the ESP root, then individual root-level files. This ensures EFI boot entries are updated before auxiliary content such as firmwares, minimizing the window where the ESP is in an inconsistent state. Also create parent directories before the swap so that new directory trees (that did not previously exist on the ESP) can be added. Assisted-by: Cursor (Claude Opus 4)
The /usr/lib/efi/ layout stores component files under a two-level prefix <component>/<evr>/ (e.g. shim/15.8/EFI/fedora/shimaa64.efi). To compare this tree against the flat ESP layout, the prefix must be stripped so that keys match the destination paths (EFI/fedora/shimaa64.efi). Add a file tree constructor that walks a set of <component>/<evr>/ subdirs, strips that prefix from each path, and records the original source path. This can be used by the source_map mechanism when applying diffs, to only include the desired version when multiple versions of a component coexist, avoiding the non-deterministic version mixing that currently exists. Assisted-by: Cursor (Claude Opus 4)
bootupd currently assumes that all managed content lives under the EFI/ subdirectory of the ESP. Platforms (such as the Raspberry Pi) place its firmware blobs (device-tree files, GPU firmware, config.txt) directly in the ESP root, outside EFI/. These files cannot be managed by bootupd today. Widen the scope of all ESP operations (install, adopt, update, validate) to work on the ESP root rather than just the EFI/ subdirectory. Now, the file paths themselves determine their destination in the ESP. The keys starting with EFI/ are written under the EFI/ subdirectory as before, while all other files are placed directly at the ESP root. Assisted-by: Cursor (Claude Opus 4)
On OSTree-based deployments, the /boot/efi/ files are stored under the usr/lib/ostree-boot/efi/ directory, and must transferred to usr/lib/efi/ before bootupd can manage them. The transfer currently only walks the EFI/ sub-directory, so root-level firmware files are never picked up. It also splits the RPM package name on the first hyphen to derive the component directory name, which truncates names that contain hyphens (e.g. "bcm2711-firmware" becomes "bcm2711"). Walk the entire usr/lib/ostree-boot/efi/ tree so that root-level files are discovered alongside EFI boot entries, and use the full RPM package name as the component directory name. Closes: coreos#959 Assisted-by: Cursor (Claude Opus 4)
d3e0ee4 to
b1de2cb
Compare
Platforms like the Raspberry Pi place firmware blobs (device-tree files, GPU firmware, config.txt) directly at the ESP root. But currently,
bootupdcan only manage the files in theEFIsub-directory of the ESP.This series extends
bootupdto manage those files alongside traditional EFI bootloaders.It also fixes two pre-existing issues: non-deterministic version mixing when multiple component versions coexist under
/usr/lib/efi/, and truncation of hyphenated RPM names (e.g.bcm2711-firmwarebecomesbcm2711) during theostree-boottransfer.The commits were developed assisted by the Cursor IDE and Claude Opus 4 model. But all the code and commit messages were manually reviewed and edited by me, ensured that the changes are properly separated in different commits, the code is kept bisectable, etc.
AI assistance is noted via
Assisted-by:tags per the bootc agent skills review guidelines.End-to-end was tested on a Raspberry Pi 4 using Fedora CoreOS 43, with the
bcm2711-firmwareinstalled on top of the initial OSTree deployment. The complete flow has been verified:generate-update-metadata,adopt-and-update,validate, and theupdatepath with a simulatedbcm2711-firmwarecomponent version bump.