imagecache: fall back to legacy mount on pre-6.5 kernels - #504
imagecache: fall back to legacy mount on pre-6.5 kernels#504Mesut Oezdil (mesutoezdil) wants to merge 7 commits into
Conversation
The new mount API needs overlayfs lowerdir+, which only exists on Linux 6.5+. Probe it once at first mount and use plain mount(2) when missing, so older kernels do not fail every actor mount with EINVAL.
| // (Linux >= 6.5). The probe opens a throwaway fs context and tries the | ||
| // option against a directory that always exists; it never creates a | ||
| // superblock or touches the filesystem otherwise. | ||
| var lowerdirPlusSupported = sync.OnceValue(func() bool { |
There was a problem hiding this comment.
generally init() and friends are an anti-pattern especially interacting with the host from them ..., do we even need this probe versus just handling the error and falling back?
There was a problem hiding this comment.
thx for feedback, i removed the separate probe. now it just tries lowerdir+ on first layer directly, if kernel doesnt know it that call fails right away, then we fall back to legacy mount. No init() and no extra state.
added a test that forces this path with a fake option name so the fallback is actually covered now.
I think it’s better this way, it’s now responding to actual behaviour (rather than an assumption). PTAL
Drop the separate startup probe. mountOverlay now tries lowerdir+ on the first layer directly; if the kernel does not recognize it, that call fails immediately (before any path is touched), and we fall back to legacy mount(2) from there. Simpler and the fallback is now covered by a test that forces the failure.
Benjamin Elder (BenTheElder)
left a comment
Benjamin Elder (BenTheElder)
left a comment
There was a problem hiding this comment.
description is out of date.
agentic review pass (human reviewed before submission) comments inline
| return fmt.Errorf("while opening overlay fs context: %w", err) | ||
| } | ||
|
|
||
| if err := unix.FsconfigSetString(fsfd, lowerdirPlusOption, lowers[0]); err != nil { |
There was a problem hiding this comment.
🤖 should-fix 🟡 – Every error from this first call is read as "kernel too old", including errors that have nothing to do with the kernel version. On 6.5+ overlayfs resolves the path while parsing the parameter (ovl_parse_layer → kern_path), so a missing or unreadable layer directory fails right here with ENOENT. The operator then gets "kernel lacks overlayfs lowerdir+ (need >= 6.5)" followed by a second, unrelated failure from the legacy mount, and goes off chasing a kernel upgrade.
The kernel distinguishes the two cases for you: an unsupported parameter queues Unknown parameter 'lowerdir+' on the fs context log, which fsContextLog already reads — but this path closes the fd without draining it, discarding exactly the diagnostics #467 added to replace bare EINVALs. Gating the fallback on that message, or on the one-time probe against a known-good directory that the PR description describes, would keep real errors reporting themselves.
| // signal, rather than a separate startup probe: a bad lowerdir path would | ||
| // only surface later, at FsconfigCreate, so this first call failing is an | ||
| // unambiguous kernel-version signal. | ||
| func mountOverlay(mountpoint string, lowers []string, upper, work string) error { |
There was a problem hiding this comment.
🤖 nit 🟢 – lowers[0] below makes this panic on an empty slice, where the old range loop was safe. Only SetupBundleRootfs calls it and it returns early for zero layers, so nothing hits it today — but a one-line guard is cheap insurance for a privileged per-actor path.
mountOverlay was reading any error from the first lowerdir+ fsconfig call as a pre-6.5 kernel and falling back to legacy mount(2). A bad lowerdir path fails that same call (ENOENT) on 6.5+, so it was being misreported as a kernel version problem instead of the real error. Now the fallback only triggers when the kernel's fs context log says the parameter is unrecognized; any other error is returned as-is. Also guard mountOverlay against an empty lowers slice, unreachable today but no longer implicitly safe once lowers[0] is read this early.
2 participants
Fixes #500.
SetupBundleRootfs mounts overlay rootfs with the new mount API,
using fsconfig lowerdir+ per layer. That option only exists on
Linux 6.5+. On older kernels every actor mount fails with a bare
EINVAL, which looks like flakiness, not a kernel version problem.
Fix: mountOverlay tries lowerdir+ directly on the first layer. If
the kernel does not recognize it, that call fails immediately and
queues "Unknown parameter" on the fs context log; only that specific
diagnostic triggers the fallback to a plain mount(2) call built from
the same lowerdir list, so a bad lowerdir path (ENOENT) is reported
as-is instead of being misread as a kernel version problem. The
legacy path keeps the old ~34 layer cap and errors with a kernel
hint if the option string would go over the mount(2) page limit.
Tested with go test ./internal/imagecache/... on a real Linux
kernel (root, CAP_SYS_ADMIN, tmpfs upperdir), including a test that
forces the fallback with an unrecognized option name and one for
the option string builder.