Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve extra overlayfs mount options when loading many-layered images #1156

Merged
merged 1 commit into from
Feb 28, 2022
Merged

Preserve extra overlayfs mount options when loading many-layered images #1156

merged 1 commit into from
Feb 28, 2022

Conversation

kousu
Copy link
Contributor

@kousu kousu commented Feb 28, 2022

When overlayfs loads a container image that has many layers, each becomes an extra argument to the 'lowerdir' list, and if it has many many many of them it will try to squeeze more in by rewriting them as relative paths.

Unfortunately, this case was forgetting 'userxattr' and 'volatile' and any other user-supplied mount options, which meant that in rootless mode (which needs userxattr to behave properly) you'd get an EXDEV error anytime you tried to rename() a directory.

This swaps a couple steps around to make it easier to make both cases the same.

This should fix containers/podman#13123, but I haven't actually tested it yet because I haven't figured out how. But I traced enough out to spot the bug and wanted to get some code up before I forgot it.

Cheers!

Comment on lines +1535 to +1537
if len(optsList) > 0 {
opts = fmt.Sprintf("%s,%s", opts, strings.Join(optsList, ","))
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is the fix. The rest of the patch is just supporting changes for this line.

When overlayfs loads a container image that has many layers, each becomes
an extra argument to the 'lowerdir' list, and if it has many many many
of them it will try to squeeze more in by rewriting them as relative paths.

Unfortunately, this case was forgetting 'userxattr' and 'volatile' and
any other user-supplied mount options. It meant that in rootless mode
(which needs userxattr to behave properly) you'd get an EXDEV error
anytime you tried to rename() a directory.

Fixes containers/podman#13123

Signed-off-by: Nick Guenther <[email protected]>
Copy link
Collaborator

@flouthoc flouthoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good to me. Just need to try it once.

@containers/storage-maintainers PTAL

@flouthoc
Copy link
Collaborator

or a dummy PR against podman would be a good verifier.

@rhatdan
Copy link
Member

rhatdan commented Feb 28, 2022

@giuseppe PTAL

Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rhatdan
Copy link
Member

rhatdan commented Feb 28, 2022

Thanks @kousu
LGTM

@rhatdan rhatdan merged commit ebc90ab into containers:main Feb 28, 2022
@kousu kousu deleted the userxattr-even-on-large-images branch February 28, 2022 17:07
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.

overlay-mode containerization breaks apt: Invalid cross-device link
4 participants