Skip to content

Conversation

@smoser
Copy link
Contributor

@smoser smoser commented Nov 10, 2022

The overlay attributes [1] have given us some grief [2]. They don't seem to provide benefit. We are keeping the overlay.opaque values, but drop everything else in .overlay. namespace.

[1] https://www.kernel.org/doc/html/latest/filesystems/overlayfs.html
[2] vasi/squashfuse#78

@smoser smoser force-pushed the fix/strip-the-overlay-attrs branch from d3e4d0f to 7294ba9 Compare November 10, 2022 20:47
The overlay attributes [1] have given us some grief [2].
They don't seem to provide benefit.  We are keeping the overlay.opaque
values, but drop everything else in *.overlay.* namespace.

[1] https://www.kernel.org/doc/html/latest/filesystems/overlayfs.html
[2] vasi/squashfuse#78
@smoser smoser force-pushed the fix/strip-the-overlay-attrs branch from 7294ba9 to 4f26b25 Compare November 10, 2022 20:58
@hallyn
Copy link
Contributor

hallyn commented Nov 10, 2022

Should the trusted.overlay.opaque xattrs be renamed to user.overlay.opaque?

@smoser
Copy link
Contributor Author

smoser commented Nov 14, 2022

Should the trusted.overlay.opaque xattrs be renamed to user.overlay.opaque?

It feels to me like that should be a mount time thing, rather than a fs-create time thing. The only thing that I see about 'trusted.overlay' vs 'user.overlay' namespace is from kernel overlay doc .:

The “-o userxattr” mount option forces overlayfs to use the “user.overlay.” xattr namespace instead of “trusted.overlay.”. This is useful for unprivileged mounting of overlayfs.

What do you think, am I missing something? Suggesting that I should modify the creation of a filesystem to account for unpriviledged mounts seems somewhat like suggesting I should shift all my UIDs by 65535 so that a usernamespace would have the right uid/gid mappings. it seems not the right place to do it.

It is absolutely p ossible I'm missing something.

@hallyn
Copy link
Contributor

hallyn commented Nov 15, 2022

So actually we shouldn't need to do a post-build step, we should be able to just mount over build overlays with the ,userxattr option.

Regarding a mount option - how would it work? Would it be an overlay mount option which says "when copying up a file that has a trusted.overlay xattr, ignore that xattr"?

@hallyn
Copy link
Contributor

hallyn commented Nov 15, 2022

I opened #323 - I'm fuzzy on whether that would be a replacement or an enhancement to this PR. I'm thinking replacement right now. What do you think?

Confirmed with a toy busybox based local testcase that i now get user xattr:

$ ~/test/llistxattrfull /mnt/a
ret 20 on /mnt/a
nret 20 list user.overlay.origin

@smoser
Copy link
Contributor Author

smoser commented Nov 15, 2022

So actually we shouldn't need to do a post-build step, we should be able to just mount over build overlays with the ,userxattr option.

Regarding a mount option - how would it work? Would it be an overlay mount option which says "when copying up a file that has a trusted.overlay xattr, ignore that xattr"?

Isn't that what "-o userxattr does" ?

@hallyn
Copy link
Contributor

hallyn commented Nov 15, 2022

So actually we shouldn't need to do a post-build step, we should be able to just mount over build overlays with the ,userxattr option.
Regarding a mount option - how would it work? Would it be an overlay mount option which says "when copying up a file that has a trusted.overlay xattr, ignore that xattr"?

Isn't that what "-o userxattr does" ?

No. -o userxattr will create new xattrs are user., but I don't believe it will prevent breakage if you try to copy up a file from an underlaying fs which has a trusted.overlay. xattr.

@hallyn
Copy link
Contributor

hallyn commented Nov 15, 2022

So, do we still want this patch, @smoser ? (Last night I was thinking no, but now I'm thinking yes.)

@hallyn hallyn self-requested a review November 15, 2022 20:55
@smoser
Copy link
Contributor Author

smoser commented Nov 15, 2022

So, do we still want this patch, @smoser ? (Last night I was thinking no, but now I'm thinking yes.)

well, it solves the problem for the (we think) buggy kernel on our jenkins.

@hallyn hallyn self-requested a review November 15, 2022 22:03
@hallyn hallyn merged commit 52ab507 into project-stacker:main Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants