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

[Pavel Emelyanov]

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.

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