[CRIU] [PATCH v3 2/4] mount: Add forced mounts sceleton

[Pavel Emelyanov]

On 07/13/2016 04:43 PM, Kirill Tkhai wrote:
> On 13.07.2016 15:46, Pavel Emelyanov wrote:
>> On 06/29/2016 02:49 PM, Kirill Tkhai wrote:
>>> Mechanism for mounts, which are forced by criu during dump stage.
>>>
>>> v2: New patch, splitted from v1's "[2/3] mount: Forced mount unmounted binfmt_misc to do not lost its content"
>>>     Cleanup for dump failed case is added
>>>
>>> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
>>> ---
>>>  criu/cr-dump.c       |    1 +
>>>  criu/include/mount.h |    1 +
>>>  criu/mount.c         |   76 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 78 insertions(+)
>>>
>>> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
>>> index 00d28e9..7609544 100644
>>> --- a/criu/cr-dump.c
>>> +++ b/criu/cr-dump.c
>>> @@ -1588,6 +1588,7 @@ static int cr_dump_finish(int ret)
>>>  	if (ret || post_dump_ret || opts.final_state == TASK_ALIVE) {
>>>  		network_unlock();
>>>  		delete_link_remaps();
>>> +		cleanup_forced_mounts();
>>>  	}
>>>  	pstree_switch_state(root_item,
>>>  			    (ret || post_dump_ret) ?
>>> diff --git a/criu/include/mount.h b/criu/include/mount.h
>>> index c7992ac..423b6d0 100644
>>> --- a/criu/include/mount.h
>>> +++ b/criu/include/mount.h
>>> @@ -127,5 +127,6 @@ extern int ext_mount_add(char *key, char *val);
>>>  extern int mntns_maybe_create_roots(void);
>>>  extern int read_mnt_ns_img(void);
>>>  extern void cleanup_mnt_ns(void);
>>> +extern void cleanup_forced_mounts(void);
>>>  
>>>  #endif /* __CR_MOUNT_H__ */
>>> diff --git a/criu/mount.c b/criu/mount.c
>>> index 55f248d..4a288e4 100644
>>> --- a/criu/mount.c
>>> +++ b/criu/mount.c
>>> @@ -41,6 +41,15 @@
>>>  #define LOG_PREFIX "mnt: "
>>>  
>>>  static struct fstype fstypes[];
>>> +static LIST_HEAD(forced_mounts_list);
>>> +
>>> +struct forced_mount {
>>> +	struct list_head list;
>>> +	unsigned int ns_id;
>>> +	unsigned int mnt_id;
>>> +	char *path;
>>> +	pid_t pid;
>>> +};
>>
>> Why separate structure? We have mount_info already, let's add
>> necessary flags on it.
> 
> Firstly, we add such mounts, when namespaces are not collected,
> and there are no any mount_infos populated. If we create a new
> mount_info, it will be terrible and crutch'ly to use it in
> mnt_ns collecting engine.
> 
> Secondly, the patch introduces some new fields, which are not
> need for common case. We need list entry to keep forced mounts
> order, and to cleanup them in proper way in case of dump failed.
> 
> We use separate structure for ghost files, and this simplifies
> logic, so why should we overload mountpoints another way?!
> I think we shouldn't.
>  
>>>  
>>>  int ext_mount_add(char *key, char *val)
>>>  {
>>> @@ -3685,4 +3694,71 @@ int dump_mnt_namespaces(void)
>>>  	return 0;
>>>  }
>>>  
>>> +static int add_forced_mount(pid_t pid, const char *path)
>>> +{
>>> +	struct forced_mount *fm;
>>> +	int fd, mnt_id, ret = 0;
>>> +	unsigned int ns_id;
>>> +
>>> +	if (read_ns_id(pid, &mnt_ns_desc, &ns_id) < 0 || !ns_id) {
>>
>> I don't get why we need to read this stuff from kernel, while we have all
>> the ns_id engine at hands. If you need to add binfmt_misc mountpoint, you
>> want it per-namespace, not per-task, don't you?
> 
> Which engine do you mean? Namespaces are not parced yet; no information at all.

Thus don't add forced mounts that early. Add them once namespace is parsed
and collected, you don't need this thing before.

> See next patch, and places where we add forced mounts.
> 
> Mounts is per-namespace, but you need a task to attach a ns. You can't save
> "mnt:[4026531840]" and attach to it when you want. If so, you should iterate
> over tasks list and look for one, who owns it. Linux doesn't provide a list
> of used namespaces.
>  
>>> +		pr_err("Can't read mnt_ns id\n");
>>> +		return -1;
>>> +	}
>>> +
>>> +	fm = xmalloc(sizeof(*fm));
>>> +	if (!fm)
>>> +		return -1;
>>> +
>>> +	fd = open(path, O_PATH|O_DIRECTORY);
>>> +	if (fd < 0) {
>>> +		pr_perror("Can't open %s\n", path);
>>> +		goto err_free;
>>> +	}
>>> +
>>> +	ret = get_fd_mntid(fd, &mnt_id);
>>> +	close(fd);
>>> +
>>> +	if (ret < 0) {
>>> +		pr_err("Can't get mnt_id of %s\n", path);
>>> +		goto err_free;
>>> +	}
>>> +
>>> +	fm->path = xstrdup(path);
>>> +	if (!fm->path)
>>> +		goto err_free;
>>> +
>>> +	fm->ns_id = ns_id;
>>> +	fm->mnt_id = mnt_id;
>>> +	fm->pid = pid;
>>> +	list_add(&fm->list, &forced_mounts_list);
>>> +
>>> +	return 0;
>>> +err_free:
>>> +	pr_err("Can't add forced mount\n");
>>> +	free(fm);
>>> +	return -1;
>>> +}
>>> +
>>> +void cleanup_forced_mounts(void)
>>> +{
>>> +	struct forced_mount *fm;
>>> +	int mnt_fd, ret;
>>> +
>>> +	list_for_each_entry(fm, &forced_mounts_list, list) {
>>> +		ret = switch_ns(fm->pid, &mnt_ns_desc, &mnt_fd);
>>> +		if (ret) {
>>> +			pr_err("Can't switch to pid's %u mnt_ns\n", fm->pid);
>>> +			continue;
>>> +		}
>>> +
>>> +		if (umount(fm->path) < 0)
>>> +			pr_perror("Can't umount forced mount %s\n", fm->path);
>>> +
>>> +		if (restore_ns(mnt_fd, &mnt_ns_desc)) {
>>> +			pr_err("cleanup_forced_mounts exiting with wrong mnt_ns\n");
>>> +			return;
>>> +		}
>>> +	}
>>> +}
>>> +
>>>  struct ns_desc mnt_ns_desc = NS_DESC_ENTRY(CLONE_NEWNS, "mnt");
>>>
>>> _______________________________________________
>>> CRIU mailing list
>>> CRIU at openvz.org
>>> https://lists.openvz.org/mailman/listinfo/criu
>>> .
>>>
>>
> .
>