storage, overlay: stop using symlinks for new layers

[giuseppe]

The overlay driver maintained a l/ directory of symlinks solely to shorten mount option strings below the kernel's page-size limit.
This is unnecessary since mountOverlayFrom() already handles long mount options via fd-based mounting.

New layers now store relative diff paths (e.g. "layer-id/diff") directly in the lower file instead of short symlink references.

Old-format layers with l/ symlinks continue to work.

[giuseppe]

@mtrmac I believe you are going to like this PR :)

[mtrmac]

At a glance, yes I do :)

[Luap99]

I wonder, do we have a test case that mounts a 500 layer overlay? If not should we have one to ensure mounting always works?

[giuseppe]

Mounting with the new code gives always shorter paths since we use the fd number as the full path instead of a symlink that is still an absolute path. I think this code was left there just for historical reasons and there is no case where it is better

[kolyshkin]

Maybe it makes sense to add comments marking the remaining "legacy" code paths (dealing with l/ symlinks) as such, so they can be removed in the future. Otherwise it's hard to grok for future readers as it is -- we don't create any links but we handle them.

One other thing is, I'm not sure if we need the entire fork/exec open lower fds. This is a further optimization but maybe we can just do it in the same process? I mean, the fork/exec was done because of chdir, but it looks like we no longer need it (yet it's left there, and that's another part of "legacy" code I guess). So, maybe we should call mountOverlayFrom when working with legacy l/ symlinks, and do a via-fds in the same process?

As for the tests, I think this is covered by existing @test "allow storing images with more than 127 layers" which creates 300 layers (although I haven't checked it if actually crosses the pagesize threshold).

[kolyshkin]

BTW this removes the only user of the second value returned from dir2, so maybe it makes sense to drop it (perhaps in a separate subsequent commit).

[giuseppe]

thanks for spotting it, fixed now!

[kolyshkin]

I wonder, do we have a test case that mounts a 500 layer overlay? If not should we have one to ensure mounting always works?

We do have one for 300 layers, see storage/tests/layers.bats, although as I said I haven't checked we're actually crossing the pagesize threshold with it.

[TomSweeneyRedHat]

@giuseppe do you spin up a PR in Podman to see how this does there?
Other than @kolyshkin 's comments, LGTM, and happy green test buttons.

[giuseppe]

@giuseppe do you spin up a PR in Podman to see how this does there?
Other than @kolyshkin 's comments, LGTM, and happy green test buttons.

fixed, and opened a test PR here: containers/podman#28164

[TomSweeneyRedHat]

I restarted the failing test here, it looked to be a time out flake.
The Podman PR, however, is very red.

[giuseppe]

I restarted the failing test here, it looked to be a time out flake.
The Podman PR, however, is very red.

that failure is expected because we are not rebuilding Buildah. Buildah fails to find the symlink since we are not creating it anymore.

[giuseppe]

@Luap99 @mtrmac @kolyshkin PTAL

[Luap99]

that failure is expected because we are not rebuilding Buildah. Buildah fails to find the symlink since we are not creating it anymore.

Well how do you expect us to get CI passing then? We should not merge this if it breaks CI tests.
Also we cannot really control the update timings of podman/buildah/skopeo so if mixing them breaks them completely we have a big problem for the distributions shipping these tools. We can only control fedora/RHEL.

i.e. this test is using skopeo for example

[+0054s] not ok 39 [010] podman pull image with additional store in 1071ms
         # (in test file test/system/[010-images.bats, line 395](https://github.com/containers/podman/blob/2839aa038a596ef18fcf60a9731517fa8a1a6dc5/test/system/010-images.bats#L395))
         #   `skopeo copy containers-storage:$IMAGE \' failed
         #
<+     > # # podman  image exists quay.io/libpod/testimage:20241011
         # Getting image source signatures
         # Copying blob sha256:5f70bf18a086007016e948b04aed3b82103a36bea41755b6cddfaf10ace3c6ef
         # Copying blob sha256:b66a884aaf08f1c410c61682a7072d68a0d837ba8dc16ff13b9509bdceb32fd2
         # time="2026-02-27T11:30:24-06:00" level=fatal msg="reading blob sha256:5f70bf18a086007016e948b04aed3b82103a36bea41755b6cddfaf10ace3c6ef: creating file-getter: readlink /var/lib/containers/storage/overlay/b66a884aaf08f1c410c61682a7072d68a0d837ba8dc16ff13b9509bdceb32fd2/diff: invalid argument"
         # # [teardown]
         # rm: cannot remove '/tmp/CI_M4p5/podman_bats.ozboK6/imagestore/root/overlay': Device or resource busy

[giuseppe]

ok let's make it in two phases, I'll change this PR to not use the symlinks but still create them

[giuseppe]

Podman tests pass now

[Luap99]

LGTM

[kolyshkin]

Assuming the new functionality is covered by existing tests, LGTM