overlay: extend overlayFileGetter to understand composefs #1950
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
I’m afraid this is not helpful, but just to avoid later surprises: Per #1899 (comment), I will not be approving PRs adding any more alternatives or indirections to the overlay driver. That doesn’t at all mean that I think no-one should be working on those things, just that I can’t be helping. |
|
Seems fair; at a procedural level here: we're looking at using c/storage underneath bootc and I'm trying to spin up on the codebase more as I'm sure you've noticed. Let me know what would be the most helpful/useful way to do that. It'd be good to try to get to a place where have rough consensus and working code. |
so where should it be done if the composefs implementation is in the overlay driver? |
|
I’m not saying this is the wrong place; just that I can’t keep up with the complexity of this subpackage as it is, and as it continues to grow. |
|
… thats “a me problem”, as the young kids say. |
df2f61f to
28254bb
Compare
|
LGTM |
28254bb to
d48b0e8
Compare
|
we need this for the composefs tests in the Podman CI @edsantiago PTAL |
| // there is no *at equivalent for getxattr, but it can be emulated by opening the file under /proc/self/fd/$FD/$PATH | ||
| len, err := unix.Getxattr(fmt.Sprintf("/proc/self/fd/%d/%s", int(f.Fd()), path), "trusted.overlay.redirect", buf) |
There was a problem hiding this comment.
This Getxattr probably wants at some point to be factored out to a helper function, we have a lot of copies of the /proc/self/fd/ trick in this codebase, and there is actually finally some movement on a proper syscall for lgetxattrat that we could change in one place instead of many.
On a different topic it feels a bit like we're poking into composefs internals here? One thing I've thought about in the past that would be nice perhaps is something like an ioctl() usable on an overlayfs that would reopen the file descriptor with the real lower file. Specifically, what I hit is I wanted to be able to reflink across an overlay view and a target in the real underlying filesystem. That's a case that it seems to me we could make safe and viable, and in that model the overlayfs kernel implementation would handle "chasing" this trusted.overlay.redirect.
7a0f217 to
c8f3004
Compare
extend the overlayFileGetter interface to correctly read files from a composefs layer. The overlayFileGetter creates a temporary mount of the composefs blob, grab a directory to its root and immediately unmount it so the mount is visible (and could be leaked) only for a short period. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
c8f3004 to
f1a8923
Compare
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
|
/lgtm |
|
tests pass: https://github.com/containers/podman/pull/22888/checks?check_run_id=25957981302 /hold cancel |
extend the overlayFileGetter interface to correctly read files from a composefs layer.
The overlayFileGetter creates a temporary mount of the composefs blob, grab a directory to its root and immediately unmount it so the mount is visible (and could be leaked) only for a short period.
Alternative to #1944
Marked as a Draft until Podman tests pass