composefs: Find GRUB's boot directory on a separate /boot when upgrading

[cgwalters-bot]

With GRUB and a separate /boot partition (as every image-builder disk has), composefs bootc switch and bootc upgrade fail with "Getting sorted Type1 boot entries: No such file or directory": staging built its paths from /sysroot, and /sysroot/boot is empty when the boot partition is mounted on /boot. bootc-dev#2440 fixed the same for status (and, through the booted storage, rollback, finalize and GC), so this keeps the path that get_boot_dir_for_grub() picked in Storage and derives the BLS and GRUB UKI staging paths from it, including the kernel paths in BLS entries, which must then be relative to the boot partition.

The second commit adds plan-57, which gives the test VM's disk a separate ext4 /boot (carved out of the ESP, like the XBOOTLDR test does) and then switches, upgrades and rolls back. bcvk's disks have no separate /boot, and a disk installed to a loop device inside the VM can't be booted, so repartitioning was the way to cover the booted paths.

Tested on a 16-core devspace (composefs, GRUB, ext4, BLS): just unit-tests, just validate, and plans 56 (now renumbered 57), 24 and 36 passed. Without the fix, the new plan fails with the error above. The fix also passed switch + reboot + upgrade + reboot in rhel-bootc-examples' composefs e2e test on an image-builder disk: https://gist.github.com/cgwalters-bot/04b3e5503547a054ebcdf227238eaec1

Related: bootc-dev#2440

Generated-by: https://github.com/cgwalters/#llms

---

Review draft in cgwalters-forge, not upstream yet. This section is removed when the PR is opened upstream.

• Upstream: bootc-dev/bootc, base main
• Board item: PVTI_lAHOAQ_SPs4Bj2Gizg83mOA
• Fork CI: off; the devspace testing described above is this PR's CI, and upstream CI runs once it is opened there

To review:

• Approve, or comment /promote on a line of its own, to open it upstream, ready for review. Either covers only the commits pushed so far.
• If upstream requires DCO, approving also signs off: promote adds Signed-off-by: Colin Walters <walters@verbum.org> to the commits lacking it (the bot's and yours; anyone else's only if you ask), with you as committer.
• Add a /draft line (in the same comment or before) to open it upstream as a draft (/ready undoes that).
• Close to drop it.
• Edit the title and description freely: they become the upstream PR's. Review comments are squashed into the commits they concern, with a reply here.

[cgwalters-bot]

Review guide for head f2bc24f740: where to look closely and what is safe to skim. It is advice from the bot's reviewer and doesn't replace reading the diff; the review app walks it.

Upgrade and switch on composefs+GRUB used to build the BLS and GRUB-UKI paths from /sysroot, which is empty when /boot is a separate partition. The fix stores the path get_boot_dir_for_grub() picked in Storage and derives every staging path, plus the kernel prefix in new BLS entries ("/" vs "/boot"), from it. The risk is concentrated in two decisions that now affect what gets written, not just what status reads: which directory holds GRUB's entries (classify), and which prefix the kernel paths get (grub_bls_abs_entries_path). A wrong answer in either writes entries GRUB can't use, and you only find out at the next boot. The GRUB+UKI path changed too but has no test. The new tmt test rebuilds the VM's disk layout by hand; read it for what it proves and what it assumes about install.

Hotspots

1. risky · logic — crates/lib/src/bootc_composefs/boot.rs:336-354 (36f1d52): Every new BLS entry's linux/initrd prefix comes from this: "/" if the boot dir is a mount point, else "/boot". That's right for a whole partition on /boot, but a bind mount or btrfs subvolume on /boot is also a mount point while the kernels sit under /boot on that filesystem, so entries get "/bootc_composefs-..." and GRUB can't find the kernel at the next boot. is_mountpoint() returning None also silently means "/boot".
2. look-closely · logic — crates/lib/src/store/mod.rs:394-411 (36f1d52): This decision used to pick only where status and GC read. Now it also picks where upgrade and switch write kernels and entries. A boot partition with no grub2 dir, or an ESP on /boot that really holds GRUB's config (ESP-as-/boot layouts, pinned by the ESP+grub-dir=PhysicalRoot test case), falls back to /sysroot/boot, and staging then writes entries GRUB never reads without any error.
3. note · api — crates/lib/src/store/mod.rs:522-540 (36f1d52): grub_boot_path is set only here, for booted composefs with GRUB; Storage::new and the ostree branch leave it None. Any Upgrade setup reached with such a Storage now fails with "Grub boot path not found" where it used to fall back to /sysroot. Check that update.rs's callers only ever pass the booted storage.
4. note · logic — crates/lib/src/bootc_composefs/boot.rs:947-952 (36f1d52): This is where the original bug showed: the shared kernel dir is found through storage.boot_dir (an fd), but its path prefix comes from abs_entries_path (a path check). They agree only because both now derive from one GrubBootLocation. The v2 upgrade in plan-57 (back to the base initramfs) is the only coverage of this branch.
5. look-closely · test-gap — crates/lib/src/bootc_composefs/boot.rs:1619-1655 (36f1d52): GRUB+UKI with a separate /boot now reads and writes /boot/grub2/user.cfg(.staged), but plan-57 skips UKI (fixme_skip_if_uki, and first_boot skips non-BLS), so nothing tests this path. If no menu entries are parsed at the new location, entries[0] panics rather than returning an error.
6. look-closely · test-gap — tmt/tests/booted/test-composefs-separate-boot.nu:151-166 (f2bc24f): The test writes the entries' relative kernel paths and the systemd.mount-extra karg itself, imitating install. So it checks upgrade against its own reconstruction of install's output, not install's real output (boot_mount_spec's source and options format). Install with a separate /boot stays untested.
7. note · test-gap — tmt/tests/booted/test-composefs-separate-boot.nu:72-100 (f2bc24f): These assertions are what catches a regression: /sysroot/boot stays empty and the kernel paths are relative to the partition. referenced_kernel_dirs reads only linux lines, so a wrong initrd prefix, or GC dropping an initramfs dir that only an initrd line refers to, would pass.
8. note · error-handling — tmt/tests/booted/test-composefs-separate-boot.nu:114-141 (f2bc24f): This repartitions and reformats the running VM's ESP. A failure between the sfdisk and the copy back (a busy umount, partx) leaves a VM that can't boot, and the plan then fails by timeout instead of with an error. The 512-byte sector size is hard-coded where sfdisk reports sectorsize.

Safe to skim

• tmt/plans/integration.fmf: generated from the test's header
• tmt/tests/tests.fmf: generated from the test's header
• crates/lib/src/bootc_composefs/boot.rs:777-853: mechanical: drops root_path from the setup tuple
• crates/lib/src/bootc_composefs/boot.rs:1778-1884: mechanical: drops root_path from the UKI setup tuple
• crates/lib/src/store/mod.rs:637-646: doc comment and new field

[cgwalters-bot]

Review guide for head f2bc24f740: where to look closely and what is safe to skim. It is advice from the bot's reviewer and doesn't replace reading the diff; the review app walks it.

Upgrade and switch on composefs+GRUB used to build the BLS and GRUB-UKI paths from /sysroot, which is empty when /boot is a separate partition. The fix stores the path get_boot_dir_for_grub() picked in Storage and derives every staging path, plus the kernel prefix in new BLS entries ("/" vs "/boot"), from it. The risk is concentrated in two decisions that now affect what gets written, not just what status reads: which directory holds GRUB's entries (classify), and which prefix the kernel paths get (grub_bls_abs_entries_path). A wrong answer in either writes entries GRUB can't use, and you only find out at the next boot. The GRUB+UKI path changed too but has no test. The new tmt test rebuilds the VM's disk layout by hand; read it for what it proves and what it assumes about install.

Hotspots

1. risky · logic — crates/lib/src/bootc_composefs/boot.rs:336-354 (36f1d52): Every new BLS entry's linux/initrd prefix comes from this: "/" if the boot dir is a mount point, else "/boot". That's right for a whole partition on /boot, but a bind mount or btrfs subvolume on /boot is also a mount point while the kernels sit under /boot on that filesystem, so entries get "/bootc_composefs-..." and GRUB can't find the kernel at the next boot. (The fallback to "/boot" when the mount check is inconclusive predates this PR.)
2. look-closely · logic — crates/lib/src/store/mod.rs:394-411 (36f1d52): Finalize, delete and the BLS compat path already wrote through this decision; now upgrade and switch stage kernels and entries through it too. A boot partition with no grub2 dir, or an ESP on /boot that really holds GRUB's config (ESP-as-/boot layouts, pinned by the ESP+grub-dir=PhysicalRoot test case), falls back to /sysroot/boot, and staging then writes entries GRUB never reads without any error.
3. note · api — crates/lib/src/store/mod.rs:522-540 (36f1d52): grub_boot_path is set only here, for booted composefs with GRUB; Storage::new and the ostree branch leave it None. Any Upgrade setup reached with such a Storage now fails with "Grub boot path not found" where it used to fall back to /sysroot. Check that update.rs's callers only ever pass the booted storage.
4. note · logic — crates/lib/src/bootc_composefs/boot.rs:947-952 (36f1d52): This is where the original bug showed: the shared kernel dir is found through storage.boot_dir (an fd), but its path prefix comes from abs_entries_path (a path check). They agree only because both now derive from one GrubBootLocation. The v2 upgrade in plan-57 (back to the base initramfs) is the only coverage of this branch.
5. look-closely · test-gap — crates/lib/src/bootc_composefs/boot.rs:1619-1655 (36f1d52): GRUB+UKI with a separate /boot now reads and writes /boot/grub2/user.cfg(.staged), but plan-57 skips UKI (fixme_skip_if_uki, and first_boot skips non-BLS), so nothing tests this path. If no menu entries are parsed at the new location, entries[0] panics rather than returning an error.
6. look-closely · test-gap — tmt/tests/booted/test-composefs-separate-boot.nu:151-166 (f2bc24f): The test writes the entries' relative kernel paths and the systemd.mount-extra karg itself, imitating install. So it checks upgrade against its own reconstruction of install's output, not install's real output (boot_mount_spec's source and options format). Install with a separate /boot stays untested.
7. note · test-gap — tmt/tests/booted/test-composefs-separate-boot.nu:72-100 (f2bc24f): These assertions are what catches a regression: /sysroot/boot stays empty and the kernel paths are relative to the partition. referenced_kernel_dirs reads only linux lines, so a wrong initrd prefix, or GC dropping an initramfs dir that only an initrd line refers to, would pass.
8. note · error-handling — tmt/tests/booted/test-composefs-separate-boot.nu:114-141 (f2bc24f): This repartitions and reformats the running VM's ESP. A failure between the sfdisk and the copy back (a busy umount, partx) leaves a VM that can't boot, and the plan then fails by timeout instead of with an error. The 512-byte sector size is hard-coded where sfdisk reports sectorsize.

Safe to skim

• tmt/plans/integration.fmf: generated from the test's header
• tmt/tests/tests.fmf: generated from the test's header
• crates/lib/src/bootc_composefs/boot.rs:777-853: mechanical: drops root_path from the setup tuple
• crates/lib/src/bootc_composefs/boot.rs:1778-1884: mechanical: drops root_path from the UKI setup tuple
• crates/lib/src/store/mod.rs:637-646: doc comment and new field

[cgwalters-bot]

Signed off 2 commit(s) with Signed-off-by: Colin Walters <walters@verbum.org> for bootc-dev/bootc's DCO check, on cgwalters's approval #29 (review) of f2bc24f740c3. Same trees; the head is now 58ac50908d39.

[cgwalters-bot]

Opened upstream as bootc-dev#2516. Closing this review draft.

[cgwalters-bot]

Signed off 2 commit(s) with Signed-off-by: Colin Walters <walters@verbum.org> for bootc-dev/bootc's DCO check, on cgwalters's approval bootc-dev#2516 (review) of 4e10ca5cee5e. Same trees; the head is now fa5e9c32caef.

[cgwalters-bot]

Signed off 1 commit(s) with Signed-off-by: Colin Walters <walters@verbum.org> for bootc-dev/bootc's DCO check, on cgwalters's approval bootc-dev#2516 (review) of 0f565f9a99e5. Same trees; the head is now d26a2cdde2cc.