[CRIU] [PATCH] criu: Add exec-cmd option.

Pavel Emelyanov xemul at parallels.com
Thu Mar 20 02:07:10 PDT 2014


On 03/20/2014 01:02 PM, Deyan Doychev wrote:
> On 03/20/2014 10:34 AM, Pavel Emelyanov wrote:
>> On 03/19/2014 09:39 PM, Deyan Doychev wrote:
>>
>>> diff --git a/cr-restore.c b/cr-restore.c
>>> index b352daa..e528e3c 100644
>>> --- a/cr-restore.c
>>> +++ b/cr-restore.c
>>> @@ -1600,6 +1600,9 @@ int cr_restore_tasks(void)
>>>  {
>>>  	int ret = -1;
>>>  
>>> +	if (opts.exec_cmd && daemon(1, 0))
>>> +		return -1;
>>> +
>> I'm not quite sure that daemonizing before doing the restore is
>> good idea. How will check the restore return code?
>>>  	if (cr_plugin_init())
>>>  		return -1;
>>>  
>>> diff --git a/cr-service.c b/cr-service.c
>>> index 46a1004..bdb7257 100644
>>> --- a/cr-service.c
>>> +++ b/cr-service.c
>>> @@ -265,6 +265,11 @@ static int setup_opts_from_req(int sk, CriuOpts *req)
>>>  	if (req->has_force_irmap)
>>>  		opts.force_irmap = req->force_irmap;
>>>  
>>> +	if (req->exec_cmd) {
>>> +		opts.exec_cmd = parse_exec_cmd(req->exec_cmd);
>>> +		opts.restore_detach = true;
>> Deyan, can you explain why detaching becomes implied with the exec
>> cmd is given? I can imagine the use case, when I run criu restore
>> from shell and ask it to exec some other binary, but do not want
>> it to become daemon afterwards. But I can be missing something.
> 
> Now as you're asking I think it should not be implied.
> 
> I thought it should be because the exec-ed command has no clean option
> of becoming a daemon without losing its child. However you're right that
> we might need the new command to interact with the shell and stay on
> foreground.
> 
> Maybe I should change it to become a daemon only if -d is specified?

Yup, I think it makes sense. Thanks! :)

Thinking aloud: we have some options name conflict here :( The -d is short
for both -- the --restore-detached one and the --daemon one. Which one should
be considered as "go daemon after exec"? %)

Thanks,
Pavel


More information about the CRIU mailing list