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

Tycho Andersen tycho.andersen at canonical.com
Wed Apr 15 16:06:39 UTC 2015


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
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


More information about the lxc-devel mailing list