[lxc-devel] [PATCH] c/r: rework external mountpoint handling v2

[Serge Hallyn]

Quoting Tycho Andersen (tycho.andersen at canonical.com):
> On Wed, Apr 15, 2015 at 03:48:10PM +0000, Serge Hallyn wrote:
> > Quoting Tycho Andersen (tycho.andersen at canonical.com):
> > > CRIU now supports autodetection of external mounts via the --ext-mount-map auto
> > > --enable-external-sharing --enable-external-masters options, so we don't need
> > > to explicitly pass the cgmanager mount or any of the mounts from the config.
> > > This also means that lxcfs mounts (since they are bind mounts from outside the
> > > container) are autodetected, meaning that c/r of containers using lxcfs works.
> > > 
> > > A further advantage of this patch is that it addresses some of the ugliness
> > > that was in the exec_criu() function. There are other criu options that will
> > > allow us to trim this even further, though.
> > > 
> > > Finally, with --enable-external-masters, criu understands slave mounts in the
> > > container with shared mounts in the peer group that are outside the namespace.
> > > This allows containers on a systemd host to be dumped and restored correctly.
> > > 
> > > However, these options have just landed in criu trunk today, and the next
> > > tagged release will be 1.6 on June 1, so we should avoid merging this into any
> > 
> > Ouch, June 1, that's a ways away.
> > 
> > > stable releases until then.
> > 
> > Should there be a lxc_check_criu_version() fn, updated in cases like these,
> > which ensures that criu is new enough version for the lxc version?
> 
> Yes, I was thinking about this, the only question is how to detect it;
> I can fork() and just pass --version, but then on every checkpoint or
> restore we have an extra fork() in the critical path. Unfortunately, I

The critical path of checkpoint/restore?  Isn't that as speedy as mollasses
now?  You could also cache the result in a global variable (-1 by default)
so only the first lxcapi_checkpoint or lxcapi_restart will invoke the
penalty.

> think that may be our best option right now. If you're ok with the
> forks (I am), I can send a patch for a version check.
> 
> > Given the heavy criu development I do NOT think we should fallback to
> > different behavior on older versions - just require the minversion we
> > need and fail with a nice informational message otherwise.
> 
> Yep, agreed.
> 
> > > v2: remount / as private before bind mounting the container's directory for
> > 
> > It's probably a whacky enough case that lxc+criu would fail anyway, but
> > it *is* possible for / to not be a mountpoint.  Init could chroot you
> > instead of pivot-rooting.  In that case you'd have to bind-mount / onto
> > itself before doing the MS_PRIVATE remount.
> > 
> > I'm also kind of surprised it worked without a MS_REMOUNT flag, but <shrug>
> 
> Yes, in fact criu doesn't pass MS_REMOUNT most of the time, nor does
> LXC when re-mounting / for startup.
> 
> > >     criu. The problem here is that if / is mounted as shared, even if we
> > >     unshare() the /var/lib/lxc/rootfs mountpoint propagates outside of our
> > >     mount namespace, which is bad, since we don't want to leak mounts. In
> > >     particular, this leak confuses criu the second time it goes to checkpoint
> > >     the container.
> > > 
> > > Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
> > 
> > Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> 
> Thanks, although there is a v3 of this patch that I see you replied
> to, I'll try and address those concerns there.
> 
> > But where do we keep this?  We need a github cronjob to post merge request
> > on a given date.
> 
> I was thinking we could just merge it into master since we have the
> stable-1.1 branch that we're tagging 1.1 stable release from.
> 
> Tycho
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel