Skip to content

Fix ro mount option being passed#8852

Merged
fuweid merged 1 commit intocontainerd:mainfrom
bpfoster:issue-8851
Jul 21, 2023
Merged

Fix ro mount option being passed#8852
fuweid merged 1 commit intocontainerd:mainfrom
bpfoster:issue-8851

Conversation

@bpfoster
Copy link
Copy Markdown
Contributor

@bpfoster bpfoster commented Jul 20, 2023 •
edited
Loading

I am quite unfamiliar with the containerd codebase, but this seems to fix issue #8851 in my testing.

Fixes #8851

@k8s-ci-robot
Copy link
Copy Markdown

k8s-ci-robot commented Jul 20, 2023

Hi @bpfoster. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@bpfoster
Fix ro mount option being passed
f3daf32 
"ro" was not parsed out of the string, so it was passed as part of data
to mount().
This would lead to mount() returning an invalid argument code.
Separate out the "ro" option, much like "userxattr", which will allow
the MS-RDONLY mountflag to get set.

Signed-off-by: Ben Foster <bpfoster@gmail.com>
@dcantah
Copy link
Copy Markdown
Member

dcantah commented Jul 21, 2023

/ok-to-test

fuweid
fuweid approved these changes Jul 21, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

@fuweid fuweid merged commit 59959b3 into containerd:main Jul 21, 2023
@bpfoster bpfoster deleted the issue-8851 branch July 21, 2023 13:36
@bpfoster
Copy link
Copy Markdown
Contributor Author

bpfoster commented Jul 28, 2023

Sorry for not knowing the process, but is it possible to get this fix incorporated into a 1.7.x release?

@estesp estesp added cherry-pick/1.6.x cherry-pick/1.7.x Change to be cherry picked to release/1.7 branch labels Jul 28, 2023
@estesp estesp mentioned this pull request Jul 28, 2023
Merged
@estesp estesp added cherry-picked/1.7.x PR commits are cherry-picked into release/1.7 branch and removed cherry-pick/1.7.x Change to be cherry picked to release/1.7 branch labels Jul 28, 2023
@estesp
Copy link
Copy Markdown
Member

estesp commented Jul 28, 2023

Sorry for not knowing the process, but is it possible to get this fix incorporated into a 1.7.x release?

I've cherry picked it into a PR for release/1.7 so it should make the next 1.7.x release

@estesp estesp mentioned this pull request Jul 28, 2023
Merged
@estesp estesp added cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch and removed cherry-pick/1.6.x labels Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fuweid fuweid fuweid approved these changes

@dcantah dcantah dcantah approved these changes

@ktock ktock ktock approved these changes

Assignees

No one assigned

Labels

cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch cherry-picked/1.7.x PR commits are cherry-picked into release/1.7 branch ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Filesystem userxattr check in overlayfs snapshotter can fail due to improper mount call

6 participants

@bpfoster @k8s-ci-robot @dcantah @estesp @fuweid @ktock