[CRIU] [PATCH] remap: add a dead pid /proc remap

[Pavel Emelyanov]

On 09/05/2014 05:32 PM, Tycho Andersen wrote:
> Hi Pavel,
> 
> On Thu, Sep 04, 2014 at 04:19:49PM +0400, Pavel Emelyanov wrote:
>> On 09/04/2014 01:13 AM, Tycho Andersen wrote:
>>> From: Tycho Andersen <tycho at tycho.ws>
>>>
>>> If a file like /proc/20/mountinfo is open, but 20 is a zombie (or doesn't exist
>>> any more), we can't read this file at all, so a link remap won't work. Instead,
>>> we add a new remap, called the dead process remap, which forks a TASK_HELPER as
>>> that dead pid so that the restore task can open the new /proc/20/mountinfo
>>> instead.
>>>
>>> Previously, TASK_HELPERs died before the restore stage took place. However, the
>>> fds are set up during the restore stage, so we need TASK_HELPERs to live at
>>> least until then. This patch also rearranges some of the synchronization to
>>> keep TASK_HELPERs alive until the restore stage ends.
>>
>> Can you fix the helpers lifetime with separate patch, please?
>> And a couple of more comments inline.
>>
>>
>>> @@ -539,6 +562,38 @@ static int check_path_remap(char *rpath, int plen, const struct fd_parms *parms,
>>>  	struct stat pst;
>>>  	const struct stat *ost = &parms->stat;
>>>  
>>> +	if (is_path_prefix(rpath, "./proc")) {
>>
>> I would suggest checking that the file is procfs's one by looking at the
>> fd_parms.fs_type field.
>>
>>> +		/* The file points to /proc/pid/<foo> where pid is a dead
>>> +		 * process. We remap this file by adding this pid to be
>>> +		 * fork()ed into a TASK_HELPER state so that we can point to it
>>> +		 * on restore.
>>> +		 */
>>> +		pid_t pid;
>>> +		char *start, *end;
>>> +
>>> +		/* skip "./proc/" */
>>> +		start = strstr(rpath, "/") + 1;
>>> +		if (!start)
>>> +			return -1;
>>> +		start = strstr(start, "/") + 1;
>>> +		if (!start)
>>> +			return -1;
>>> +		pid = strtol(start, &end, 10);
>>> +
>>> +		/* if we didn't find another /, this path something
>>> +		 * like ./proc/kmsg, which we shouldn't mess with. */
>>
>> ...
>>
>>>  static inline int shared_fdtable(struct pstree_item *item) {
>>> diff --git a/protobuf/pstree.proto b/protobuf/pstree.proto
>>> index 6cbcfd3..6cc3953 100644
>>> --- a/protobuf/pstree.proto
>>> +++ b/protobuf/pstree.proto
>>> @@ -4,4 +4,5 @@ message pstree_entry {
>>>  	required uint32			pgid		= 3;
>>>  	required uint32			sid		= 4;
>>>  	repeated uint32			threads		= 5;
>>> +	repeated uint32			dead_pids	= 6;
>>
>> Can we have the "dead" tasks encoded in remap-fpath image instead of
>> the pstree one? I.e. -- the remap_id should be the pid of the process
>> we need and there should be new optional remap_type field set to
>> PROCFS constant.
> 
> So I looked into this, the problem is that we don't have the remap
> images loaded by fork time, so we can't use the same fork/reap
> machinery if we do it this way. We could put the dead pids in a
> separate image and load that when we need. Does that make sense?

Well, remap images are loaded like this:

restore_root_task
 `- fork_with_pid
     `- restore_task_with_children
         `- /* current->parent == NULL branch */
            root_prepare_shared()
             `- collect_image() /* loop */
                 `- remap_cinfo->collect_one_remap

At this point no other task has been forked, so we can
just add the required items into root's children list. Can we?

> Tycho
> 
>>>  }
>>
>> Thanks,
>> Pavel
>>
> .
>