<div dir="ltr"><div><span style="font-size:12.8px">Conceptually we're able to get the same state in criu gc as criu restore gets.</span></div><span style="font-size:12.8px"><div><span style="font-size:12.8px"><br></span></div>I am agree about namespaces and network unlock. Our folk has missed this.</span><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">About link remaps and namespaces. This thing from patch looks like it should working:</span></div><div><blockquote style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex" class="gmail_quote"><span style="font-size:12.8px">+<span class="" style="white-space:pre"> </span>if (rfe->remap_type == REMAP_TYPE__LINKED) {<br></span><span style="font-size:12.8px"> +<span class="" style="white-space:pre"> </span>if (open_remap_linked(rfi, rfe))<br></span><span style="font-size:12.8px"> +<span class="" style="white-space:pre"> </span>{<br></span><span style="font-size:12.8px"> +<span class="" style="white-space:pre"> </span>pr_err("open_remap_linked failed");<br></span><span style="font-size:12.8px"> +<span class="" style="white-space:pre"> </span>return -1;<br></span><span style="font-size:12.8px"> +<span class="" style="white-space:pre"> </span>}<br></span><span style="font-size:12.8px"> +<br></span><span style="font-size:12.8px"> +<span class="" style="white-space:pre"> </span>int mntns_root = mntns_get_root_by_mnt_id(rfi->remap->rmnt_id);<br></span><span style="font-size:12.8px"> +<span class="" style="white-space:pre"> </span>if (!unlinkat(mntns_root, rfi->remap->rpath, rfi->remap->is_dir ? AT_REMOVEDIR : 0))</span></blockquote></div><div style=""><span style="font-size:12.8px"><br></span></div><div style=""><span style="font-size:12.8px"><br></span></div></div><div class="gmail_extra"><br><div class="gmail_quote">2016-05-16 20:35 GMT+03:00 Tycho Andersen <span dir="ltr"><<a href="mailto:tycho.andersen@canonical.com" target="_blank">tycho.andersen@canonical.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Mon, May 16, 2016 at 04:00:41PM +0300, Batalov Eugene wrote:<br>
</span><span class="">> Hello.<br>
><br>
> The idea of Pavel to reuse part of "restore: restore freezer cgroup state"<br>
> patch looks good and simple.<br>
><br>
> About networking unlock. Yes. There is a work in progress on new criu gc<br>
> command.<br>
> You can find patch commit here:<br>
> <a href="https://github.com/AuthenticEshkinKot/criu/commit/2ab52fec7a05f9792c871709758d14137fe9f379" rel="noreferrer" target="_blank">https://github.com/AuthenticEshkinKot/criu/commit/2ab52fec7a05f9792c871709758d14137fe9f379</a><br>
><br>
> The main problem with it is the fact that network unlock and link remaps<br>
> cleanup logic<br>
> is coupled with logic of criu restore command. And criu restore logic is<br>
> not trivial.<br>
> So we're trying to carefully reduce the amount of code in CRIU gc. We need<br>
> to reuse CRIU restore command code and need not to introduce ugly hacks and<br>
> excessive CRIU restore code dependencies (this dependence would limit CRIU<br>
> restore code changes at the same time).<br>
<br>
</span>Hm. Actually I wonder if this approach will work entirely.<br>
gc_prepare_namespace() just calls prepare_pstree() and<br>
prepare_mnt_ns(), which won't attach to the namespaces of the dumped<br>
(or restored) tasks, but instead creates entirely new namespaces and<br>
removes the files from there. It also doesn't (I don't think) setns to<br>
the right place when it does network_unlock, because nothing sets up<br>
the right network namespace.<br>
<br>
It seems like this mode also needs a -t option to attach to the right<br>
task that it should clean up, as well as the images directory. Does<br>
that make sense?<br>
<span class="HOEnZb"><font color="#888888"><br>
Tycho<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
> GRIU gc both able print what's left in the system after dump (iptables,<br>
> link remaps) and to cleanup this stuff too.<br>
><br>
><br>
> 2016-05-16 15:31 GMT+03:00 Pavel Emelyanov <<a href="mailto:xemul@virtuozzo.com">xemul@virtuozzo.com</a>>:<br>
><br>
> > On 05/10/2016 08:04 PM, Tycho Andersen wrote:<br>
> > > Hi guys,<br>
> > ><br>
> > > I'm looking at implementing some kind of --leave-frozen option in<br>
> > > CRIU, so that we can have a basic UX in LXD where we can wait for the<br>
> > > restore to be successful before we kill the checkpointed container. I<br>
> > > know p.haul does this by just using a callback, but it would be sort<br>
> > > of painful to absorb just the callback part without doing a lot of<br>
> > > extra engineering. We'll get LXD using p.haul someday, though :)<br>
> > ><br>
> > > The actual --leave-frozen patch is not so bad (see attached), but I'm<br>
> > > not sure what to do about the network locking/unlocking bits.<br>
> ><br>
> > There was a patch 8b04551c (restore: restore freezer cgroup state) in 2.0<br>
> > that turned cgroup into whatever state it was before dump. Can it be fixed<br>
> > to make '--leave-frozen' alter the behavior of<br>
> > add_freezer_state_for_restore()<br>
> > and set it to 'frozen' always?<br>
> ><br>
> > > It seems like it is always safe to do the bits in<br>
> > > cpt_unlock_tcp_connections() since that's just disabling tcp repair<br>
> > > mode, but all of the iptables rules seem necessary in order to keep<br>
> > > the network locked.<br>
> > ><br>
> > > So my question is: is there a nice way we can "tag" these rules so<br>
> > > that something can come by and delete them later? I was thinking about<br>
> > > having criu add a comment (via -m comment --comment "CRIU-LOCK-RULE")<br>
> > > to each rule it adds, but I'm not sure if there's a better way, or if<br>
> > > I've missed something entirely.<br>
> ><br>
> > Yes, there's an issue #45 -- show what's left in the system after dump.<br>
> > Iptables<br>
> > rules are in the list :) I know that some gentlemen (Cc) from<br>
> > Saint-Petersburg were<br>
> > interested in implementing it in form of 'criu gc' action, so probably<br>
> > tuning this<br>
> > option to support '--show-only' would help you?<br>
> ><br>
> > > Thanks!<br>
> > ><br>
> > > Tycho<br>
> > ><br>
> > ><br>
> > ><br>
> > > _______________________________________________<br>
> > > CRIU mailing list<br>
> > > <a href="mailto:CRIU@openvz.org">CRIU@openvz.org</a><br>
> > > <a href="https://lists.openvz.org/mailman/listinfo/criu" rel="noreferrer" target="_blank">https://lists.openvz.org/mailman/listinfo/criu</a><br>
> > ><br>
> ><br>
> ><br>
><br>
><br>
> --<br>
> Best regards,<br>
> Eugene Batalov.<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature">Best regards,<br>Eugene Batalov.</div>
</div>