update: Skip bootloader update when no block devices back the root#1072
update: Skip bootloader update when no block devices back the root#1072cgwalters wants to merge 2 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the issue of bootupctl update failing in environments without block-backed root filesystems, such as ephemeral VMs using virtiofs. The core change to make get_devices() return an Option is well-implemented and propagated correctly to the callers, allowing them to gracefully skip the update. The addition of ci/ephemeral-test.sh is a great way to ensure this new behavior is tested. I have one main concern about the completeness of the fix regarding other components, which I've detailed in a specific comment.
| let Some(devices) = crate::blockdev::get_devices("/")? else { | ||
| return Ok(ValidationResult::Skip); | ||
| }; |
There was a problem hiding this comment.
This change correctly handles the case where no block devices are found for the EFI component's validation. It's highly likely that a similar change is needed for the validate implementation in bios.rs and any other Component implementations that call get_devices(). Without updating all call sites, bootupctl validate could still fail for other components (like BIOS) in the exact ephemeral environments this PR aims to support.
| run: sudo podman build --build-arg=base=quay.io/fedora/fedora-bootc:43 -t localhost/bootupd:latest -f Dockerfile . | ||
| - name: Smoke test (bcvk ephemeral) | ||
| timeout-minutes: 10 | ||
| run: bcvk ephemeral run-ssh localhost/bootupd:latest -- /usr/libexec/bootupd-tests/ephemeral-test.sh |
There was a problem hiding this comment.
Overall LGTM, maybe you should add sudo to run bcvk, as image localhost/bootupd:latest is built with sudo
|
Added one more check |
cgwalters
force-pushed
the
ephemeral-noupdate
branch
from
32258d5 to
218236d
Compare
|
This won't work on Live systems. Here is my branch with fix, mind checking it? Logs from live-iso: Logs from qemu: |
cgwalters
force-pushed
the
ephemeral-noupdate
branch
from
218236d to
3df3e2a
Compare
In environments without block-backed boot filesystems (virtiofs in bcvk ephemeral, NFS root, ISO boot, etc.) there is no on-disk bootloader to manage. Previously the update path would fail because list_dev_current_root() bailed when it could not find a block device from /boot or /sysroot. Assisted-by: OpenCode (Claude Opus 4) Signed-off-by: Colin Walters <walters@verbum.org>
On Fedora 43 dnf is dnf5, and the copr subcommand is provided by the dnf5-plugins package rather than dnf-plugins-core. Without it the ephemeral CI job fails with: Unknown argument "copr" for command "dnf5". Install dnf5-plugins with a fallback (|| true) so it's a no-op on CentOS Stream 9 where that package doesn't exist. Assisted-by: OpenCode (Claude Sonnet 4.5) Signed-off-by: Colin Walters <walters@verbum.org>
cgwalters
force-pushed
the
ephemeral-noupdate
branch
from
3df3e2a to
cdc2944
Compare
3 participants
Fix the problem that
bcvk ephemeral run quay.io/fedora/fedora-bootc:43shows a systemd error by default.