[CRIU] [PATCH] add join-ns opt to criu restore

[Pavel Emelyanov]

On 03/15/2016 09:08 AM, Dengguangxing wrote:
> 
> join-ns will restore process with specified existing namespace.
> This opt can be used in this fomat:
> 	--join-ns NS:PID, for example net:12345.
> 
> pid namespaces is not supported yet. As fork() is needed to make
> new pid-namespace take effect. That makes it hard for criu to track the
> child-process through pid because another child process has been
> created after fork().

Thanks for the great work :)
btw, this fixes issue #109 on github, doesn't it?

Please, find my comments inline.

> Signed-off-by: Deng Guangxing <dengguangxing at huawei.com>
> ---
>  criu/cr-restore.c         |  5 +++
>  criu/crtools.c            | 44 ++++++++++++++++++++++++-
>  criu/include/cr_options.h |  1 +
>  criu/include/namespaces.h | 10 ++++++
>  criu/namespaces.c         | 82 ++++++++++++++++++++++++++++++++++++++++-------
>  5 files changed, 129 insertions(+), 13 deletions(-)
> 
> diff --git a/criu/cr-restore.c b/criu/cr-restore.c
> index 30ddff9..e3f0add 100644
> --- a/criu/cr-restore.c
> +++ b/criu/cr-restore.c
> @@ -1566,6 +1566,11 @@ static int restore_task_with_children(void *_arg)
> 
>  	/* Restore root task */
>  	if (current->parent == NULL) {
> +		if (join_namespaces()) {

There are three things to sort out:

1. What if root task lives in netns, but we ask for --join-ns net?
2. What if we pass --unshare net and --join-ns net?
3. What if we pass --empty-ns net and --join-ns net?

> +			pr_perror("Join namespaces failed");
> +			goto err;
> +		}
> +
>  		if (restore_finish_stage(CR_STATE_RESTORE_NS) < 0)
>  			goto err;
> 

> @@ -234,6 +269,7 @@ int main(int argc, char *argv[], char *envp[])
>  		{ "work-dir",			required_argument,	0, 'W'	},
>  		{ "log-file",			required_argument,	0, 'o'	},
>  		{ "namespaces",			required_argument,	0, 'n'	},
> +		{ "join-ns",			required_argument,	0, 'J'	},

Please, also fix the images/rpc.proto and criu/cr-service.c to support this option
in RPC API.

>  		{ "root",			required_argument,	0, 'r'	},
>  		{ USK_EXT_PARAM,		optional_argument,	0, 'x'	},
>  		{ "help",			no_argument,		0, 'h'	},

> @@ -73,28 +106,41 @@ bool check_ns_proc(struct fd_link *link)
>  int switch_ns(int pid, struct ns_desc *nd, int *rst)
>  {
>  	char buf[32];
> -	int nsfd;
> +	int nsfd, self_fd;
>  	int ret = -1;
> +	struct stat st, self_st;
> 
>  	nsfd = open_proc(pid, "ns/%s", nd->str);
>  	if (nsfd < 0) {
> -		pr_perror("Can't open ipcns file");
> +		pr_perror("Can't open ns file");
> +		goto err_ns;
> +	}
> +	if (fstat(nsfd, &st) == -1) {
> +		pr_perror("Can't get ns file stat");
>  		goto err_ns;
>  	}
> 
> +	snprintf(buf, sizeof(buf), "/proc/self/ns/%s", nd->str);
> +	self_fd = open(buf, O_RDONLY);
> +	if (self_fd < 0) {
> +		pr_perror("Can't open ns file");
> +		goto err_rst;
> +	}
> +	if (fstat(self_fd, &self_st) == -1) {
> +		pr_perror("Can't get ns file stat");
> +		goto err_ns;
> +	}
>  	if (rst) {
> -		snprintf(buf, sizeof(buf), "/proc/self/ns/%s", nd->str);
> -		*rst = open(buf, O_RDONLY);
> -		if (*rst < 0) {
> -			pr_perror("Can't open ns file");
> -			goto err_rst;
> -		}
> +		*rst = self_fd;
>  	}
> 
> -	ret = setns(nsfd, nd->cflag);
> -	if (ret < 0) {
> -		pr_perror("Can't setns %d/%s", pid, nd->str);
> -		goto err_set;
> +	//re-set the same user-ns would fail, check it here
> +	if (st.st_ino != self_st.st_ino) {
> +		ret = setns(nsfd, nd->cflag);
> +		if (ret < 0) {
> +			pr_perror("Can't setns %d/%s", pid, nd->str);
> +			goto err_set;
> +		}

Please describe the change in switch_ns(), it's unclear what you're
trying to achieve here.

>  	}
> 
>  	close(nsfd);

-- Pavel