[CRIU] [PATCH v4 3/3] mount: Restore binfmt_misc content in case of missing mount

[Andrew Vagin]

On Tue, Aug 02, 2016 at 04:41:54PM +0300, Kirill Tkhai wrote:
> 
> 
> On 02.08.2016 16:39, Pavel Emelyanov wrote:
> > On 08/02/2016 04:29 PM, Kirill Tkhai wrote:
> >>
> >>
> >> On 02.08.2016 16:03, Pavel Emelyanov wrote:
> >>> On 07/20/2016 01:13 PM, Kirill Tkhai wrote:
> >>>> In case of mount image is not containing binfmt_misc mountpoint,
> >>>> scan image directory for binfmt_misc content file (binfmt-misc-xxx).
> >>>> If a file is found, add temporary mountpoint to mount tree and
> >>>> restore the content in ordinary way. Then, umount temporary mountpoint.
> >>>>
> >>>> v4: New
> >>>>
> >>>> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
> >>>> ---
> >>>>  criu/mount.c |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >>>>  1 file changed, 59 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/criu/mount.c b/criu/mount.c
> >>>> index d583429..8657c36 100644
> >>>> --- a/criu/mount.c
> >>>> +++ b/criu/mount.c
> >>>> @@ -1379,6 +1379,39 @@ static int try_mount_binfmt_misc(struct ns_id *ns, unsigned int *s_dev)
> >>>>  	return ret ? -1 : exit_code;
> >>>>  }
> >>>>  
> >>>> +static int find_binfmt_misc_img(unsigned int *s_dev)
> >>>> +{
> >>>> +	struct dirent *de;
> >>>> +	int dfd, ret;
> >>>> +	DIR *dir;
> >>>> +
> >>>> +	*s_dev = 0;
> >>>> +	dfd = dup(get_service_fd(IMG_FD_OFF));
> >>>> +	dir = fdopendir(dfd);
> >>>> +	if (!dir) {
> >>>> +		close(dfd);
> >>>> +		pr_perror("Can't open img dir\n");
> >>>> +		return -1;
> >>>> +	}
> >>>> +	rewinddir(dir);
> >>>> +
> >>>> +	errno = 0;
> >>>> +	while ((de = readdir(dir)) != NULL) {
> >>>
> >>> Scanning the images dir doesn't look valid. Instead, there should be a proper
> >>> collection of this image (like we do with fdinfo-s, sockets and others). Then,
> >>> when reading the mountpoint images, the binfmt_misc collected data should be
> >>> looked up and orphan data should add the cr-time mountpoints.
> >>
> >> So, for every type you want to make array of images, don't you?
> > 
> > What do you mean?
> 
> Introduce a new type of objects, "dump images". And collect names of images,
> like we do for other object types.
> 
> As I understand right, you want so.

        img = open_image(CR_FD_BINFMT_MISC, O_DUMP, pm->s_dev);

I think we choose a wrong format for an image name here.

binfmt_misc is a property of a container (the ve structure). If someone
will containerize it in the upstream kernel, it probably will be a
property of a mount namespace. I suggest to use mntns->id instead of
pm->s_dev for binfmt images.


> 
>  
> >>>
> >>>> +		if (strncmp(de->d_name, "binfmt-misc-", 12) == 0) {
> >>>> +			*s_dev = (unsigned int)strtoul(de->d_name + 12, NULL, 10);
> >>>> +			break;
> >>>> +		}
> >>>> +	}
> >>>> +	ret = errno;
> >>>> +	if (ret)
> >>>> +		pr_perror("Can't read dir\n");
> >>>> +	else if (*s_dev)
> >>>> +		pr_info("Found binfmt_misc img for dev=%u\n", *s_dev);
> >>>> +	closedir(dir);
> >>>> +
> >>>> +	return ret ? -1 : (*s_dev != 0);
> >>>> +}
> >>>> +
> >>>>  static int binfmt_misc_virtual(struct mount_info *pm)
> >>>>  {
> >>>>  	return kerndat_fs_virtualized(KERNDAT_FS_STAT_BINFMT_MISC, pm->s_dev);
> >>>> @@ -2442,6 +2475,15 @@ static int do_new_mount(struct mount_info *mi)
> >>>>  	if (tp->restore && tp->restore(mi))
> >>>>  		return -1;
> >>>>  
> >>>> +	if (!mi->mnt_id) {
> >>>> +		/* C-r time mountpoint, umount it */
> >>>> +		if (umount(mi->mountpoint) < 0) {
> >>>
> >>> I don't see the corresponding mount() callback. Where is it?
> >>
> >> Generic code does this after mount_info is created in add_cr_time_mount().
> > 
> > Ah! OK, we need a code comment saying this explicitly.
> > 
> >>>> +			pr_perror("Can't umount %s\n", mi->mountpoint);
> >>>> +			return -1;
> >>>> +		}
> >>>> +		goto out;
> >>>> +	}
> >>>> +
> >>>>  	if (remount_ro && mount(NULL, mi->mountpoint, tp->name,
> >>>>  				     MS_REMOUNT | MS_RDONLY, NULL)) {
> >>>>  		pr_perror("Unable to apply mount options");
> >>>> @@ -2461,7 +2503,7 @@ static int do_new_mount(struct mount_info *mi)
> >>>>  	BUG_ON(mi->master_id);
> >>>>  	if (restore_shared_options(mi, !mi->shared_id, mi->shared_id, 0))
> >>>>  		return -1;
> >>>> -
> >>>> +out:
> >>>>  	mi->mounted = true;
> >>>>  
> >>>>  	return 0;
> >>>> @@ -3324,7 +3366,7 @@ static int populate_roots_yard(void)
> >>>>  
> >>>>  static int populate_mnt_ns(void)
> >>>>  {
> >>>> -	struct mount_info *pms;
> >>>> +	struct mount_info *pms, *mi;
> >>>>  	struct ns_id *nsid;
> >>>>  	struct mount_info *roots_mp = NULL;
> >>>>  	int ret;
> >>>> @@ -3343,6 +3385,21 @@ static int populate_mnt_ns(void)
> >>>>  	if (!pms)
> >>>>  		return -1;
> >>>>  
> >>>> +	for (mi = mntinfo; mi != NULL; mi = mi->next) {
> >>>> +		if (strcmp(mi->fstype->name, "binfmt_misc") == 0)
> >>>> +			break;
> >>>> +	}
> >>>> +
> >>>> +	if (!mi) {
> >>>> +		unsigned int s_dev;
> >>>> +
> >>>> +		ret = find_binfmt_misc_img(&s_dev);
> >>>> +		if (ret > 0)
> >>>> +			ret = add_cr_time_mount(pms, "binfmt_misc", BINFMT_MISC_HOME, s_dev);
> >>>> +		if (ret < 0)
> >>>> +			return -1;
> >>>> +	}
> >>>> +
> >>>>  	if (resolve_shared_mounts(mntinfo, pms->master_id))
> >>>>  		return -1;
> >>>>  
> >>>>
> >>>> _______________________________________________
> >>>> CRIU mailing list
> >>>> CRIU at openvz.org
> >>>> https://lists.openvz.org/mailman/listinfo/criu
> >>>> .
> >>>>
> >>>
> >> .
> >>
> > 
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu