On Wed, Apr 15, 2015 at 04:19:54PM +0000, Serge Hallyn wrote: > Quoting Tycho Andersen (tycho.ander...@canonical.com): > > On Wed, Apr 15, 2015 at 03:48:10PM +0000, Serge Hallyn wrote: > > > Quoting Tycho Andersen (tycho.ander...@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?
A fair point :) > 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. Right, but in the case of e.g. checkpoint it will only ever be called once, because as soon as you invoke checkpoint the monitor process dies. Anyway, I think I'll just check it in the critical path. When one extra fork is a performance bottleneck for migration I'll probably be rotting away peacefully in a pine box somewhere :) Tycho > > 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.ander...@canonical.com> > > > > > > Acked-by: Serge E. Hallyn <serge.hal...@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@lists.linuxcontainers.org > > http://lists.linuxcontainers.org/listinfo/lxc-devel > _______________________________________________ > lxc-devel mailing list > lxc-devel@lists.linuxcontainers.org > http://lists.linuxcontainers.org/listinfo/lxc-devel _______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel
