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

[Kirill Tkhai]

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.
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
>> .
>>
>