[CRIU] [PATCH 2/3] ns: Set NS_ROOT namespaces after tasks are read

[Kirill Tkhai]

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 at 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;
>>