[CRIU] [PATCH] restore: define root_as_sibling before using it

[Andrey Vagin]

On Wed, Sep 10, 2014 at 03:46:06PM +0400, Pavel Emelyanov wrote:
> On 09/10/2014 02:18 PM, Pavel Emelyanov wrote:
> 
> > How about adding the 2nd option called --restore-child and make their
> > meaning look like this:
> 
> I mean like this:
> 
> ---
> 
> diff --git a/cr-restore.c b/cr-restore.c
> index 7e00bb3..4d5ccd5 100644
> --- a/cr-restore.c
> +++ b/cr-restore.c
> @@ -963,8 +963,7 @@ static void maybe_clone_parent(struct pstree_item *item,
>  	 * off of 3.11, this condition can be simplified to just test the
>  	 * options and not have the pdeath_sig test.
>  	 */
> -	if (opts.swrk_restore ||
> -	    (opts.restore_detach && ca->core->thread_core->pdeath_sig)) {
> +	if (opts.restore_sibling) {
>  		/*
>  		 * This means we're called from lib's criu_restore_child().
>  		 * In that case create the root task as the child one to+
> @@ -985,6 +984,10 @@ static void maybe_clone_parent(struct pstree_item *item,
>  		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");
> +	} else if (opts.restore_detach) {
> +		if (ca->core->thread_core->pdeath_sig)
> +			pr_warn("Root task has pdeath_sig configured, so it will receive one _right_"
> +				"after restore on CRIU exit\n");
>  	}
>  }
>  
> diff --git a/cr-service.c b/cr-service.c
> index 43840d6..97df433 100644
> --- a/cr-service.c
> +++ b/cr-service.c
> @@ -259,6 +259,14 @@ static int setup_opts_from_req(int sk, CriuOpts *req)
>  	if (req->root)
>  		opts.root = req->root;
>  
> +	if (req->has_rst_sibling) {
> +		pr_err("rst_sibling is not allowed in standalone service\n");

This error should be under the next check.

All other look good for me.

Acked-by: Andrew Vagin <avagin at openvz.org>

> +		if (!opts.swrk_restore)
> +			return -1;
> +
> +		opts.restore_sibling = req->rst_sibling;
> +	}
> +
>  	if (req->has_tcp_established)
>  		opts.tcp_established_ok = req->tcp_established;
>  
> diff --git a/crtools.c b/crtools.c
> index 1c31392..3e2299f 100644
> --- a/crtools.c
> +++ b/crtools.c
> @@ -133,13 +133,14 @@ int main(int argc, char *argv[], char *envp[])
>  	int log_level = LOG_UNSET;
>  	char *imgs_dir = ".";
>  	char *work_dir = NULL;
> -	static const char short_opts[] = "dsRf:F:t:p:hcD:o:n:v::xVr:jlW:L:M:";
> +	static const char short_opts[] = "dSsRf:F:t:p:hcD:o:n:v::xVr:jlW:L:M:";
>  	static struct option long_opts[] = {
>  		{ "tree", required_argument, 0, 't' },
>  		{ "pid", required_argument, 0, 'p' },
>  		{ "leave-stopped", no_argument, 0, 's' },
>  		{ "leave-running", no_argument, 0, 'R' },
>  		{ "restore-detached", no_argument, 0, 'd' },
> +		{ "restore-sibling", no_argument, 0, 'S' },
>  		{ "daemon", no_argument, 0, 'd' },
>  		{ "contents", no_argument, 0, 'c' },
>  		{ "file", required_argument, 0, 'f' },
> @@ -247,6 +248,9 @@ int main(int argc, char *argv[], char *envp[])
>  		case 'd':
>  			opts.restore_detach = true;
>  			break;
> +		case 'S':
> +			opts.restore_sibling = true;
> +			break;
>  		case 'D':
>  			imgs_dir = optarg;
>  			break;
> @@ -404,6 +408,11 @@ int main(int argc, char *argv[], char *envp[])
>  		}
>  	}
>  
> +	if (!opts.restore_detach && opts.restore_sibling) {
> +		pr_msg("--restore-sibling only makes sense with --restore-detach\n");
> +		return 1;
> +	}
> +
>  	if (work_dir == NULL)
>  		work_dir = imgs_dir;
>  
> @@ -540,6 +549,7 @@ usage:
>  "* Generic:\n"
>  "  -t|--tree PID         checkpoint a process tree identified by PID\n"
>  "  -d|--restore-detached detach after restore\n"
> +"  -S|--restore-sibling  restore root task as sibling\n"
>  "  -s|--leave-stopped    leave tasks in stopped state after checkpoint\n"
>  "  -R|--leave-running    leave tasks in running state after checkpoint\n"
>  "  -D|--images-dir DIR   directory for image files\n"
> diff --git a/include/cr_options.h b/include/cr_options.h
> index e00d075..5a7edd9 100644
> --- a/include/cr_options.h
> +++ b/include/cr_options.h
> @@ -24,6 +24,7 @@ struct cr_options {
>  	bool			check_ms_kernel;
>  	bool			show_pages_content;
>  	bool			restore_detach;
> +	bool			restore_sibling;
>  	bool			ext_unix_sk;
>  	bool			shell_job;
>  	bool			handle_file_locks;
> diff --git a/lib/criu.c b/lib/criu.c
> index c83c4d8..ad419ba 100644
> --- a/lib/criu.c
> +++ b/lib/criu.c
> @@ -666,6 +666,9 @@ int criu_restore_child(void)
>  	req.type	= CRIU_REQ_TYPE__RESTORE;
>  	req.opts	= opts;
>  
> +	req.opts->has_rst_sibling = true;
> +	req.opts->rst_sibling = true;
> +
>  	ret = send_req_and_recv_resp_sk(sks[0], &req, &resp);
>  
>  	close(sks[0]);
> diff --git a/protobuf/rpc.proto b/protobuf/rpc.proto
> index 8ace89c..4617e7b 100644
> --- a/protobuf/rpc.proto
> +++ b/protobuf/rpc.proto
> @@ -53,6 +53,8 @@ message criu_opts {
>  	repeated ext_mount_map		ext_mnt		= 23;
>  	optional bool			manage_cgroups	= 24;
>  	repeated cgroup_root		cg_root		= 25;
> +
> +	optional bool			rst_sibling	= 26; /* swrk only */
>  }
>  
>  message criu_dump_resp {
> 
>