[CRIU] Re: [PATCH 6/9] pstree: Allow to dump and restore session non-leaders if --ext-tty passed

[Cyrill Gorcunov]

On Tue, Oct 09, 2012 at 08:01:35PM +0400, Pavel Emelyanov wrote:
> > @@ -1614,7 +1614,7 @@ int cr_dump_tasks(pid_t pid, const struct cr_options *opts)
> >  	if (dump_zombies())
> >  		goto err;
> >  
> > -	if (dump_pstree(root_item))
> > +	if (dump_pstree(root_item, opts))
> >  		goto err;
> >  
> >  	if (opts->namespaces_flags)
> 
> The opts is a good global variable, no need in passing it all over the stack.

OK, will update.

> > +	/*
> > +	 * Make sure we're dumping session leader, if not an
> > +	 * appropriate option must be passed. Another ruse is
> > +	 * that to restore external tty connections we need to
> > +	 * inherit process group as well, thus zap it here.
> > +	 */
> > +	if (root_item->pid.virt != root_item->sid) {
> 
> I think we need a comment, why root's session leader can't sit somewhere
> in the subtree _below_ it.

Actually I don't get how can we have root session leader somewhere
below the root. The root either session leader either belongs to
some session. What I'm missing here?

> 
> > +		if (item->pgid == old_sid)
> > +			e.pgid = INHERIT_SID;
> > +		else if (item->pgid == old_pgid)
> > +			e.pgid = INHERIT_PGID;
> > +		else
> > +			e.pgid = item->pgid;
> 
> I don't understand this.

This tests are inside a loop over all processes in tree and
if some process was being inheriting sid or pgid in original
tree it should remain in the same state after restore. That's
why we check for SID/PGID first and then we assign the real
value if there.

> 
> > @@ -148,6 +204,7 @@ int prepare_pstree(void)
> >  			}
> >  			root_item = pi;
> >  			pi->parent = NULL;
> > +
> >  		} else {
> >  			/*
> >  			 * Fast path -- if the pstree image is not edited, the
> 
> Trash

oops, sorry

> 
> > @@ -331,6 +388,9 @@ int prepare_pstree_ids(void)
> >  		if (gleader)
> >  			continue;
> >  
> > +		if (current_pgid == item->pgid)
> > +			continue;
> > +
> >  		helper = alloc_pstree_item();
> >  		if (helper == NULL)
> >  			return -1;
> 
> Please, explain this hunk.

When we restore tty connection we have to inherit process group
in root task, thus once we have changed the root process group
to an inherited value the logic in prepare_pstree_ids will try
to create process group leader as a helper task, but we don;t
need this, we need the root task to inherit group from the parent.