[CRIU] [PATCH 3/4] mount: handle mnt_flags and sb_flags separatly

[Pavel Emelyanov]

On 08/13/2015 09:05 PM, Andrew Vagin wrote:
> On Thu, Aug 13, 2015 at 06:34:40PM +0300, Pavel Emelyanov wrote:
>> On 08/11/2015 06:33 PM, Andrey Vagin wrote:
>>> They both can container the MS_READONLY flag. And in one case it will be
>>> read-only bind-mount and in another case it will be read-only
>>> super-block.
>>>
>>> Signed-off-by: Andrey Vagin <avagin at openvz.org>
>>> ---
>>>  include/proc_parse.h |  1 +
>>>  mount.c              | 27 +++++++++++++++++++--------
>>>  proc_parse.c         |  2 +-
>>>  protobuf/mnt.proto   |  2 ++
>>>  4 files changed, 23 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/include/proc_parse.h b/include/proc_parse.h
>>> index 42f8893..ed68c95 100644
>>> --- a/include/proc_parse.h
>>> +++ b/include/proc_parse.h
>>> @@ -118,6 +118,7 @@ struct mount_info {
>>>  	char		*mountpoint;
>>>  	char		*ns_mountpoint;
>>>  	unsigned	flags;
>>> +	unsigned	sb_flags;
>>
>> OK, let's look at random mountinfo entry
>>
>> 37 17 0:13 / /dev/mqueue rw,relatime shared:24 - mqueue mqueue rw
>>
>> The rw.relatime is mountpoint flags and the trailing rw is sb flags.
>> In old version of criu wil restore relatime with the single mount()
>> call. With this patch will mount mqueue with RW flag and then call
>> remount(MS_REMOUNT | MS_BIND | MS_RELATIME). Will this create the
>> same mountinfo entry?
> 
> No, it will not. We use MS_REMOUNT.
> 
> do_mount:
> ...
>         if (flags & MS_REMOUNT)
>                 retval = do_remount(&path, flags & ~MS_REMOUNT, mnt_flags,
>                                     data_page);
>         else if (flags & MS_BIND)
>                 retval = do_loopback(&path, dev_name, flags & MS_REC);
> ...
> 
> do_remount
> ...
>         if (flags & MS_BIND)
>                 err = change_mount_flags(path->mnt, flags);
>         else if (!capable(CAP_SYS_ADMIN))
>                 err = -EPERM;
>         else
>                 err = do_remount_sb(sb, flags, data, 0);
> ....

So after the patch the whole mount tree will be restored differently as compared
as it used to. Right?

>>
>>>  	int		master_id;
>>>  	int		shared_id;
>>>  	struct fstype	*fstype;
>>> diff --git a/mount.c b/mount.c
>>> index 344d646..4589399 100644
>>> --- a/mount.c
>>> +++ b/mount.c
>>> @@ -1598,6 +1598,8 @@ static int dump_one_mountpoint(struct mount_info *pm, struct cr_img *img)
>>>  	me.root_dev		= pm->s_dev;
>>>  	me.parent_mnt_id	= pm->parent_mnt_id;
>>>  	me.flags		= pm->flags;
>>> +	me.sb_flags		= pm->sb_flags;
>>> +	me.has_sb_flags		= true;
>>>  	me.mountpoint		= pm->mountpoint + 1;
>>>  	me.source		= pm->source;
>>>  	me.options		= pm->options;
>>> @@ -1806,14 +1808,22 @@ static char *resolve_source(struct mount_info *mi)
>>>  
>>>  static int restore_shared_options(struct mount_info *mi, bool private, bool shared, bool slave)
>>>  {
>>> +	unsigned int mflags = MS_SHARED | MS_SLAVE | MS_PRIVATE;
>>> +
>>>  	pr_debug("%d:%s private %d shared %d slave %d\n",
>>>  			mi->mnt_id, mi->mountpoint, private, shared, slave);
>>>  
>>> -	if (mi->flags & MS_UNBINDABLE) {
>>> -		if (shared || slave)
>>> -			pr_warn("%s has both unbindable and sharing, ignoring unbindable\n", mi->mountpoint);
>>> -		else
>>> -			return mount(NULL, mi->mountpoint, NULL, MS_UNBINDABLE, NULL);
>>> +
>>> +	mflags = mi->flags & (~mflags);
>>> +
>>> +	if ((mi->flags & MS_UNBINDABLE) && (shared || slave)) {
>>> +		pr_warn("%s has both unbindable and sharing, ignoring unbindable\n", mi->mountpoint);
>>> +		mflags &= ~MS_UNBINDABLE;
>>> +	}
>>> +
>>> +	if (mount(NULL, mi->mountpoint, NULL, MS_REMOUNT | MS_BIND | mflags, NULL)) {
>>
>> Can we save an extra mount() call in case plain mountpoint creation
>> sets mount flags correctly?
> 
> I will try.

Thanks!