-
Notifications
You must be signed in to change notification settings - Fork 237
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
overlay: use idmapped lower layers where supported #1180
overlay: use idmapped lower layers where supported #1180
Conversation
557c051
to
5be1290
Compare
tested with these kernel patches: https://lore.kernel.org/linux-unionfs/20220329103526.1207086-1-brauner@kernel.org/T/#m84a0bbc1476af67073a48f5800c858a361848413 |
57b55cc
to
8700893
Compare
drivers/overlay/check.go
Outdated
workDir := filepath.Join(layerDir, "work") | ||
|
||
defer func() { | ||
_ = unix.Unmount(mergedDir, unix.MNT_DETACH) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this defer be lower down at least for the Umount.
drivers/overlay/idmapped_utils.go
Outdated
for _, m := range idmap { | ||
mappings = mappings + fmt.Sprintf("%d %d %d\n", m.ContainerID, m.HostID, m.Size) | ||
} | ||
if err := ioutil.WriteFile(fmt.Sprintf("/proc/%d/%s", pid, fname), []byte(mappings), 0600); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should just return ioutil.WriteFile()
drivers/overlay/overlay.go
Outdated
} else { | ||
logrus.Debugf("Cached value indicated that overlay is not supported") | ||
} | ||
if !overlayCacheResult { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for this check if, just move return false below after Debugf line above.
8700893
to
9fe4a9a
Compare
thanks for the review. I've addressed the comments and pushed a new version |
drivers/overlay/idmapped_utils.go
Outdated
// MOUNT_ATTR__ATIME - Setting on how atime should be updated | ||
MOUNT_ATTR__ATIME = 0x00000070 //nolint:golint | ||
// MOUNT_ATTR_RELATIME - Update atime relative to mtime/ctime | ||
MOUNT_ATTR_RELATIME = 0x00000000 //nolint:golint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick question is this one correct? No bits set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it is correct:
$ grep MOUNT_ATTR_RELATIME /usr/include/linux/mount.h
#define MOUNT_ATTR_RELATIME 0x00000000 /* - Update atime relative to mtime/ctime. */
I am not sure if we should add all of these (added for completeness) and if they should be public.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use them from here?
grep -r MOUNT_ATTR_NODEV .
./vendor/golang.org/x/sys/unix/zerrors_linux.go: MOUNT_ATTR_NODEV = 0x4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've dropped the ones not used by the module and made the other 3 private
9fe4a9a
to
8dff045
Compare
use idmapped mounts for the overlay lower layers when the kernel supports them. For each lower directory with ID=0...N-1, it creates a idmapped mount at $GRAPHROOT/overlay/$LAYER/mapped/$ID. The final overlay mount will use these idmapped mounts instead of the original source directory. The upperdir is not idmapped, so files are created with the same IDs used by the user namespace. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
8dff045
to
b9b8a59
Compare
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
use idmapped mounts for the overlay lower layers when the kernel
supports them.
For each lower directory with ID=0...N-1, it creates a idmapped mount
at $GRAPHROOT/overlay/$LAYER/mapped/$ID. The final overlay mount will
use these idmapped mounts instead of the original source directory.
The upperdir is not idmapped, so files are created with the same
IDs used by the user namespace.
Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com