[CRIU] [PATCH] options: Drop off --leave-running|-R option

[Pavel Emelyanov]

On 10/31/2013 04:16 PM, Andrew Vagin wrote:
> On Thu, Oct 31, 2013 at 04:09:14PM +0400, Pavel Emelyanov wrote:
>> On 10/31/2013 01:48 PM, Cyrill Gorcunov wrote:
>>> Leaving the task running is rather danger option
>>
>> NAK. It's in use for e.g. sequential dumps.
> 
> -R is dangerous. For example currently if a process with dumped with -R,
> all tcp connections remain in repair mode, so criu corrupts the process state...

Then we need to fix this. Not remove the option itself.
E.g. -- make -R conflict with --tcp-established.

> An user must undestand, that in case of sequential dumps he should make
> snapshot of file system and he will use post-dump scripts for that,
> so -R is not needed for him...

What do you mean "not needed"? Post-dump script has nothing to do with
process' state.

>>
>>> which may mislead a user, if dump is completed
>>> succesfully the task rather should be killed,
>>> otherwise if task continue running it might
>>> change own resources (files and etc) and the
>>> image on the disk won't match anymore, moreover
>>> TCP connection will be changed inside kernel
>>> so next "restore" action will fail for sure.
>>>
>>> Thus deprecate this option.
>>>
>>> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
>>> ---
>>>  cr-service.c       | 4 ----
>>>  crtools.c          | 4 ++--
>>>  protobuf/rpc.proto | 2 +-
>>>  3 files changed, 3 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/cr-service.c b/cr-service.c
>>> index 771944c..0ff80e2 100644
>>> --- a/cr-service.c
>>> +++ b/cr-service.c
>>> @@ -137,10 +137,6 @@ static int setup_opts_from_req(int sk, CriuOpts *req)
>>>  		return -1;
>>>  	}
>>>  
>>> -	/* checking flags from client */
>>> -	if (req->has_leave_running && req->leave_running)
>>> -		opts.final_state = TASK_ALIVE;
>>> -
>>>  	if (!req->has_pid) {
>>>  		req->has_pid = true;
>>>  		req->pid = ids.pid;
>>> diff --git a/crtools.c b/crtools.c
>>> index fe725bf..4ca141c 100644
>>> --- a/crtools.c
>>> +++ b/crtools.c
>>> @@ -131,7 +131,8 @@ int main(int argc, char *argv[])
>>>  			opts.final_state = TASK_STOPPED;
>>>  			break;
>>>  		case 'R':
>>> -			opts.final_state = TASK_ALIVE;
>>> +			pr_warn("The --leave-running|-R option is deprecated "
>>> +				"and will be removed in next release\n");
>>>  			break;
>>>  		case 'x':
>>>  			opts.ext_unix_sk = true;
>>> @@ -380,7 +381,6 @@ usage:
>>>  "  -t|--tree PID         checkpoint a process tree identified by PID\n"
>>>  "  -d|--restore-detached detach after restore\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"
>>>  "     --pidfile FILE     write a pid of a root task, service or page-server\n"
>>>  "                        to this file\n"
>>> diff --git a/protobuf/rpc.proto b/protobuf/rpc.proto
>>> index 5d1fbe7..510f699 100644
>>> --- a/protobuf/rpc.proto
>>> +++ b/protobuf/rpc.proto
>>> @@ -2,7 +2,7 @@ message criu_opts {
>>>  	required int32 images_dir_fd	= 1;
>>>  	optional int32 pid		= 2; //if not set on dump, will dump requesting process
>>>  
>>> -	optional bool leave_running	= 3;
>>> +	optional bool leave_running	= 3; /* Deprecated, ignoring */
>>>  	optional bool ext_unix_sk	= 4;
>>>  	optional bool tcp_established	= 5;
>>>  	optional bool evasive_devices	= 6;
>>>
>>
>>
> .
>