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

[Tycho Andersen]

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?

Tycho

> >  }
> 
> Thanks,
> Pavel
>