composefs/status: Detect BLS layout on non-EFI systems - #2376
composefs/status: Detect BLS layout on non-EFI systems#2376dustinkirkland wants to merge 1 commit into
Conversation
|
One thing I want to flag proactively for the reviewer: The BLS probe here reads But it introduces an implicit dependency on that host mount being let bls_present = esp_mount.fd.metadata("loader/entries")
.map(|m| m.is_dir())
.unwrap_or(false);That's namespace-safe and doesn't rely on external mount ordering. Happy to switch to the ESP-mount version if you'd prefer that Assisted-by: Claude (Opus 4) |
bb650ef to
2e421b5
Compare
|
Small tidy-up force-pushed as |
Johan-Liebert1
left a comment
There was a problem hiding this comment.
The logic looks good, but tests for grub-cc failing
|
@Johan-Liebert1 thanks for the review — you're right, root cause diagnosed. My refactor accidentally changed the caching semantics: the original Reworking now to preserve the pre-existing "don't cache on EFI" behavior; the non-EFI BLS-probe path (the actual point of the PR) will still cache since the FS-probe result is stable. Will force-push shortly. Assisted-by: Claude (Opus 4) |
`get_bootloader()` unconditionally returns `Bootloader::Grub` when
there are no EFI variables to inspect (`SystemNotUEFI` /
`MissingVar`). That's wrong for many non-EFI setups that use the Boot
Loader Specification Type 1 entry layout at `/boot/loader/entries/`
without any EFI vars to advertise it — Raspberry Pi 4/5 with direct-
kernel boot from Pi firmware, U-Boot with the extlinux/BLS loader,
coreboot chaining to a bare kernel, and various ARM/embedded boards.
When bootc misclassifies these as `Bootloader::Grub` →
`BootloaderKind::GRUBClassic`, `storage::new` sets `boot_dir =
physical_root.open_dir("boot")` = `/sysroot/boot/`. On systems where
`/boot` is a separate partition (the ESP mounted at `/boot` via the
`systemd.mount-extra=UUID=<ESP>:/boot:auto:ro` cmdline that `bootc
install to-filesystem` itself writes), `/sysroot/boot/` is empty.
Every subsequent code path that reads BLS entries via
`boot_dir.read_dir("loader/entries")` then `ENOENT`s — including the
idempotent `prepend_custom_prefix()` backwards-compat migration
called from `storage::new` itself, which is why `bootc status`,
`bootc upgrade`, and `bootc switch` all fail at storage init.
This bug was masked before bootc-dev#2356 by an EBUSY on the pre-mounted ESP.
With that fixed, execution now reaches `prepend_custom_prefix`, which
is where the wrong `boot_dir` gets used.
Fix: when there are no EFI vars, stat `/boot/loader/entries`. If it
is a directory, treat the bootloader as BLS-compatible; otherwise
fall back to `Bootloader::Grub` as before. The probe is a single
`stat(2)` and the else-branch preserves prior behaviour on real
grub-classic systems (where `/boot/grub2/` exists but
`/boot/loader/entries/` does not).
Split the inner match into a pure `classify_bootloader(efi_result,
bls_present) -> Result<Bootloader>` helper per REVIEW_RUST.md
"separate parsing from I/O" guidance, and add a table-driven unit
test covering both prior branches and both new branches.
Preserve the pre-existing "don't cache on EFI" behavior of
`get_bootloader()`: the old code had an early-return in the
`Ok(loader)` branch that bypassed the `OnceLock` cache, and the
grub-cc TMT plans observed bootloader-info changes over a run
(discovered via `is_composefs` → `bootc status --json` in
`tap.nu` after v1 of this PR unified the caching path). The new
code caches only when the classification came from the FS probe
(non-EFI, filesystem-stable state).
Verified on aarch64 with `bootc` built from this branch: before the
fix, `bootc status` errored at "Prepending custom prefix to EFI and
BLS entries: Getting sorted Type1 boot entries: No such file or
directory (os error 2)"; after, it returns a healthy `BootcHost`
report with `bootType: Bls`, and `bootc switch --transport=registry`
proceeds normally.
Assisted-by: Claude (Opus 4)
Signed-off-by: Dustin Kirkland <dustin.kirkland@chainguard.dev>
Closes: bootc-dev#2375
2e421b5 to
34ea3cb
Compare
|
Force-pushed as Fix preserves the pre-existing "don't cache on EFI" semantics: Diff shape unchanged ( Assisted-by: Claude (Opus 4) |
|
The Grub CC failures are unrelated to this PR. See #2378 I see Claude's hallucinating again :) |
|
Update: the 4 CI failures on this PR (
Compared to the last fully-green PR (#2363, merged 2026-08-07), something between then and 2026-08-09 broke these tests on main. Happy to help track that down separately if useful — but this PR isn't the trigger. Live verification of this PR on aarch64 (Raspberry Pi 5, direct-kernel boot from Pi firmware, no UEFI at runtime):
Reworked commit at Assisted-by: Claude (Opus 4) |
| Err(EfiError::SystemNotUEFI) | Err(EfiError::MissingVar) => { | ||
| if bls_entries_dir_present { | ||
| tracing::debug!( | ||
| "No EFI vars but {BLS_ENTRIES_DIR} is a directory; \ | ||
| treating bootloader as BLS-compatible (systemd-boot)" | ||
| ); | ||
| Ok(Bootloader::Systemd) |
There was a problem hiding this comment.
Does this mean that a legacy BIOS system that HAS a BLS directory would default to systemd-boot? Even though it could still be grub?
|
Ah, ninja'd — hadn't seen #2378 when I posted the timeline. Same conclusion, thanks for tracking it separately. |
Fixes #2375.
get_bootloader()unconditionally returnsBootloader::Grubwhen thereare no EFI variables to inspect. That's wrong for many non-EFI setups
that use the BLS Type 1 entry layout at
/boot/loader/entries/regardless — Raspberry Pi 4/5 with direct-kernel boot from Pi firmware,
U-Boot with the extlinux/BLS loader, coreboot chaining to a bare kernel,
various ARM/embedded boards.
When bootc misclassifies these as
GRUBClassic,storage::newsetsboot_dir = /sysroot/boot/, which is empty on systems where/bootisa separate ESP mounted at
/boot— exactly whatbootc install to-filesystemwrites. Every BLS-reading code path then ENOENTs,including the idempotent
prepend_custom_prefix()migration instorage::newitself, sobootc status,bootc upgrade, andbootc switchall fail at storage init on affected systems.Fix: when there are no EFI vars,
stat /boot/loader/entries/. If itis a directory, treat the bootloader as BLS-compatible; otherwise fall
back to
Bootloader::Grubas before. The else-branch preserves priorbehaviour on real grub-classic systems.
Structure: split the inner match into a pure
classify_bootloader(efi_result, bls_present) -> Result<Bootloader>helper per REVIEW_RUST.md "separate parsing from I/O" guidance. Added a
table-driven unit test with 7 cases covering both prior branches
(systemd-boot, GRUB CC, generic GRUB via
EFI_LOADER_INFO) and all fourcombinations of
{SystemNotUEFI, MissingVar} × {BLS present, BLS absent}, plus a propagation test for otherEfiErrorvariants.Verification: aarch64 Raspberry Pi 5, bootc 1.16.7, composefs
install with BLS layout on FAT ESP, direct-kernel boot from Pi firmware
(no UEFI at runtime).
Before:
After:
bootc switch --transport=registry <target>also proceeds normally onthe same system after this change.
History note
This bug was masked before #2356 by an EBUSY on the pre-mounted ESP.
With that fixed, execution now reaches
prepend_custom_prefix(), whichis where the wrong
boot_dirgets used.Assisted-by: Claude (Opus 4)
I authored issue #2375, directed the design (classify-vs-io split,
table-driven test coverage), and did the live before/after verification
on a real aarch64 bootc-composefs host. I have prior bootc contributions
(#2356) but am not a Rust regular — happy to iterate on style/naming.