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

[Deyan Doychev]

Hi Christopher,

Thanks for the feedback.

On 03/19/2014 10:41 PM, Christopher Covington wrote:
> Hi Deyan,
>
> On 03/19/2014 01:39 PM, Deyan Doychev wrote:
>> From: Deyan Doychev <deyandoichev at gmail.com>
>>
>> The --exec-cmd option specifies a command that will be execve()-ed on successful
>> restore. This way the command specified here will become the parent process of
>> the restored process tree.
>>
>> When this option is specified criu will fork to become a daemon before it starts
>> restoring the processes. It also implies the -d option so waiting for the
>> restored processes to finish is responsibility of the command specified here.
>>
>> This option will be used when restoring LinuX Containers.
> I think it might be useful to document why you've chosen this implementation
> over the subreaper option (even if the answer is simple). I'm imagining that
> one possible benefit of the subreaper option is that the exec'd command could
> have its own standard I/O file descriptors. CRIU has -o, though, so I don't
> know if sharing would really be a problem. The environment is shared too,
> right? But again, I don't know if that'd really be a problem.

OK, I will add some reasons to the message.

>> diff --git a/crtools.c b/crtools.c
>> index 047ac53..5591225 100644
>> --- a/crtools.c
>> +++ b/crtools.c
>> @@ -390,7 +412,16 @@ int main(int argc, char *argv[])
>>  	if (!strcmp(argv[optind], "restore")) {
>>  		if (tree_id)
>>  			pr_warn("Using -t with criu restore is obsoleted\n");
>> -		return cr_restore_tasks() != 0;
>> +
>> +		ret = cr_restore_tasks();
>> +		if (ret == 0 && opts.exec_cmd) {
>> +			extern char **environ;
>> +			execve(opts.exec_cmd[0], opts.exec_cmd, environ);
> Can you use execvp so that users don't have to specify an absolute path?

Yes, will resubmit.

>
>> +			pr_perror("Failed to exec command %s\n", opts.exec_cmd[0]);
>> +			ret = 1;
>> +		}
>> +
>> +		return ret != 0;
> In my testing, a non-zero exit code wasn't propagated to the command line on
> failure.

This is because the restore process becomes a daemon prior to restoring
the dumped process when --exec-cmd is used.

I am not sure what the right action has to be if we fail to execute the
command as we have already restored the processes.
Should we consider this a full failure and if so - should we kill the
processes we have restored?

Maybe the right thing  to do is daemonize only when -d was given instead
of implying this option and always daemonizing. This way if -d is not
specified we will exit with failure. But please advise what should we do
with the restored processes?

>>  	}
>>  
>>  	if (!strcmp(argv[optind], "show"))
> I think perf has the same general problem as lxc. While I've been able to
> develop other workarounds like using a third program to ptrace attach the
> restored process and essentially proxy the SIGCHLD something more
> straightforward would be nice. I tested this patch by running perf stat on a
> restored process in a 32-bit ARM simulator and it seemed to work.
>
> ./criu restore -v4 -o restore.log --exec-cmd "perf stat -v -p $pid -e
> instructions:u -o perf.log"
>
> Tested-by: Christopher Covington <cov at codeaurora.org>
>
> Regards,
> Christopher
>
> P.S. Apologies for not cross-posting.
>

Thanks,
Deyan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.openvz.org/pipermail/criu/attachments/20140320/265066fa/attachment.sig>