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

[Andrew Vagin]

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);
....
> 
> >  	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.