[CRIU] [PATCH] criu: Add exec-cmd option (v2)

[Pavel Emelyanov]

On 03/20/2014 08:24 PM, Deyan Doychev wrote:
> From: Deyan Doychev <deyandoichev at gmail.com>
> 
> The --exec-cmd option specifies a command that will be execvp()-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 and it seems helpful
> for perf or other use cases when restored processes must be supervised by a
> parent.
> 
> Two directions were researched in order to integrate CRIU and LXC:
> 
> 1. We tell to CRIU, that after restoring container is should execve()
>    lxc properly explaining to it that there's a new container hanging
>    around.
> 
> 2. We make LXC set himself as child subreaper, then fork() criu and ask
>    it to detach (-d) from restore container afterwards. Being a subreaper,
>    it should get the container's init into his child list after it.
> 
> The main reason for choosing the first option is that the second one can't work
> with the RPC service. If we call restore via the service then criu service will
> be the top-most task in the hierarchy and will not be able to reparent the
> restore trees to any other task in the system. Calling execve from service
> worker sub-task (and daemonizing it) should solve this.
> 
> Signed-off-by: Deyan Doychev <deyandoichev at gmail.com>
> ---
>  cr-restore.c         |    5 ++++-
>  cr-service.c         |   22 ++++++++++++++++++++++
>  crtools.c            |   33 ++++++++++++++++++++++++++++++++-
>  include/cr_options.h |    1 +
>  lib/criu.c           |    5 +++++
>  protobuf/rpc.proto   |    1 +
>  6 files changed, 65 insertions(+), 2 deletions(-)
> 
> diff --git a/cr-restore.c b/cr-restore.c
> index b352daa..6b3bf10 100644
> --- a/cr-restore.c
> +++ b/cr-restore.c
> @@ -1552,7 +1552,7 @@ static int restore_root_task(struct pstree_item *init)
>  
>  	write_stats(RESTORE_STATS);
>  
> -	if (!opts.restore_detach)
> +	if (!opts.restore_detach && !opts.exec_cmd)

I feel that I'm missing something, but why is the check for
!opts.exec_cmd is needed here?

>  		wait(NULL);
>  
>  	return 0;
> @@ -1600,6 +1600,9 @@ int cr_restore_tasks(void)
>  {
>  	int ret = -1;
>  
> +	if (opts.exec_cmd && opts.restore_detach && daemon(1, 0))

In that case we will lose the ability to get error code from failed restore.
Can we move the daemonizing to the very end of the restore procedure?

> +		return -1;
> +
>  	if (cr_plugin_init())
>  		return -1;
>  

> @@ -37,6 +37,7 @@ message criu_opts {
>  
>  	optional uint32			cpu_cap		= 20 [default = 0xffffffff];
>  	optional bool			force_irmap	= 21;
> +	optional string			exec_cmd	= 22;

Maybe it's better to make this field repeated to allow for spaces in
arguments and to simplify the parsing (no strtoks would be required)?

>  }
>  
>  message criu_dump_resp {
>