Skip to content

Conversation

@cyphar
Copy link
Member

@cyphar cyphar commented Apr 22, 2025

This is a backport of #4734 to 1.2.x.


While debugging an issue involving failing mounts, I discovered that
just returning the plain mount error message when we are in the fallback
code for handling locked mounts leads to unnecessary confusion.

It also doesn't help that podman currently forcefully sets "rw" on
mounts, which means that rootless containers are likely to hit the
locked mounts issue fairly often.

So we should improve our error messages to explain why the mount is
failing in the locked flags case.

Also, when reading mount errors, it is quite hard to make sense of mount
flags in their hex form. As this is the error path, the minor performance
impact of constructing a string is probably not worth hyper-optimising.
(Though maybe @kolyshkin has an opinion on this.)

Signed-off-by: Aleksa Sarai [email protected]

@cyphar cyphar added the backport/1.2-pr A backport PR to release-1.2 label Apr 22, 2025
@cyphar cyphar added this to the 1.2.7 milestone Apr 22, 2025
Copy link
Member

@rata rata left a comment

Choose a reason for hiding this comment

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

LGTM

@kolyshkin
Copy link
Contributor

Let's fix CI first so we get all green here: #4742

cyphar added 2 commits April 23, 2025 14:35
When reading mount errors, it is quite hard to make sense of mount flags
in their hex form. As this is the error path, the minor performance
impact of constructing a string is probably not worth hyper-optimising.

(cherry pick from commit 30302a2)
Signed-off-by: Aleksa Sarai <[email protected]>
While debugging an issue involving failing mounts, I discovered that
just returning the plain mount error message when we are in the fallback
code for handling locked mounts leads to unnecessary confusion.

It also doesn't help that podman currently forcefully sets "rw" on
mounts, which means that rootless containers are likely to hit the
locked mounts issue fairly often.

So we should improve our error messages to explain why the mount is
failing in the locked flags case.

Fixes: 7c71a22 ("rootfs: remove --no-mount-fallback and finally fix MS_REMOUNT")
(cherry picked from commit 58c3ab7)
Signed-off-by: Aleksa Sarai <[email protected]>
@cyphar cyphar force-pushed the 1.2-mount-errors branch from ce6598a to 3954766 Compare April 23, 2025 04:35
@lifubang lifubang merged commit 98eb876 into opencontainers:release-1.2 Apr 23, 2025
35 checks passed
@cyphar cyphar deleted the 1.2-mount-errors branch April 23, 2025 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/1.2-pr A backport PR to release-1.2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants