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

[Pavel Emelyanov]

On 10/31/2013 04:55 PM, Andrew Vagin wrote:
> On Thu, Oct 31, 2013 at 04:31:32PM +0400, Pavel Emelyanov wrote:
>> 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.
> 
> Sure. But it can return negative value and criu will rollback all
> changes (e.g. unlock tcp connection) and all proccess will be resumed.
> In this case user take responsibility upon himself, that he make
> snapshot of file system, the he don't care about tcp connections and so on.

I don't think mixing "error" vs "OK, we can proceed" reports from script is
good idea.

Actually, the --link-remap thing should be rolled-back on error, but should
_not_ in case of -R + "OK" from script.

>>
>>>>
>>>>> 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;
>>>>>
>>>>
>>>>
>>> .
>>>
>>
>>
> .
>