fix: clarify comments in molecule.Mount and lxcRootfsString #494

smoser · 2023-08-30T19:51:36Z

fix: clarify comments in molecule.Mount and lxcRootfsString

molecule.overlayArgs provides options for a kernel mount-option string.
Kernel mount options for overlay require more than one lowerdir if
there is no upperdir. molecule.Mount always performs read-only
mounts (thus no upperdir). So molecule.overlayArgs must add
a workaround lowerdir if there is only one.

Separately, lxcRootfsString function returns a string to be supplied to
lxc's lxc.rootfs.path. lxc.rootfs.path is of the form:
overlayfs:lowerdir[:lowerdir2:lowerdir3]:upperdir.
It requires 1 or more lowerdirs and an upperdir.
So in lxcRootfsString we need to create a workaround dir only if there
are zero lowerdirs (an empty container).

Signed-off-by: Scott Moser smoser@brickies.net

this is a follow-on to #493

hallyn · 2023-08-30T20:53:44Z

The comment is true if there are no upperdirs. I couldn't easily tell with quick code inspection - do you know whether this path can happen with no upperdir?

mikemccracken · 2023-08-30T21:39:51Z

moved from chat to avoid getting it lost:

I think the comment is wrong, but isn't it possible for there to be a molecule with just one atom?
and hence one dir, requiring a fake workaround lowerdir...

smoser · 2023-08-31T14:08:39Z

moved from chat to avoid getting it lost:

I think the comment is wrong, but isn't it possible for there to be a molecule with just one atom? and hence one dir, requiring a fake workaround lowerdir...

Thanks for pushing. You're right. I wasn't thinking about the fact that the molecule mount actually just mounts read-only. I didn't realize that there was a limitation if you didn't use an upperdir.

If you don't have an upperdir, then you need more than one lowerdir. That doesn't seem like it would have to be so, but it currently is. A mount with one lowerdir and no upperdir would just be effectively a bind 'mount -o ro,bind'.

Just to clarify for myself, some shell snippets below. 'unshared' func just allows unpriv mounts.

To summarize what I have found:

1 lowerdir with no upperdir errors with dmesg message
overlayfs: at least 2 lowerdir are needed while upperdir nonexistent
1 lowerdir with upperdir works fine.
2 lowerdir with no upperdir works fine in read-only.
without upperdir and workdir mount is read-only ('rw' is silently ignored if upperdir and workdir are not given)
upperdir without workdir errors with dmesg message (overlayfs: missing 'workdir')
unpriv rw mount gives dmesg warning fs:
fs on 'ld1' does not support file handles, falling back to xino=off.

$ unshared() { unshare --user --pid --fork --mount --map-root-user "$@"; }

$ uname -r
5.19.0-46-generic
$ id -u
1000

$ rm -Rf ld1 ld2 ud wk mp
$ mkdir ld1 ld2 ud wkd mp
$ touch ld1/f1 ld2/f2

## one lower dir, no upper dir
$ unshared sh -xc "mount -t overlay -o lowerdir=ld1,ro ovl mp && ls -l mp"
+ mount -t overlay -o lowerdir=ld1,ro ovl mp
mount: /home/smoser/src/stacker/mp: wrong fs type, bad option, bad superblock on ovl, missing codepage or helper program, or other error.
$ dmesg  | tail -n 1
[3534424.448506] overlayfs: at least 2 lowerdir are needed while upperdir nonexistent

## one lower dir, upper dir, ro (probably doens't make sense)
$ unshared sh -xc "mount -t overlay -o lowerdir=ld1,workdir=wkd,upperdir=ud,ro ovl mp && ls -l mp"
+ mount -t overlay -o lowerdir=ld1,workdir=wkd,upperdir=ud,ro ovl mp
+ ls -l mp
total 0
-rw-rw-r-- 1 root root 0 Aug 31 09:41 f1
$ rm -Rf mp; mkdir mp

## two lower dir, no upper dir, ro
$ unshared sh -xc "mount -t overlay -o lowerdir=ld1:ld2,ro ovl mp && ls -l mp"
+ mount -t overlay -o lowerdir=ld1:ld2,ro ovl mp
+ ls -l mp
total 0
-rw-rw-r-- 1 root root 0 Aug 31 09:41 f1
-rw-rw-r-- 1 root root 0 Aug 31 09:41 f2

## two lower dir, upper dir, rw
$ rm -Rf mp ud; mkdir mp ud
$ unshared sh -xc "mount -t overlay -o lowerdir=ld1:ld2,workdir=wkd,upperdir=ud,rw ovl mp && ls -l mp && touch mp/f1 && grep $PWD/mp /proc/mounts"
+ mount -t overlay -o lowerdir=ld1:ld2,workdir=wkd,upperdir=ud,rw ovl mp
+ ls -l mp
total 0
-rw-rw-r-- 1 root root 0 Aug 31 09:41 f1
-rw-rw-r-- 1 root root 0 Aug 31 09:41 f2
+ touch mp/f1
+ grep /home/smoser/src/stacker/mp /proc/mounts
ovl /home/smoser/src/stacker/mp overlay rw,relatime,lowerdir=ld1:ld2,upperdir=ud,workdir=wkd 0 0
$ dmesg | tail -n 1
[3535363.890175] overlayfs: fs on 'ld1' does not support file handles, falling back to xino=off.

## two lower dirs, no upper or work, attempt 'rw'
$ unshared sh -xc "mount -t overlay -o rw,lowerdir=ld1:ld2 ovl mp && ls -l mp && touch mp/f1 && grep $PWD/mp /proc/mounts"
+ mount -t overlay -o rw,lowerdir=ld1:ld2 ovl mp
+ ls -l mp
total 0
-rw-rw-r-- 1 root root 0 Aug 31 09:41 f1
-rw-rw-r-- 1 root root 0 Aug 31 09:41 f2
+ touch mp/f1
touch: cannot touch 'mp/f1': Read-only file system

## privileged mount 2 lower in RW
$ sudo sh -xc "mount -t overlay -o rw,lowerdir=ld1:ld2,workdir=wkd,upperdir=ud ovl mp && ls -l mp && touch mp/f1 && grep $PWD/mp /proc/mounts"
+ mount -t overlay -o rw,lowerdir=ld1:ld2,workdir=wkd,upperdir=ud ovl mp
+ ls -l mp
total 0
-rw-rw-r-- 1 smoser smoser 0 Aug 31 10:04 f1
-rw-rw-r-- 1 smoser smoser 0 Aug 31 09:41 f2
+ touch mp/f1
+ grep /home/smoser/src/stacker/mp /proc/mounts
ovl /home/smoser/src/stacker/mp overlay rw,relatime,lowerdir=ld1:ld2,upperdir=ud,workdir=wkd 0 0
$ sudo umount mp
# dmesg does *not* show a a message about ld1 filehandles as unpriv mount does.

molecule.overlayArgs provides options for a kernel mount-option string. Kernel mount options for overlay require *more* than one lowerdir if there is no upperdir. molecule.Mount always performs read-only mounts (thus no upperdir). So molecule.overlayArgs must add a workaround lowerdir if there is only one. Separately, lxcRootfsString function returns a string to be supplied to lxc's lxc.rootfs.path. lxc.rootfs.path is of the form: overlayfs:lowerdir[:lowerdir2:lowerdir3]:upperdir. It requires 1 or more lowerdirs and an upperdir. So in lxcRootfsString we need to create a workaround dir only if there are zero lowerdirs (an empty container). Signed-off-by: Scott Moser <smoser@brickies.net>

smoser · 2023-08-31T15:01:39Z

OK, Sorry for the confusion and for taking you all down the rathole with me.

Now this PR is a comment-only change. Hopefully it commit message and the comments changes are an improvement.

smoser requested review from rchincha and hallyn as code owners August 30, 2023 19:51

smoser force-pushed the fix/one-more-workaround-removal branch from 83834fe to 2bc1e6f Compare August 30, 2023 19:52

smoser force-pushed the fix/one-more-workaround-removal branch from 2bc1e6f to 1da68ae Compare August 31, 2023 14:53

smoser changed the title ~~fix: Remove use of workaround mount dir from molecule.overlayArgs~~ further adjust lxcRootfsString, clarify comments in molecule.Mount Aug 31, 2023

smoser force-pushed the fix/one-more-workaround-removal branch from 1da68ae to 01ed425 Compare August 31, 2023 14:59

smoser changed the title ~~further adjust lxcRootfsString, clarify comments in molecule.Mount~~ fix: clarify comments in molecule.Mount and lxcRootfsString Aug 31, 2023

mikemccracken approved these changes Aug 31, 2023

View reviewed changes

hallyn approved these changes Aug 31, 2023

View reviewed changes

smoser merged commit 396ff9d into project-stacker:main Sep 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: clarify comments in molecule.Mount and lxcRootfsString #494

fix: clarify comments in molecule.Mount and lxcRootfsString #494

Uh oh!

smoser commented Aug 30, 2023 •

edited

Loading

Uh oh!

hallyn commented Aug 30, 2023

Uh oh!

mikemccracken commented Aug 30, 2023

Uh oh!

smoser commented Aug 31, 2023

Uh oh!

smoser commented Aug 31, 2023

Uh oh!

Uh oh!

fix: clarify comments in molecule.Mount and lxcRootfsString #494

fix: clarify comments in molecule.Mount and lxcRootfsString #494

Uh oh!

Conversation

smoser commented Aug 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hallyn commented Aug 30, 2023

Uh oh!

mikemccracken commented Aug 30, 2023

Uh oh!

smoser commented Aug 31, 2023

Uh oh!

smoser commented Aug 31, 2023

Uh oh!

Uh oh!

smoser commented Aug 30, 2023 •

edited

Loading