[CRIU] [PATCH v5 5/5] mount: Restore binfmt_misc content in case of missing mount

[Kirill Tkhai]

On 22.08.2016 16:31, Pavel Emelyanov wrote:
> On 08/15/2016 11:57 AM, Kirill Tkhai wrote:
>>
>>
>> On 05.08.2016 18:43, Pavel Emelyanov wrote:
>>> On 08/04/2016 04:49 PM, Kirill Tkhai wrote:
>>>> In case of mount image is not containing binfmt_misc mountpoint,
>>>> add temporary mountpoint to mount tree and try to restore
>>>> binfmt_misc content in ordinary way. Then, umount temporary mountpoint.
>>>>
>>>> v4: New
>>>> v5: Check for opts.has_binfmt_misc to determine if image presents
>>>>
>>>> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
>>>> ---
>>>>  criu/mount.c |   19 ++++++++++++++++++-
>>>>  1 file changed, 18 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/criu/mount.c b/criu/mount.c
>>>> index 926817a..964dbbc 100644
>>>> --- a/criu/mount.c
>>>> +++ b/criu/mount.c
>>>> @@ -2451,6 +2451,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) {
>>>> +			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");
>>>> @@ -2470,7 +2479,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;
>>>> @@ -3339,6 +3348,7 @@ static int populate_mnt_ns(void)
>>>>  	struct mount_info *pms;
>>>>  	struct ns_id *nsid;
>>>>  	struct mount_info *roots_mp = NULL;
>>>> +	struct stat st;
>>>>  	int ret;
>>>>  
>>>>  	if (mnt_roots) {
>>>> @@ -3355,6 +3365,13 @@ static int populate_mnt_ns(void)
>>>>  	if (!pms)
>>>>  		return -1;
>>>>  
>>>> +	if (!opts.has_binfmt_misc && stat_image(CR_FD_BINFMT_MISC, &st, 0) == 0) {
>>>
>>> This should be the other way. In cr-restore.c we have an array of so called cinfos -- these
>>> are images that should be pre-loaded during restore. So it's better to add new-style binfmt
>>> cinfo and read_mnt_ns_img should check cinfo->flags for COLLECT_HAPPENED (not 100% sure it
>>> handles empty images correctly, plz, check) and add cr-time mount if it had.
>>
>> You had suggested to dump any binfmt_misc content (belonging to a mount point or not)
>> to the file of the same name. So, the new suggestion doesn't work.
> 
> Why?

Because in case of binfmt_misc image belongs to a real mount, we should not add a cr-time mount;
it will be restored when real mount is restored.


The second thing I should pointed in previous letter, you have suggested to use opts to check
if image present: https://patchwork.criu.org/patch/1069/:

"For aufs we have opts.aufs bit. For ... overlayfs (?) we have similar.
We can add binfmt_misc to opts, or have separate entity to ask "whether fs X was
mounted" for."

Why now we are moving to another "check cinfo->flags for COLLECT_HAPPENED" mechanism???

> 
>> The code, I did, populates opts.has_binfmt_misc in dependence of belonging mountpoint exists or not.
>> Your new suggestion isn't able to handle this.
> 
> Why? The cinfo collection code calls callback so you can set opts.whatver there.

It was the single comment with two paragraphs. You suggested to add cr_time mount uncontested,
and it does not work. See my above comment.

>>> -- Pavel
>>>
>>>> +		/* Add to mount tree. Generic code will mount it later */
>>>> +		ret = add_cr_time_mount(pms, "binfmt_misc", BINFMT_MISC_HOME, 0);
>>>> +		if (ret)
>>>> +			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
>>>> .
>>>>
>>>
>> .
>>
>