ns: Implement setns_from_fdstore() for repeating code

Submitted by Kirill Tkhai on July 11, 2017, 9:47 a.m.

Details

Message ID 149976635497.26819.17227174689058089242.stgit@localhost.localdomain
State Accepted
Series "ns: Implement setns_from_fdstore() for repeating code"
Commit 7628eb27e83d9e56e18a8e229d7513d7a6040beb
Headers show

Commit Message

Kirill Tkhai July 11, 2017, 9:47 a.m.
Introduce a helper and use it instead of repeating code.
Use file and line of caller in error message printing
to allow the caller do not use additional print.

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 criu/cr-restore.c         |   31 ++++---------------------------
 criu/include/namespaces.h |    5 +++++
 criu/mount.c              |   23 +----------------------
 criu/namespaces.c         |   40 ++++++++++++++++++++++++++++------------
 criu/net.c                |   28 ++++------------------------
 criu/sockets.c            |   12 +++---------
 6 files changed, 45 insertions(+), 94 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/cr-restore.c b/criu/cr-restore.c
index e14fa0694..a9dacce8f 100644
--- a/criu/cr-restore.c
+++ b/criu/cr-restore.c
@@ -431,8 +431,6 @@  static void wait_pid_ns_helper_prepared(struct ns_id *pid_ns, struct pid *pid)
 
 static int set_pid_for_children_ns(struct ns_id *pid_ns)
 {
-	int fd, ret = 0;
-
 	if (!(root_ns_mask & CLONE_NEWPID))
 		return 0;
 
@@ -441,21 +439,11 @@  static int set_pid_for_children_ns(struct ns_id *pid_ns)
 	if (current->pid_for_children_ns == pid_ns)
 		return 0;
 
-	fd = fdstore_get(pid_ns->pid.nsfd_id);
-	if (fd < 0) {
-		pr_err("Can't get pid_ns fd\n");
+	if (setns_from_fdstore(pid_ns->pid.nsfd_id, CLONE_NEWPID) < 0)
 		return -1;
-	}
 
-	if (setns(fd, CLONE_NEWPID) < 0) {
-		pr_perror("Can't set pid ns");
-		ret = -1;
-	} else	{
-		current->pid_for_children_ns = pid_ns;
-	}
-
-	close(fd);
-	return ret;
+	current->pid_for_children_ns = pid_ns;
+	return 0;
 }
 
 static int restore_task_pfc_before_user_ns(void)
@@ -1310,7 +1298,6 @@  static int call_clone_fn(void *arg)
 	struct cr_clone_arg *ca = arg;
 	struct ns_id *pid_ns;
 	pid_t pid;
-	int fd;
 
 	pid_ns = lookup_ns_by_id(ca->item->ids->pid_ns_id, &pid_ns_desc);
 	BUG_ON(!pid_ns);
@@ -1320,18 +1307,8 @@  static int call_clone_fn(void *arg)
 		return -1;
 	}
 
-	fd = fdstore_get(pid_ns->user_ns->user.nsfd_id);
-	if (fd < 0) {
-		pr_err("Can't get ns fd\n");
+	if (setns_from_fdstore(pid_ns->user_ns->user.nsfd_id, CLONE_NEWUSER))
 		return -1;
-	}
-
-	if (setns(fd, CLONE_NEWUSER) < 0) {
-		pr_perror("Can't set user ns");
-		close(fd);
-		return -1;
-	}
-	close(fd);
 
 	close_pid_proc();
 	pid = clone_noasan(restore_task_with_children, ca->clone_flags | CLONE_PARENT | SIGCHLD, ca);
diff --git a/criu/include/namespaces.h b/criu/include/namespaces.h
index 44520a81a..bc7372c22 100644
--- a/criu/include/namespaces.h
+++ b/criu/include/namespaces.h
@@ -264,6 +264,11 @@  extern int __userns_call(const char *func_name, uns_call_t call, int flags,
 	__userns_call(__stringify(__call), __call, __flags,	\
 		      __arg, __arg_size, __fd)
 
+extern int __setns_from_fdstore(int fd_id, int nstype, const char *, int);
+
+#define setns_from_fdstore(fd_id, nstype)			\
+	__setns_from_fdstore(fd_id, nstype, __FILE__, __LINE__)
+
 extern int add_ns_shared_cb(int (*actor)(void *data), void *data);
 
 extern struct ns_id *get_socket_ns(int lfd);
diff --git a/criu/mount.c b/criu/mount.c
index 83915b7b2..b825b7086 100644
--- a/criu/mount.c
+++ b/criu/mount.c
@@ -2713,24 +2713,6 @@  int mntns_maybe_create_roots(void)
 	return create_mnt_roots();
 }
 
-static int do_restore_task_mnt_ns(struct ns_id *nsid, struct pstree_item *current)
-{
-	int fd;
-
-	fd = fdstore_get(nsid->mnt.nsfd_id);
-	if (fd < 0)
-		return -1;
-
-	if (setns(fd, CLONE_NEWNS)) {
-		pr_perror("Can't restore mntns");
-		close(fd);
-		return -1;
-	}
-	close(fd);
-
-	return 0;
-}
-
 int restore_task_mnt_ns(struct pstree_item *current)
 {
 	unsigned int id = current->ids->mnt_ns_id;
@@ -2758,10 +2740,7 @@  int restore_task_mnt_ns(struct pstree_item *current)
 
 	BUG_ON(nsid->type == NS_CRIU);
 
-	if (do_restore_task_mnt_ns(nsid, current))
-		return -1;
-
-	return 0;
+	return setns_from_fdstore(nsid->mnt.nsfd_id, CLONE_NEWNS);
 }
 
 void fini_restore_mntns(void)
diff --git a/criu/namespaces.c b/criu/namespaces.c
index 100a631e9..127a202d6 100644
--- a/criu/namespaces.c
+++ b/criu/namespaces.c
@@ -2553,26 +2553,16 @@  int prepare_namespace_before_tasks(void)
 
 int __set_user_ns(struct ns_id *ns)
 {
-	int fd;
-
 	if (!(root_ns_mask & CLONE_NEWUSER))
 		return 0;
 
 	if (current->user_ns && current->user_ns->id == ns->id)
 		return 0;
 
-	fd = fdstore_get(ns->user.nsfd_id);
-	if (fd < 0) {
-		pr_err("Can't get ns fd\n");
+	if (setns_from_fdstore(ns->user.nsfd_id, CLONE_NEWUSER))
 		return -1;
-	}
-	if (setns(fd, CLONE_NEWUSER) < 0) {
-		pr_perror("Can't setns");
-		close(fd);
-		return -1;
-	}
+
 	current->user_ns = ns;
-	close(fd);
 
 	if (prepare_userns_creds() < 0) {
 		pr_err("Can't set creds\n");
@@ -2834,5 +2824,31 @@  int destroy_pid_ns_helpers(void)
 	return 0;
 }
 
+int __setns_from_fdstore(int fd_id, int nstype, const char *file, int line)
+{
+	int fd, saved_errno, ret;
+
+	fd = fdstore_get(fd_id);
+	if (fd < 0)
+		goto err;
+
+	ret = setns(fd, nstype);
+	saved_errno = errno;
+	close(fd);
+	if (ret) {
+		errno = saved_errno;
+		pr_perror("Can't set user ns");
+		goto err;
+	}
+
+	return 0;
+err:
+	pr_err("Can't set %s_ns from fdstore (called from %s: %d)\n",
+		ns_to_string(nstype), file, line);
+	return -1;
+}
+
+
+
 struct ns_desc pid_ns_desc = NS_DESC_ENTRY(CLONE_NEWPID, "pid", "pid_for_children");
 struct ns_desc user_ns_desc = NS_DESC_ENTRY(CLONE_NEWUSER, "user", NULL);
diff --git a/criu/net.c b/criu/net.c
index 15aae2be4..99a78ee14 100644
--- a/criu/net.c
+++ b/criu/net.c
@@ -2060,23 +2060,16 @@  static int do_create_net_ns(struct ns_id *ns)
 static int create_net_ns(void *arg)
 {
 	struct ns_id *uns, *ns = arg;
-	int ufd, ret;
+	int ret;
 
 	uns = ns->user_ns;
-	ufd = fdstore_get(uns->user.nsfd_id);
-	if (ufd < 0) {
-		pr_err("Can't get user ns\n");
+	if (setns_from_fdstore(uns->user.nsfd_id, CLONE_NEWUSER))
 		exit(1);
-	}
-	if (setns(ufd, CLONE_NEWUSER) < 0) {
-		pr_perror("Can't set user ns");
-		exit(2);
-	}
+
 	if (prepare_userns_creds() < 0) {
 		pr_err("Can't prepare creds\n");
 		exit(3);
 	}
-	close(ufd);
 	ret = do_create_net_ns(ns) ? 3 : 0;
 	exit(ret);
 }
@@ -2160,23 +2153,10 @@  int prepare_net_namespaces(void)
 
 static int do_restore_task_net_ns(struct ns_id *nsid, struct pstree_item *current)
 {
-	int fd;
-
 	if (!(root_ns_mask & CLONE_NEWNET))
 		return 0;
 
-	fd = fdstore_get(nsid->net.nsfd_id);
-	if (fd < 0)
-		return -1;
-
-	if (setns(fd, CLONE_NEWNET)) {
-		pr_perror("Can't restore netns");
-		close(fd);
-		return -1;
-	}
-	close(fd);
-
-	return 0;
+	return setns_from_fdstore(nsid->net.nsfd_id, CLONE_NEWNET);
 }
 
 int restore_task_net_ns(struct pstree_item *current)
diff --git a/criu/sockets.c b/criu/sockets.c
index 9b0c4df99..fd6f525a0 100644
--- a/criu/sockets.c
+++ b/criu/sockets.c
@@ -750,7 +750,6 @@  static uint32_t last_ns_id = 0;
 int set_netns(uint32_t ns_id)
 {
 	struct ns_id *ns;
-	int nsfd;
 
 	if (!(root_ns_mask & CLONE_NEWNET))
 		return 0;
@@ -763,16 +762,11 @@  int set_netns(uint32_t ns_id)
 		pr_err("Unable to find a network namespace\n");
 		return -1;
 	}
-	nsfd = fdstore_get(ns->net.nsfd_id);
-	if (nsfd < 0)
-		return -1;
-	if (setns(nsfd, CLONE_NEWNET)) {
-		pr_perror("Unable to switch a network namespace");
-		close(nsfd);
+
+	if (setns_from_fdstore(ns->net.nsfd_id, CLONE_NEWNET))
 		return -1;
-	}
+
 	last_ns_id = ns_id;
-	close(nsfd);
 
 	close_pid_proc();
 

Comments

Andrey Vagin July 22, 2017, 3:38 a.m.
Applied, thanks
On Tue, Jul 11, 2017 at 12:47:37PM +0300, Kirill Tkhai wrote:
> Introduce a helper and use it instead of repeating code.
> Use file and line of caller in error message printing
> to allow the caller do not use additional print.
> 
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
>  criu/cr-restore.c         |   31 ++++---------------------------
>  criu/include/namespaces.h |    5 +++++
>  criu/mount.c              |   23 +----------------------
>  criu/namespaces.c         |   40 ++++++++++++++++++++++++++++------------
>  criu/net.c                |   28 ++++------------------------
>  criu/sockets.c            |   12 +++---------
>  6 files changed, 45 insertions(+), 94 deletions(-)
> 
> diff --git a/criu/cr-restore.c b/criu/cr-restore.c
> index e14fa0694..a9dacce8f 100644
> --- a/criu/cr-restore.c
> +++ b/criu/cr-restore.c
> @@ -431,8 +431,6 @@ static void wait_pid_ns_helper_prepared(struct ns_id *pid_ns, struct pid *pid)
>  
>  static int set_pid_for_children_ns(struct ns_id *pid_ns)
>  {
> -	int fd, ret = 0;
> -
>  	if (!(root_ns_mask & CLONE_NEWPID))
>  		return 0;
>  
> @@ -441,21 +439,11 @@ static int set_pid_for_children_ns(struct ns_id *pid_ns)
>  	if (current->pid_for_children_ns == pid_ns)
>  		return 0;
>  
> -	fd = fdstore_get(pid_ns->pid.nsfd_id);
> -	if (fd < 0) {
> -		pr_err("Can't get pid_ns fd\n");
> +	if (setns_from_fdstore(pid_ns->pid.nsfd_id, CLONE_NEWPID) < 0)
>  		return -1;
> -	}
>  
> -	if (setns(fd, CLONE_NEWPID) < 0) {
> -		pr_perror("Can't set pid ns");
> -		ret = -1;
> -	} else	{
> -		current->pid_for_children_ns = pid_ns;
> -	}
> -
> -	close(fd);
> -	return ret;
> +	current->pid_for_children_ns = pid_ns;
> +	return 0;
>  }
>  
>  static int restore_task_pfc_before_user_ns(void)
> @@ -1310,7 +1298,6 @@ static int call_clone_fn(void *arg)
>  	struct cr_clone_arg *ca = arg;
>  	struct ns_id *pid_ns;
>  	pid_t pid;
> -	int fd;
>  
>  	pid_ns = lookup_ns_by_id(ca->item->ids->pid_ns_id, &pid_ns_desc);
>  	BUG_ON(!pid_ns);
> @@ -1320,18 +1307,8 @@ static int call_clone_fn(void *arg)
>  		return -1;
>  	}
>  
> -	fd = fdstore_get(pid_ns->user_ns->user.nsfd_id);
> -	if (fd < 0) {
> -		pr_err("Can't get ns fd\n");
> +	if (setns_from_fdstore(pid_ns->user_ns->user.nsfd_id, CLONE_NEWUSER))
>  		return -1;
> -	}
> -
> -	if (setns(fd, CLONE_NEWUSER) < 0) {
> -		pr_perror("Can't set user ns");
> -		close(fd);
> -		return -1;
> -	}
> -	close(fd);
>  
>  	close_pid_proc();
>  	pid = clone_noasan(restore_task_with_children, ca->clone_flags | CLONE_PARENT | SIGCHLD, ca);
> diff --git a/criu/include/namespaces.h b/criu/include/namespaces.h
> index 44520a81a..bc7372c22 100644
> --- a/criu/include/namespaces.h
> +++ b/criu/include/namespaces.h
> @@ -264,6 +264,11 @@ extern int __userns_call(const char *func_name, uns_call_t call, int flags,
>  	__userns_call(__stringify(__call), __call, __flags,	\
>  		      __arg, __arg_size, __fd)
>  
> +extern int __setns_from_fdstore(int fd_id, int nstype, const char *, int);
> +
> +#define setns_from_fdstore(fd_id, nstype)			\
> +	__setns_from_fdstore(fd_id, nstype, __FILE__, __LINE__)
> +
>  extern int add_ns_shared_cb(int (*actor)(void *data), void *data);
>  
>  extern struct ns_id *get_socket_ns(int lfd);
> diff --git a/criu/mount.c b/criu/mount.c
> index 83915b7b2..b825b7086 100644
> --- a/criu/mount.c
> +++ b/criu/mount.c
> @@ -2713,24 +2713,6 @@ int mntns_maybe_create_roots(void)
>  	return create_mnt_roots();
>  }
>  
> -static int do_restore_task_mnt_ns(struct ns_id *nsid, struct pstree_item *current)
> -{
> -	int fd;
> -
> -	fd = fdstore_get(nsid->mnt.nsfd_id);
> -	if (fd < 0)
> -		return -1;
> -
> -	if (setns(fd, CLONE_NEWNS)) {
> -		pr_perror("Can't restore mntns");
> -		close(fd);
> -		return -1;
> -	}
> -	close(fd);
> -
> -	return 0;
> -}
> -
>  int restore_task_mnt_ns(struct pstree_item *current)
>  {
>  	unsigned int id = current->ids->mnt_ns_id;
> @@ -2758,10 +2740,7 @@ int restore_task_mnt_ns(struct pstree_item *current)
>  
>  	BUG_ON(nsid->type == NS_CRIU);
>  
> -	if (do_restore_task_mnt_ns(nsid, current))
> -		return -1;
> -
> -	return 0;
> +	return setns_from_fdstore(nsid->mnt.nsfd_id, CLONE_NEWNS);
>  }
>  
>  void fini_restore_mntns(void)
> diff --git a/criu/namespaces.c b/criu/namespaces.c
> index 100a631e9..127a202d6 100644
> --- a/criu/namespaces.c
> +++ b/criu/namespaces.c
> @@ -2553,26 +2553,16 @@ int prepare_namespace_before_tasks(void)
>  
>  int __set_user_ns(struct ns_id *ns)
>  {
> -	int fd;
> -
>  	if (!(root_ns_mask & CLONE_NEWUSER))
>  		return 0;
>  
>  	if (current->user_ns && current->user_ns->id == ns->id)
>  		return 0;
>  
> -	fd = fdstore_get(ns->user.nsfd_id);
> -	if (fd < 0) {
> -		pr_err("Can't get ns fd\n");
> +	if (setns_from_fdstore(ns->user.nsfd_id, CLONE_NEWUSER))
>  		return -1;
> -	}
> -	if (setns(fd, CLONE_NEWUSER) < 0) {
> -		pr_perror("Can't setns");
> -		close(fd);
> -		return -1;
> -	}
> +
>  	current->user_ns = ns;
> -	close(fd);
>  
>  	if (prepare_userns_creds() < 0) {
>  		pr_err("Can't set creds\n");
> @@ -2834,5 +2824,31 @@ int destroy_pid_ns_helpers(void)
>  	return 0;
>  }
>  
> +int __setns_from_fdstore(int fd_id, int nstype, const char *file, int line)
> +{
> +	int fd, saved_errno, ret;
> +
> +	fd = fdstore_get(fd_id);
> +	if (fd < 0)
> +		goto err;
> +
> +	ret = setns(fd, nstype);
> +	saved_errno = errno;
> +	close(fd);
> +	if (ret) {
> +		errno = saved_errno;
> +		pr_perror("Can't set user ns");
> +		goto err;
> +	}
> +
> +	return 0;
> +err:
> +	pr_err("Can't set %s_ns from fdstore (called from %s: %d)\n",
> +		ns_to_string(nstype), file, line);
> +	return -1;
> +}
> +
> +
> +
>  struct ns_desc pid_ns_desc = NS_DESC_ENTRY(CLONE_NEWPID, "pid", "pid_for_children");
>  struct ns_desc user_ns_desc = NS_DESC_ENTRY(CLONE_NEWUSER, "user", NULL);
> diff --git a/criu/net.c b/criu/net.c
> index 15aae2be4..99a78ee14 100644
> --- a/criu/net.c
> +++ b/criu/net.c
> @@ -2060,23 +2060,16 @@ static int do_create_net_ns(struct ns_id *ns)
>  static int create_net_ns(void *arg)
>  {
>  	struct ns_id *uns, *ns = arg;
> -	int ufd, ret;
> +	int ret;
>  
>  	uns = ns->user_ns;
> -	ufd = fdstore_get(uns->user.nsfd_id);
> -	if (ufd < 0) {
> -		pr_err("Can't get user ns\n");
> +	if (setns_from_fdstore(uns->user.nsfd_id, CLONE_NEWUSER))
>  		exit(1);
> -	}
> -	if (setns(ufd, CLONE_NEWUSER) < 0) {
> -		pr_perror("Can't set user ns");
> -		exit(2);
> -	}
> +
>  	if (prepare_userns_creds() < 0) {
>  		pr_err("Can't prepare creds\n");
>  		exit(3);
>  	}
> -	close(ufd);
>  	ret = do_create_net_ns(ns) ? 3 : 0;
>  	exit(ret);
>  }
> @@ -2160,23 +2153,10 @@ int prepare_net_namespaces(void)
>  
>  static int do_restore_task_net_ns(struct ns_id *nsid, struct pstree_item *current)
>  {
> -	int fd;
> -
>  	if (!(root_ns_mask & CLONE_NEWNET))
>  		return 0;
>  
> -	fd = fdstore_get(nsid->net.nsfd_id);
> -	if (fd < 0)
> -		return -1;
> -
> -	if (setns(fd, CLONE_NEWNET)) {
> -		pr_perror("Can't restore netns");
> -		close(fd);
> -		return -1;
> -	}
> -	close(fd);
> -
> -	return 0;
> +	return setns_from_fdstore(nsid->net.nsfd_id, CLONE_NEWNET);
>  }
>  
>  int restore_task_net_ns(struct pstree_item *current)
> diff --git a/criu/sockets.c b/criu/sockets.c
> index 9b0c4df99..fd6f525a0 100644
> --- a/criu/sockets.c
> +++ b/criu/sockets.c
> @@ -750,7 +750,6 @@ static uint32_t last_ns_id = 0;
>  int set_netns(uint32_t ns_id)
>  {
>  	struct ns_id *ns;
> -	int nsfd;
>  
>  	if (!(root_ns_mask & CLONE_NEWNET))
>  		return 0;
> @@ -763,16 +762,11 @@ int set_netns(uint32_t ns_id)
>  		pr_err("Unable to find a network namespace\n");
>  		return -1;
>  	}
> -	nsfd = fdstore_get(ns->net.nsfd_id);
> -	if (nsfd < 0)
> -		return -1;
> -	if (setns(nsfd, CLONE_NEWNET)) {
> -		pr_perror("Unable to switch a network namespace");
> -		close(nsfd);
> +
> +	if (setns_from_fdstore(ns->net.nsfd_id, CLONE_NEWNET))
>  		return -1;
> -	}
> +
>  	last_ns_id = ns_id;
> -	close(nsfd);
>  
>  	close_pid_proc();
>  
>