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

[Pavel Emelyanov]

On 09/10/2014 05:13 PM, Tycho Andersen wrote:
> Hi Pavel,
> 
> 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:
> 
> Acked-by: Tycho Andersen <tycho.andersen at canonical.com>

Thanks :)

BTW, do we need to make 1.3.1 with this key? To make LXC work with 1.3 as well.

>> ---
>>
>> 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");
>> +		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 {
>>
>>
> .
>