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

[Pavel Emelyanov]

On 08/03/2016 06:51 PM, Kirill Tkhai wrote:
> 
> 
> On 03.08.2016 16:34, Pavel Emelyanov wrote:
>> 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
> 
> Ok.
> 
> It's about forced mounted binfmt_misc or everything? 

Everything.

> Do we care about backward compatibility?

We do. Old images (binfmt-%device.img) should be checked for existence and used.

-- Pavel