don't return if remount of "/" fails in create_sandbox #163
Conversation
|
Thanks, could you sign the commit with |
| @@ -135,7 +135,6 @@ int create_sandbox() | |||
| ret = mount("tmpfs", "/", "tmpfs", MS_REMOUNT | MS_RDONLY, "size=0k"); | |||
There was a problem hiding this comment.
@giuseppe Why do we need this tmpfs, and why does it fail?
There was a problem hiding this comment.
was it expected to mount tmpfs on /tmp, not /?
There was a problem hiding this comment.
I think the "tmpfs" here is just ignored, the goal is to remount the entire root as read only. We didn't manage to find the reason why it fails in the @rohrschacht use case
| @@ -135,7 +135,6 @@ int create_sandbox() | |||
| ret = mount("tmpfs", "/", "tmpfs", MS_REMOUNT | MS_RDONLY, "size=0k"); | |||
| if (ret < 0) { | |||
| fprintf(stderr, "cannot mount tmpfs on /tmp\n"); | |||
There was a problem hiding this comment.
yes the error message is wrong here it should mention "cannot remount / as read-only"
There was a problem hiding this comment.
I could add another commit to the PR that changes the error message, if you'd like
I signed the commit locally via rebase, but I can't push it to github without force-push. Would this break the PR? |
|
force-push is fine |
rohrschacht
force-pushed
the
fix-issue-161
branch
from
005fba1 to
995bfc4
Compare
|
Please include |
…create_sandbox Signed-off-by: Tobias Petrich <[email protected]>
rohrschacht
force-pushed
the
fix-issue-161
branch
from
995bfc4 to
2a90980
Compare
|
@giuseppe Do you think we can merge this, or is it better to find out the cause of the issue? |
I think we can merge it if we ever find the root cause we can revert it, not having the rootfs is still a tmpfs so it is still safe. |
suggested in rootless-containers#163 (comment) Signed-off-by: Akihiro Suda <[email protected]>
This fixes issue #161.
By not returning from create_sandbox() when the remount of "/" fails, slirp4netns continues to operate normally instead of exiting.
I tested this setup for a few days and it seems to work fine.