Create bind-mount target directories before executing fuse-overlayfs#1156
Conversation
PhilippWendler
left a comment
PhilippWendler
left a comment
There was a problem hiding this comment.
Thanks a lot!
Overall the code looks good, I liked it!
While reviewing I realized that maybe we can simplify this considerably, so I only did a quick test and not tried out all cases extensively. Could you think about this (creating directories for all mount points) and try it out?
t0hsumi
force-pushed
the
add-mount-dirs-before-fuseoverlayfs
branch
from
f521d0e to
5501d41
Compare
|
Thanks for the detailed review. I just replied the comment and add the commit! |
PhilippWendler
left a comment
PhilippWendler
left a comment
There was a problem hiding this comment.
The current solution does nothing more than creating a few directories in the upper dir of the overlayfs, which AFAIK has no side effects for us and should be completely safe. This is nice and we do not need to worry if it breaks something as long as the code does not crash during execution. But I don't think this can happen, even for inaccessible mount points (which are typically a problem) because isdir just returns False for them.
And in my tests it works fine as a solution for #1136.
Also, the code is nicely readable and well explained, thanks!
So this looks ready to merge, only one question about path decoding remains.
Thank you a lot! Your thorough work on this problem and all the iterations and tests have paid off!
Generally, container.py use `bytes` instances.
Use immutable tuple instead of mutable list and add link where the base implementation is described.
The assumption made by the base implementation have is unreliable (See https://unix.stackexchange.com/questions/18048/list-only-bind-mounts). Also, creating empty directories in `upperdir` that already exist in `lowerdir` does no harm. So for simplicity, create all the mount points.
t0hsumi
force-pushed
the
add-mount-dirs-before-fuseoverlayfs
branch
from
5501d41 to
c1a51be
Compare
PhilippWendler
left a comment
PhilippWendler
left a comment
There was a problem hiding this comment.
Great, thanks a lot again!
PhilippWendler
changed the title
2 participants
Contributes to solving the following issue: #1136
Based on the discussion: #1155
get_bind_mount_points()is based on https://unix.stackexchange.com/questions/18048/list-only-bind-mounts.However, I'm unsure whether the assumption holds that the oldest mount point with the shortest relative path to the root of the mounted file system is the original mount.