overlay: drop relLowers workaround

[giuseppe]

commit 7c5964d changed the way the mountFrom program works by always using the /proc/self/fd/%d path for the lower dirs. Hence there is no gain to pass a path relative to the storage, but we can just pass the absolute path as the resulting file path length is the same and won't affect how many layers we can specify.

This fixes idmapped mounts when the total length of lower layers is > 4096 and it also solves the problem of using layers from additional stores.

Closes: #1410

Signed-off-by: Giuseppe Scrivano [email protected]

[giuseppe]

@nalind @rhatdan PTAL

[rhatdan]

LGTM
@nalind @saschagrunert @flouthoc @mtrmac @umohnani8 @vrothberg PTAL

[nalind]

This means we'll fork to do the mount in a child more often than we do now.
We should have switched to always using absolute paths for layers in additional stores in #1375.

[giuseppe]

The issue happened with an image that has many layers. I think it is fine for now to simplify the code if the additional cost happens only with these images

[mtrmac]

• It’s completely unclear to me how this is fixing anything. Was the relative-path code buggy in some way WRT additional stores?
• If this code is code the primary reason for creating the link symlink forest, should we actually drop all users of it? There are some left.
• … and show we stop creating the symlinks at all? (Would that break older-version readers, or would they workaround via recreateSymlink? And if it would break them, do we need to care, or is one-directional compatibility sufficient?)

[giuseppe]

• It’s completely unclear to me how this is fixing anything. Was the relative-path code buggy in some way WRT additional stores?

when idmapped mounts are used, we were just rewriting the absolute lower dirs, but not updating the relLowers. This is probably not possible since the idmapped mounts use a subdirectory where the idmapped bind mount is created, thus the relative dirs in link won't work.

• If this code is code the primary reason for creating the link symlink forest, should we actually drop all users of it? There are some left.
• … and show we stop creating the symlinks at all? (Would that break older-version readers, or would they workaround via recreateSymlink? And if it would break them, do we need to care, or is one-directional compatibility sufficient?)

I had a quick look at it, and it seems the link file is still used by getLower and to point to layers into additional stores so we would first need to fix these two other occurrences.

[mtrmac]

when idmapped mounts are used, we were just rewriting the absolute lower dirs, but not updating the relLowers.

Ah. Thanks for writing that down.

[giuseppe]

can we move this forward or is there anything blocking I should take care of?

[mtrmac]

The implementation LGTM.

As for the PR as a whole, do we want to drop all of the link data structure now, or not? I don’t feel qualified to have an opinion.

[rhatdan]

If we need to drop all of the link data structure now, or not
we can do it in a separate PR.
LGTM