[CRIU] [PATCH v1 05/12] gc: implement unlocking of tcp connections

[Pavel Emelyanov]

On 08/04/2016 07:26 PM, Tycho Andersen wrote:
> Hi Eugene,
> 
> On Thu, Aug 04, 2016 at 07:01:36PM +0300, Eugene Batalov wrote:
>> 2016-08-04 18:53 GMT+03:00 Tycho Andersen <tycho.andersen at canonical.com>:
>>
>>> On Thu, Aug 04, 2016 at 06:42:06PM +0300, Eugene Batalov wrote:
>>>> 2016-08-04 18:08 GMT+03:00 Tycho Andersen <tycho.andersen at canonical.com>
>>> :
>>>>
>>>>> Hi Eugene,
>>>>>
>>>>> On Thu, Aug 04, 2016 at 02:49:23PM +0300, Eugene Batalov wrote:
>>>>>> Hi Tycho,
>>>>>>
>>>>>>>> +int gc_network_unlock(void)
>>>>>>>> +{
>>>>>>>> +     /*
>>>>>>>> +      * Unshared ps tree net ns is destroyed after successful
>>> dump.
>>>>>>>> +      * No need to call network_unlock_internal.
>>>>>>>> +      * Also don't call ACT_NET_UNLOCK script because we don't
>>>>>>>> +      * resume/restore ps tree - this call would break
>>>>>>>> +      * ACT_NET_UNLOCK semantics.
>>>>>>>> +      */
>>>>>>>> +     return rst_unlock_tcp_connections();
>>>>>>>
>>>>>>> What about cpt_unlock_tcp_connections()? IIUC this list is not
>>>>>>> persisted, and so if we leave around the network lock stuff, we
>>> would
>>>>>>> never turn of TCP repair mode.
>>>>>>>
>>>>>>> Tycho
>>>>>>>
>>>>>> Let's consider the moment when we start criu gc or criu restore. At
>>>>>> this moment dumpee ps tree doesn't exist and ps tree sockets don't
>>>>>> exist.
>>>>>
>>>>> Hmm. Perhaps I misunderstood then. I thought the point was to be able
>>>>> to use --leave-stopped (so the ps tree would still exist), and then we
>>>>> could at a later time run `criu gc` to unlock things again and clean
>>>>> this up.
>>>>>
>>>> Looks like this is the use case you need to support. Let's look at
>>> current
>>>> implementation. Does it satisfies your needs?
>>>
>>> I worked around my issue in another way, so I think we can ignore my
>>> needs for the purpose of `criu gc`. If we wanted to clean up after a
>>> dump that left the network locked, I don't think this set would do it
>>> completely, but it sounds like we may not care about that.
>>>
>> Could you propose an example when this patch set doesn't unlock the network?
> 
> Sorry, I was speaking of the case I already mentioned, about trying to
> use criu gc to clean up after a criu dump --leave-stopped. I think
> this patchset doesn't handle that case because of the missing
> cpt_unlock_tcp_connections(); but it sounds like we don't care about
> that, so never mind :)

Well, yes, _cleaning_ the stopped tasks would be just killing them :)
Otherwise it should be called 'resuming' and I guess I know what you
need it for ;) -- live migration?

Actually, if you look at how p.haul works, it doesn't make criu exit
after final dump, it gets the final notifiction from it and goes on
the restore node for criu restore. If it fails, the notification
is aborted and criu just rolls back.

-- Pavel