| Message ID | 148828931242.1058.6938503600569029991.stgit@localhost.localdomain |
|---|---|
| State | New |
| Series | "Fix "NS_ROOT is not assigned"" |
| Headers | show |
diff --git a/criu/include/namespaces.h b/criu/include/namespaces.h index 2a3dc085b..05c08b52e 100644 --- a/criu/include/namespaces.h +++ b/criu/include/namespaces.h @@ -149,6 +149,7 @@ extern int collect_mnt_namespaces(bool for_dump); extern int dump_mnt_namespaces(void); extern int dump_namespaces(struct pstree_item *item, unsigned int ns_flags); extern int read_ns_with_hookups(void); +extern int set_ns_roots(void); extern int prepare_namespace_before_tasks(void); extern int prepare_namespace(struct pstree_item *item, unsigned long clone_flags); diff --git a/criu/namespaces.c b/criu/namespaces.c index b1e65df43..a92477df0 100644 --- a/criu/namespaces.c +++ b/criu/namespaces.c @@ -315,7 +315,6 @@ struct ns_id *rst_new_ns_id(unsigned int id, pid_t pid, int rst_add_ns_id(unsigned int id, struct pstree_item *i, struct ns_desc *nd) { pid_t pid = vpid(i); - int type = NS_OTHER; struct ns_id *nsid; nsid = lookup_ns_by_id(id, nd); @@ -325,9 +324,7 @@ int rst_add_ns_id(unsigned int id, struct pstree_item *i, struct ns_desc *nd) return 0; } - if (i == root_item && nd != &user_ns_desc) - type = NS_ROOT; - nsid = rst_new_ns_id(id, pid, nd, type); + nsid = rst_new_ns_id(id, pid, nd, NS_OTHER); if (nsid == NULL) return -1; @@ -1985,6 +1982,34 @@ int read_ns_with_hookups(void) return ret; } +static int mark_root_ns(uint32_t id, struct ns_desc *desc) +{ + struct ns_id *ns; + + ns = lookup_ns_by_id(id, desc); + if (!ns) { + pr_err("Can't find root ns %u, %s\n", id, desc->str); + return -1; + } + ns->type = NS_ROOT; + return 0; +} + +int set_ns_roots(void) +{ + TaskKobjIdsEntry *ids = root_item->ids; + + /* Set root for all namespaces except user_ns, which is set in read_ns_with_hookups() */ + if ((ids->has_pid_ns_id && mark_root_ns(ids->pid_ns_id, &pid_ns_desc) < 0) || + (ids->has_net_ns_id && mark_root_ns(ids->net_ns_id, &net_ns_desc) < 0) || + (ids->has_ipc_ns_id && mark_root_ns(ids->ipc_ns_id, &ipc_ns_desc) < 0) || + (ids->has_uts_ns_id && mark_root_ns(ids->uts_ns_id, &uts_ns_desc) < 0) || + (ids->has_mnt_ns_id && mark_root_ns(ids->mnt_ns_id, &mnt_ns_desc) < 0) || + (ids->has_cgroup_ns_id && mark_root_ns(ids->cgroup_ns_id, &cgroup_ns_desc) < 0)) + return -1; + return 0; +} + int prepare_userns(pid_t real_pid, UsernsEntry *e) { if (write_id_map(real_pid, e->uid_map, e->n_uid_map, "uid_map")) diff --git a/criu/pstree.c b/criu/pstree.c index dd40fbcee..b497e1989 100644 --- a/criu/pstree.c +++ b/criu/pstree.c @@ -9,6 +9,9 @@ #include "rst-malloc.h" #include "common/lock.h" #include "namespaces.h" +#include "cgroup.h" +#include "ipc_ns.h" +#include "uts_ns.h" #include "files.h" #include "tty.h" #include "mount.h" @@ -488,6 +491,18 @@ static int read_pstree_ids(struct pstree_item *pi) if (rst_add_ns_id(pi->ids->pid_ns_id, pi, &pid_ns_desc)) return -1; } + if (pi->ids->has_ipc_ns_id) { + if (rst_add_ns_id(pi->ids->ipc_ns_id, pi, &ipc_ns_desc)) + return -1; + } + if (pi->ids->has_uts_ns_id) { + if (rst_add_ns_id(pi->ids->uts_ns_id, pi, &uts_ns_desc)) + return -1; + } + if (pi->ids->has_cgroup_ns_id) { + if (rst_add_ns_id(pi->ids->cgroup_ns_id, pi, &cgroup_ns_desc)) + return -1; + } return 0; } @@ -596,6 +611,8 @@ static int read_pstree_image(pid_t *pid_max) goto err; } + if (ret == 0) + ret = set_ns_roots(); err: close_image(img); return ret;
On Tue, Feb 28, 2017 at 04:41:52PM +0300, Kirill Tkhai wrote: > Namespaces are read in read_ns_with_hookups(), > when tasks are not read. So, root_item is NULL, > and NS_ROOT is not set for appropriate namespaces. > > This patch fixes NS_ROOT after tasks are read. Also > it adds uts, ipc and cgroup namespaces for uniformity. > > Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com> > --- > criu/include/namespaces.h | 1 + > criu/namespaces.c | 33 +++++++++++++++++++++++++++++---- > criu/pstree.c | 17 +++++++++++++++++ > 3 files changed, 47 insertions(+), 4 deletions(-) > > diff --git a/criu/include/namespaces.h b/criu/include/namespaces.h > index 2a3dc085b..05c08b52e 100644 > --- a/criu/include/namespaces.h > +++ b/criu/include/namespaces.h > @@ -149,6 +149,7 @@ extern int collect_mnt_namespaces(bool for_dump); > extern int dump_mnt_namespaces(void); > extern int dump_namespaces(struct pstree_item *item, unsigned int ns_flags); > extern int read_ns_with_hookups(void); > +extern int set_ns_roots(void); > extern int prepare_namespace_before_tasks(void); > extern int prepare_namespace(struct pstree_item *item, unsigned long clone_flags); > > diff --git a/criu/namespaces.c b/criu/namespaces.c > index b1e65df43..a92477df0 100644 > --- a/criu/namespaces.c > +++ b/criu/namespaces.c > @@ -315,7 +315,6 @@ struct ns_id *rst_new_ns_id(unsigned int id, pid_t pid, > int rst_add_ns_id(unsigned int id, struct pstree_item *i, struct ns_desc *nd) > { > pid_t pid = vpid(i); > - int type = NS_OTHER; > struct ns_id *nsid; > > nsid = lookup_ns_by_id(id, nd); > @@ -325,9 +324,7 @@ int rst_add_ns_id(unsigned int id, struct pstree_item *i, struct ns_desc *nd) > return 0; > } Can we remove the hack with the fake pstree_entry? pid.ns[0].virt = -1; fake.pid = &pid; .... if (rst_add_ns_id(e->id, &fake, desc)) { pr_err("Can't add user ns\n"); goto close; } Can we call rst_new_ns_id instead of rst_add_ns_id in read_ns_with_hookups()? > > - if (i == root_item && nd != &user_ns_desc) > - type = NS_ROOT; > - nsid = rst_new_ns_id(id, pid, nd, type); > + nsid = rst_new_ns_id(id, pid, nd, NS_OTHER); > if (nsid == NULL) > return -1; > > @@ -1985,6 +1982,34 @@ int read_ns_with_hookups(void) > return ret; > } > > +static int mark_root_ns(uint32_t id, struct ns_desc *desc) > +{ > + struct ns_id *ns; > + > + ns = lookup_ns_by_id(id, desc); > + if (!ns) { > + pr_err("Can't find root ns %u, %s\n", id, desc->str); > + return -1; > + } > + ns->type = NS_ROOT; > + return 0; > +} > + > +int set_ns_roots(void) > +{ > + TaskKobjIdsEntry *ids = root_item->ids; > + > + /* Set root for all namespaces except user_ns, which is set in read_ns_with_hookups() */ > + if ((ids->has_pid_ns_id && mark_root_ns(ids->pid_ns_id, &pid_ns_desc) < 0) || > + (ids->has_net_ns_id && mark_root_ns(ids->net_ns_id, &net_ns_desc) < 0) || > + (ids->has_ipc_ns_id && mark_root_ns(ids->ipc_ns_id, &ipc_ns_desc) < 0) || > + (ids->has_uts_ns_id && mark_root_ns(ids->uts_ns_id, &uts_ns_desc) < 0) || > + (ids->has_mnt_ns_id && mark_root_ns(ids->mnt_ns_id, &mnt_ns_desc) < 0) || > + (ids->has_cgroup_ns_id && mark_root_ns(ids->cgroup_ns_id, &cgroup_ns_desc) < 0)) maybe we can add a marcos to make this code readable MARK_ROOT_NS(mnt); MARK_ROOT_NS(ipc); ... > + return -1; > + return 0; > +} > + > int prepare_userns(pid_t real_pid, UsernsEntry *e) > { > if (write_id_map(real_pid, e->uid_map, e->n_uid_map, "uid_map")) > diff --git a/criu/pstree.c b/criu/pstree.c > index dd40fbcee..b497e1989 100644 > --- a/criu/pstree.c > +++ b/criu/pstree.c > @@ -9,6 +9,9 @@ > #include "rst-malloc.h" > #include "common/lock.h" > #include "namespaces.h" > +#include "cgroup.h" > +#include "ipc_ns.h" > +#include "uts_ns.h" > #include "files.h" > #include "tty.h" > #include "mount.h" > @@ -488,6 +491,18 @@ static int read_pstree_ids(struct pstree_item *pi) > if (rst_add_ns_id(pi->ids->pid_ns_id, pi, &pid_ns_desc)) > return -1; > } > + if (pi->ids->has_ipc_ns_id) { > + if (rst_add_ns_id(pi->ids->ipc_ns_id, pi, &ipc_ns_desc)) > + return -1; > + } > + if (pi->ids->has_uts_ns_id) { > + if (rst_add_ns_id(pi->ids->uts_ns_id, pi, &uts_ns_desc)) > + return -1; > + } > + if (pi->ids->has_cgroup_ns_id) { > + if (rst_add_ns_id(pi->ids->cgroup_ns_id, pi, &cgroup_ns_desc)) > + return -1; > + } > > return 0; > } > @@ -596,6 +611,8 @@ static int read_pstree_image(pid_t *pid_max) > goto err; > } > > + if (ret == 0) > + ret = set_ns_roots(); > err: > close_image(img); > return ret; >
On 01.03.2017 03:57, Andrei Vagin wrote: > On Tue, Feb 28, 2017 at 04:41:52PM +0300, Kirill Tkhai wrote: >> Namespaces are read in read_ns_with_hookups(), >> when tasks are not read. So, root_item is NULL, >> and NS_ROOT is not set for appropriate namespaces. >> >> This patch fixes NS_ROOT after tasks are read. Also >> it adds uts, ipc and cgroup namespaces for uniformity. >> >> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com> >> --- >> criu/include/namespaces.h | 1 + >> criu/namespaces.c | 33 +++++++++++++++++++++++++++++---- >> criu/pstree.c | 17 +++++++++++++++++ >> 3 files changed, 47 insertions(+), 4 deletions(-) >> >> diff --git a/criu/include/namespaces.h b/criu/include/namespaces.h >> index 2a3dc085b..05c08b52e 100644 >> --- a/criu/include/namespaces.h >> +++ b/criu/include/namespaces.h >> @@ -149,6 +149,7 @@ extern int collect_mnt_namespaces(bool for_dump); >> extern int dump_mnt_namespaces(void); >> extern int dump_namespaces(struct pstree_item *item, unsigned int ns_flags); >> extern int read_ns_with_hookups(void); >> +extern int set_ns_roots(void); >> extern int prepare_namespace_before_tasks(void); >> extern int prepare_namespace(struct pstree_item *item, unsigned long clone_flags); >> >> diff --git a/criu/namespaces.c b/criu/namespaces.c >> index b1e65df43..a92477df0 100644 >> --- a/criu/namespaces.c >> +++ b/criu/namespaces.c >> @@ -315,7 +315,6 @@ struct ns_id *rst_new_ns_id(unsigned int id, pid_t pid, >> int rst_add_ns_id(unsigned int id, struct pstree_item *i, struct ns_desc *nd) >> { >> pid_t pid = vpid(i); >> - int type = NS_OTHER; >> struct ns_id *nsid; >> >> nsid = lookup_ns_by_id(id, nd); >> @@ -325,9 +324,7 @@ int rst_add_ns_id(unsigned int id, struct pstree_item *i, struct ns_desc *nd) >> return 0; >> } > > Can we remove the hack with the fake pstree_entry? > > pid.ns[0].virt = -1; > fake.pid = &pid; It's in the patch set; see [3/3]. > .... > if (rst_add_ns_id(e->id, &fake, desc)) { > pr_err("Can't add user ns\n"); > goto close; > } > > Can we call rst_new_ns_id instead of rst_add_ns_id in read_ns_with_hookups()? Yes, we can since read_ns_with_hookups() is called before tasks image, and ns should be uniqul. >> >> - if (i == root_item && nd != &user_ns_desc) >> - type = NS_ROOT; >> - nsid = rst_new_ns_id(id, pid, nd, type); >> + nsid = rst_new_ns_id(id, pid, nd, NS_OTHER); >> if (nsid == NULL) >> return -1; >> >> @@ -1985,6 +1982,34 @@ int read_ns_with_hookups(void) >> return ret; >> } >> >> +static int mark_root_ns(uint32_t id, struct ns_desc *desc) >> +{ >> + struct ns_id *ns; >> + >> + ns = lookup_ns_by_id(id, desc); >> + if (!ns) { >> + pr_err("Can't find root ns %u, %s\n", id, desc->str); >> + return -1; >> + } >> + ns->type = NS_ROOT; >> + return 0; >> +} >> + >> +int set_ns_roots(void) >> +{ >> + TaskKobjIdsEntry *ids = root_item->ids; >> + >> + /* Set root for all namespaces except user_ns, which is set in read_ns_with_hookups() */ >> + if ((ids->has_pid_ns_id && mark_root_ns(ids->pid_ns_id, &pid_ns_desc) < 0) || >> + (ids->has_net_ns_id && mark_root_ns(ids->net_ns_id, &net_ns_desc) < 0) || >> + (ids->has_ipc_ns_id && mark_root_ns(ids->ipc_ns_id, &ipc_ns_desc) < 0) || >> + (ids->has_uts_ns_id && mark_root_ns(ids->uts_ns_id, &uts_ns_desc) < 0) || >> + (ids->has_mnt_ns_id && mark_root_ns(ids->mnt_ns_id, &mnt_ns_desc) < 0) || >> + (ids->has_cgroup_ns_id && mark_root_ns(ids->cgroup_ns_id, &cgroup_ns_desc) < 0)) > > maybe we can add a marcos to make this code readable > > MARK_ROOT_NS(mnt); > MARK_ROOT_NS(ipc); > ... Yeah, sounds good. >> + return -1; >> + return 0; >> +} >> + >> int prepare_userns(pid_t real_pid, UsernsEntry *e) >> { >> if (write_id_map(real_pid, e->uid_map, e->n_uid_map, "uid_map")) >> diff --git a/criu/pstree.c b/criu/pstree.c >> index dd40fbcee..b497e1989 100644 >> --- a/criu/pstree.c >> +++ b/criu/pstree.c >> @@ -9,6 +9,9 @@ >> #include "rst-malloc.h" >> #include "common/lock.h" >> #include "namespaces.h" >> +#include "cgroup.h" >> +#include "ipc_ns.h" >> +#include "uts_ns.h" >> #include "files.h" >> #include "tty.h" >> #include "mount.h" >> @@ -488,6 +491,18 @@ static int read_pstree_ids(struct pstree_item *pi) >> if (rst_add_ns_id(pi->ids->pid_ns_id, pi, &pid_ns_desc)) >> return -1; >> } >> + if (pi->ids->has_ipc_ns_id) { >> + if (rst_add_ns_id(pi->ids->ipc_ns_id, pi, &ipc_ns_desc)) >> + return -1; >> + } >> + if (pi->ids->has_uts_ns_id) { >> + if (rst_add_ns_id(pi->ids->uts_ns_id, pi, &uts_ns_desc)) >> + return -1; >> + } >> + if (pi->ids->has_cgroup_ns_id) { >> + if (rst_add_ns_id(pi->ids->cgroup_ns_id, pi, &cgroup_ns_desc)) >> + return -1; >> + } >> >> return 0; >> } >> @@ -596,6 +611,8 @@ static int read_pstree_image(pid_t *pid_max) >> goto err; >> } >> >> + if (ret == 0) >> + ret = set_ns_roots(); >> err: >> close_image(img); >> return ret; >>
Namespaces are read in read_ns_with_hookups(), when tasks are not read. So, root_item is NULL, and NS_ROOT is not set for appropriate namespaces. This patch fixes NS_ROOT after tasks are read. Also it adds uts, ipc and cgroup namespaces for uniformity. Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com> --- criu/include/namespaces.h | 1 + criu/namespaces.c | 33 +++++++++++++++++++++++++++++---- criu/pstree.c | 17 +++++++++++++++++ 3 files changed, 47 insertions(+), 4 deletions(-)