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

[Pavel Emelyanov]

On 08/03/2016 03:42 PM, Kirill Tkhai wrote:
> 
> 
> On 03.08.2016 14:46, Pavel Emelyanov wrote:
>> On 08/02/2016 11:06 PM, Andrew Vagin wrote:
>>> 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.
>>
>> Agree. Since binfmt entries exist w/o any mountpoints, then ID of the
>> image with entries should be some other, not the device.
> 
> But there is no principal difference between to use mnt_id or s_dev. They both
> are options of mounted partition.

There is difference between mnt_id/s_dev AND mntNS_id. On collect stage (restore) 
you either have the binfmt image id at hands or you don't. If you use mnt_id/s_dev
-- you don't have id and have to readdir() the image dir. If you use mntNS_id
you have it (we've just collected the pstree and all the ids images) and we
can just check for binfmt-$id image.


>> Another option would be to put __all__ binfmt-s into single file w/o any
>> IDs in name. Like binfmts.img. And on restore collect one and cache.
> 
> It's not a problem, but how do you see the future when some namespace
> will support binfmt_misc isolation?
> 
> Tell your final vision of how we should name image file. Should we save
> everything in "binfmts.img"?

Saving all in single binfmtmisc.img is safe against unpredictable binfmt virtualization 
in the future :D

-- Pavel