[CRIU] [PATCH] restore: Set CLONE_PARENT iif pdeath_sig is present

[Andrew Vagin]

On Thu, Aug 14, 2014 at 07:38:22PM +0400, Pavel Emelyanov wrote:
> On 08/14/2014 07:35 PM, Cyrill Gorcunov wrote:
> > It's been discovered that on 3.11 we might fail on restore
> > if pass @CLONE_PARENT flag into clone() call due to kernel
> > limitations.
> > 
> > Because we're treating 3.11 as a base working kernel lets
> > do a trick instead
> > 
> >  - setup this flag iif pdeath_sig is present
> >  - if CLONE_NEWPID is passed warn a user about
> >    potential consequences.
> > 
> > CC: Tycho Andersen <tycho.andersen at canonical.com>
> > CC: Andrey Vagin <avagin at openvz.org>
> > CC: Pavel Emelyanov <xemul at parallels.com>
> > Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> > ---
> > 
> > Guys, what do you think?
> > 
> >  cr-restore.c | 46 +++++++++++++++++++++++++++++++++-------------
> >  1 file changed, 33 insertions(+), 13 deletions(-)
> > 
> > diff --git a/cr-restore.c b/cr-restore.c
> > index bd16b1d81ca7..ea6437d6d503 100644
> > --- a/cr-restore.c
> > +++ b/cr-restore.c
> > @@ -922,6 +922,37 @@ struct cr_clone_arg {
> >  	CoreEntry *core;
> >  };
> >  
> > +static void maybe_clone_parent(struct pstree_item *item,
> > +			      struct cr_clone_arg *ca)
> > +{
> > +	if (likely(item->pid.virt != INIT_PID))
> > +		return;
> > +
> > +	if (opts.swrk_restore || opts.restore_detach) {
> > +		/*
> > +		 * This means we're called from lib's criu_restore_child().
> > +		 * In that case create the root task as the child one to+
> > +		 * the caller. This is the only way to correctly restore the
> > +		 * pdeath_sig of the root task. But also looks nice.
> > +		 *
> > +		 * Alternatively, if we are --restore-detached, a similar trick is
> > +		 * needed to correctly restore pdeath_sig and prevent processes from
> > +		 * dying once restored.
> > +		 *
> > +		 * There were a problem in kernel 3.11 -- CLONE_PARENT can't be
> > +		 * set together with CLONE_NEWPID, which has been solved in further
> > +		 * versions of the kernels, but we treat 3.11 as a base, so at
> > +		 * least warn a user about potential problems.
> > +		 */
> > +		if (ca->core->thread_core->pdeath_sig) {
> > +			item->rst->clone_flags |= CLONE_PARENT;
> > +			if (item->rst->clone_flags & CLONE_NEWPID)
> > +				pr_warn("Set CLONE_PARENT | CLONE_NEWPID but it might cause restore problem,"
> > +					"because not all kernels support such clone flags combinations!\n");
> > +		}
> > +	}
> > +}
> > +
> >  static inline int fork_with_pid(struct pstree_item *item)
> >  {
> >  	int ret = -1, fd;
> > @@ -951,6 +982,8 @@ static inline int fork_with_pid(struct pstree_item *item)
> >  			pr_err("Unknown task state %d\n", item->state);
> >  			return -1;
> >  		}
> > +
> > +		maybe_clone_parent(item, &ca);
> >  	} else {
> >  		/*
> >  		 * Helper entry will not get moved around and thus
> > @@ -1587,19 +1620,6 @@ static int restore_root_task(struct pstree_item *init)
> >  	futex_set(&task_entries->nr_in_progress,
> >  			stage_participants(CR_STATE_RESTORE_NS));
> >  
> > -	/*
> > -	 * This means we're called from lib's criu_restore_child().
> > -	 * In that case create the root task as the child one to+
> > -	 * the caller. This is the only way to correctly restore the
> > -	 * pdeath_sig of the root task. But also looks nice.
> > -	 *
> > -	 * Alternatively, if we are --restore-detached, a similar trick is
> > -	 * needed to correctly restore pdeath_sig and prevent processes from
> > -	 * dying once restored.
> > -	 */
> > -	if (opts.swrk_restore || opts.restore_detach)
> > -		init->rst->clone_flags |= CLONE_PARENT;
> 
> There are 2 more places that check for swrk_restore || restore_detach.
> We need to fix them too, since CLONE_PARENT and these other two do things,
> that depend on each-other.

And one more place where is only swrk_restore:
ret = attach_to_tasks(opts.swrk_restore)

> 
> > -
> >  	ret = fork_with_pid(init);
> >  	if (ret < 0)
> >  		return -1;
> > 
>