| Message ID | 149976635497.26819.17227174689058089242.stgit@localhost.localdomain |
|---|---|
| State | Accepted |
| Series | "ns: Implement setns_from_fdstore() for repeating code" |
| Commit | 7628eb27e83d9e56e18a8e229d7513d7a6040beb |
| Headers | show |
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();
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(); > >
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(-)