[lxc-devel] [PATCH 1/1] lxc-start-ephemeral: handle the overlayfs workdir option (v2)

[KATOH Yasufumi]

Hi,

2015-01-13 23:51 GMT+09:00 Stéphane Graber <stgraber at ubuntu.com>:
> On Tue, Jan 13, 2015 at 02:42:24PM +0000, Serge Hallyn wrote:
>> Quoting KATOH Yasufumi (karma at jazz.email.ne.jp):
>> > Hi,
>> >
>> > I tested on kernel 3.18.1 with your patch. It works fine, but there is
>> > one problem.
>> >
>> > >>> On Tue, 13 Jan 2015 00:08:37 +0000
>> >     in message   "[lxc-devel] [PATCH 1/1] lxc-start-ephemeral: handle the overlayfs workdir option (v2)"
>> >                   Serge Hallyn-san wrote:
>> >
>> > > We fixed this some time ago for basic lxc-start, but never did
>> > > lxc-start-ephemeral.
>> >
>> > > Since the lxc-start patches were pushed, Miklos has given us a
>> > > way to detect whether we need the workdir= option.  So the
>> > > bdev.c code could be simplified to check for "overlay\n" in
>> > > /proc/filesystems just as lxc-start-ephemeral does.  This
>> > > patch doesn't do that.
>> >
>> >
>> >
>> > > Changelog (v2):
>> > >   1. use 'overlay' fstype for new overlay upstream module
>> > >   2. avoid using unneeded readlines().
>> >
>> > > Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
>> > > ---
>> > >  src/lxc/lxc-start-ephemeral.in | 25 ++++++++++++++++++++++++-
>> > >  1 file changed, 24 insertions(+), 1 deletion(-)
>> >
>> > > diff --git a/src/lxc/lxc-start-ephemeral.in b/src/lxc/lxc-start-ephemeral.in
>> > > index c999e74..36e2579 100644
>> > > --- a/src/lxc/lxc-start-ephemeral.in
>> > > +++ b/src/lxc/lxc-start-ephemeral.in
>> > > @@ -219,6 +219,14 @@ for entry in args.cdir:
>> > >          dst_path = "%s/rootfs/%s" % (dest_path, src_path)
>> > >          overlay_dirs += [(src_path, dst_path)]
>> >
>> > > +# do we have the new overlay fs which requires workdir, or the older
>> > > +# overlayfs which does not?
>> > > +have_new_overlay = False
>> > > +with open("/proc/filesystems", "r") as fd:
>> > > +    for line in fd:
>> > > +        if line == "nodev\toverlay\n":
>> > > +            have_new_overlay = True
>> >
>> > When the overlayfs is module, and the module is not loaded at this
>> > moment, "have_new_overlay" is False and the mount will fail.
>> >
>> > We need to load the overlayfs module before the check.
>>
>> Yeah I thought of that when I started, then I guess ended up
>> ignoring it.  What would be the cleanest way to do this in
>> python?  Stéphane?
>
> if os.geteuid() == 0:
>     with open(os.devnull, "w") as devnull:
>         subprocess.call(["modprobe", "overlay"], stdout=devnull, stderr=devnull)
>         subprocess.call(["modprobe", "overlayfs"], stdout=devnull, stderr=devnull)
>
> That's about as clean as I can think of. That's also entirely ignoring
> any failure since we may be euid 0 and have those built into the kernel
> or not present at all or not have cap_sys_module or be in a uid mapped
> namespace, all of which would cause a failure.

It looks good to me.
Or...
I thought a little bit about this.
Considering the case of overlayfs is not enabled on the kernel, simply
it might be good only to display the error.

Thanks,
KATOH Yasufumi