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

[Kirill Tkhai]

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? Do we care about backward compatibility?