systemd: Order bootc-finalize-staged after /boot mount - #2488
Open
ericcurtin wants to merge 1 commit into
Open
ericcurtin wants to merge 1 commit into
ericcurtin wants to merge 1 commit into
Conversation
Collaborator
|
@ericcurtin Our AGENTS.md has this intentional stumbling block around the DCO, your PRs are welcome but please do add the signoff to reduce friction. (We may change things around this in the future, xref bootc-dev/infra#260 ) |
Collaborator
|
On the technical side, I think it may be a bit more complicated than that...we changed ostree to have a unit which holds a fd open, we might need to do the same here. But this is not wrong at least! |
The unit writes to /boot but only declared RequiresMountsFor=/sysroot. When /boot is a systemd-gpt-auto automount, there is no ordering between our ExecStop at shutdown and boot.mount/boot.automount, so lsblk in composefs-finalize-staged can block forever in autofs_expire_wait; systemd kills it after TimeoutStopSec=5m and the staged deployment is never finalized. Seen intermittently in CI as plan-44-shadow-fixup reboot timeouts. Mirror ostree-finalize-staged.service, which gained /boot here for the same reason (ostreedev/ostree#2544). Generated-by: AI Signed-off-by: Eric Curtin <eric.curtin@docker.com>
ericcurtin
force-pushed
the
finalize-staged-requires-boot
branch
from
September 22, 2026 22:15
ca7f513 to
5abe476
Compare
Johan-Liebert1
approved these changes
Sep 23, 2026
Member
|
I still the |
cgwalters-bot
added a commit
to cgwalters-bot/bootc
that referenced
this pull request
Sep 23, 2026
On composefs installs /boot is usually the ESP, automounted by systemd-gpt-auto-generator with a 2 minute idle timeout. If that expire begins after the shutdown transaction is queued, but before bootc-finalize-staged's ExecStop looks up /boot (lsblk statfs()es every mountpoint), the two deadlock: the lookup blocks in autofs_expire_wait until systemd unmounts boot.mount, and boot.mount is ordered to stop after finalize via local-fs.target. After 5+5 minutes finalize is killed, and the system boots the old deployment. That's the "guest reboot timeout" flake seen mostly in plan-44, where the gap between `bootc switch` and the reboot is about 130s in CI, on both systemd-boot and grub legs. If the expire instead completes before shutdown, systemd won't remount /boot, so finalize fails immediately with "Failed to open /boot: Host is down" on grub and the old deployment boots. That's the "expected exactly one testbootcgroup" plan-44 failure on composefs grub legs. See the analyses in https://gist.github.com/cgwalters-bot/37b1789c26fd0af22f6879d93aa0a4bd and https://gist.github.com/cgwalters-bot/793b7485f34dd1586127982e40351e04 ostree hit the same problem and fixed it in ostreedev/ostree@f3db79e7 ("finalize-staged: Ensure /boot automount doesn't expire") with ostree-finalize-staged-hold.service, which keeps a file descriptor for /boot open from the root mount namespace for as long as a deployment is staged. Port that: bootc-finalize-staged.service now pulls in bootc-finalize-staged-hold.service, which runs `bootc composefs-finalize-staged --hold`. It's ordered before the finalize unit, so at shutdown it's only stopped after finalization is done. A busy /boot never expires, so it can't race with shutdown. It would also make adding Requires=boot.mount to the finalize unit (as bootc-dev#2488 does) safe, since without the hold an idle expire stops the finalize unit while the system is still running. Deviations from ostree: the hold only opens /boot rather than loading the whole sysroot, since that's all that's needed and it keeps the hold independent of storage setup (which may enter a private mount namespace, where autofs wouldn't see the fd). For the same reason the hold unit only has RequiresMountsFor=/boot, not /sysroot. And the finalize unit keeps RequiresMountsFor=/sysroot without /boot: the stop ordering against boot.mount already comes via local-fs.target, and leaving out Requires=boot.mount means that if the hold somehow fails, we're no worse off than before. test-44 now checks that staging on composefs starts the hold unit. Tested manually by rebooting inside the race window: that hung 4/4 times on main and 0/7 with this change, and plan-44 passed 3/3. Assisted-by: AI
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
bootc-finalize-staged.servicewrites to/boot(the unit even says so) but only declaresRequiresMountsFor=/sysroot. With asystemd-gpt-auto-generator/bootautomount there is no ordering between ourExecStopat shutdown andboot.mount/boot.automount, andlsblkcan block forever inautofs_expire_wait:systemd kills it after
TimeoutStopSec=5m, the staged deployment is never finalized, and in CI this shows up asplan-44-shadow-fixup"guest reboot timeout" (3 legs in https://github.com/bootc-dev/bootc/actions/runs/35764348603, from #2476).Mirror
ostree-finalize-staged.service, which hasRequiresMountsFor=/sysroot /boot /etcsince ostreedev/ostree#2544 for the same hazard.Related: #2402.
Generated-by: AI
Diagnosed from the CI console/journal logs; I have not reproduced the deadlock locally, relying on CI to exercise it.