<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">2016-08-04 18:53 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div class=""><div class="h5">On Thu, Aug 04, 2016 at 06:42:06PM +0300, Eugene Batalov wrote:<br>
> 2016-08-04 18:08 GMT+03:00 Tycho Andersen <<a href="mailto:tycho.andersen@canonical.com">tycho.andersen@canonical.com</a>><wbr>:<br>
><br>
> > Hi Eugene,<br>
> ><br>
> > On Thu, Aug 04, 2016 at 02:49:23PM +0300, Eugene Batalov wrote:<br>
> > > Hi Tycho,<br>
> > ><br>
> > > > > +int gc_network_unlock(void)<br>
> > > > > +{<br>
> > > > > + /*<br>
> > > > > + * Unshared ps tree net ns is destroyed after successful dump.<br>
> > > > > + * No need to call network_unlock_internal.<br>
> > > > > + * Also don't call ACT_NET_UNLOCK script because we don't<br>
> > > > > + * resume/restore ps tree - this call would break<br>
> > > > > + * ACT_NET_UNLOCK semantics.<br>
> > > > > + */<br>
> > > > > + return rst_unlock_tcp_connections();<br>
> > > ><br>
> > > > What about cpt_unlock_tcp_connections()? IIUC this list is not<br>
> > > > persisted, and so if we leave around the network lock stuff, we would<br>
> > > > never turn of TCP repair mode.<br>
> > > ><br>
> > > > Tycho<br>
> > > ><br>
> > > Let's consider the moment when we start criu gc or criu restore. At<br>
> > > this moment dumpee ps tree doesn't exist and ps tree sockets don't<br>
> > > exist.<br>
> ><br>
> > Hmm. Perhaps I misunderstood then. I thought the point was to be able<br>
> > to use --leave-stopped (so the ps tree would still exist), and then we<br>
> > could at a later time run `criu gc` to unlock things again and clean<br>
> > this up.<br>
> ><br>
> Looks like this is the use case you need to support. Let's look at current<br>
> implementation. Does it satisfies your needs?<br>
<br>
</div></div>I worked around my issue in another way, so I think we can ignore my<br>
needs for the purpose of `criu gc`. If we wanted to clean up after a<br>
dump that left the network locked, I don't think this set would do it<br>
completely, but it sounds like we may not care about that.<br></blockquote><div>Could you propose an example when this patch set doesn't unlock the network?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<span class=""><br>
> We can kill whole ps tree with any standard signal based logic so it looks<br>
> like there is no problem here.<br>
> But is it possible for you to have dump of this killed ps tree. criu gc<br>
> needs this dump in order to do its job.<br>
> Also criu gc requires only subset of the dump (memory is not needed).<br>
><br>
> For now I prefer current criu gc implementation that works with dumps<br>
> rather then with live ps trees.<br>
> In general If we want to c/r we always create dump from live ps tree but we<br>
> don't always restore ps tree from dump.<br>
> So dump is more basic thing that criu gc relies on. The only question is<br>
> about availability of such a dump for gc.<br>
<br>
</span>Ok, but I still don't understand, what's the use case for something this?<br></blockquote><div>I have some thoughts like "if we don't restore ps tree we leave some garbage in the system and we should be able to remove this garbage".</div><div>Maybe Pavel can tell us about more important use case.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<span class=""><br>
> ><br>
> > > That's why we don't need to turn any tcp repair mode off - it is per<br>
> > socket<br>
> > > thing.<br>
> > > Also cpt_unlock_tcp_connections called from network_unlock() that is<br>
> > called<br>
> > > during criu restore<br>
> > > does nothing because cpt_tcp_repair_sockets list is empty. So for me it<br>
> > > looks like criu gc works<br>
> > > the same as criu restore but without calling of network unlock action<br>
> > > script.<br>
> ><br>
> > I understand; it may be worth noting somewhere in the help output or<br>
> > patch notes what exactly the use case is. Right now I don't undersand<br>
> > what it is :)<br>
> ><br>
> Yep. I'll add more info into criu help message in the next patch set<br>
> version. Thank you for suggestion.<br>
<br>
</span>OK :)<br>
<span class=""><font color="#888888"><br>
Tycho<br>
</font></span><div class=""><div class="h5"><br>
> ><br>
> > Tycho<br>
> ><br>
> > > ><br>
> > > > > +}<br>
> > > > > +<br>
> > > > > int veth_pair_add(char *in, char *out)<br>
> > > > > {<br>
> > > > > char *aux;<br>
> > > > > diff --git a/criu/sk-tcp.c b/criu/sk-tcp.c<br>
> > > > > index 13d175a..c4aa58f 100644<br>
> > > > > --- a/criu/sk-tcp.c<br>
> > > > > +++ b/criu/sk-tcp.c<br>
> > > > > @@ -713,16 +713,19 @@ void tcp_locked_conn_add(struct inet_sk_info<br>
> > *ii)<br>
> > > > > ii->sk_fd = -1;<br>
> > > > > }<br>
> > > > ><br>
> > > > > -void rst_unlock_tcp_connections(<wbr>void)<br>
> > > > > +int rst_unlock_tcp_connections(<wbr>void)<br>
> > > > > {<br>
> > > > > struct inet_sk_info *ii;<br>
> > > > ><br>
> > > > > /* Network will be unlocked by network-unlock scripts */<br>
> > > > > if (root_ns_mask & CLONE_NEWNET)<br>
> > > > > - return;<br>
> > > > > + return 0;<br>
> > > > ><br>
> > > > > list_for_each_entry(ii, &rst_tcp_repair_sockets, rlist)<br>
> > > > > - nf_unlock_connection_info(ii);<br>
> > > > > + if (nf_unlock_connection_info(ii)<wbr>)<br>
> > > > > + return -1;<br>
> > > > > +<br>
> > > > > + return 0;<br>
> > > > > }<br>
> > > > ><br>
> > > > > int check_tcp(void)<br>
> > > > > --<br>
> > > > > 1.9.1<br>
> > > > ><br>
> > > > > ______________________________<wbr>_________________<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/<wbr>mailman/listinfo/criu</a><br>
> > > ><br>
> > ><br>
> > ><br>
> > ><br>
> > > --<br>
> > > Best regards,<br>
> > > Eugene Batalov.<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" data-smartmail="gmail_signature">Best regards,<br>Eugene Batalov.</div>
</div></div>