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

[Pavel Emelyanov]

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?

>  	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?

> +		pr_perror("Unable to restore mount flags: %x\n", mi->flags);
> +		return -1;
>  	}
>  
>  	if (private && mount(NULL, mi->mountpoint, NULL, MS_PRIVATE, NULL)) {
> @@ -1935,10 +1945,10 @@ skip_parent:
>  
>  static int do_new_mount(struct mount_info *mi)
>  {
> -	unsigned long mflags = MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE;
> +	unsigned long mflags = 0;
>  	char *src;
>  	struct fstype *tp = mi->fstype;
> -	bool remount_ro = (tp->restore && mi->flags & MS_RDONLY);
> +	bool remount_ro = (tp->restore && mi->sb_flags & MS_RDONLY);
>  
>  	src = resolve_source(mi);
>  	if (!src)
> @@ -1948,7 +1958,7 @@ static int do_new_mount(struct mount_info *mi)
>  		mflags |= MS_RDONLY;
>  
>  	if (mount(src, mi->mountpoint, tp->name,
> -			mi->flags & ~mflags, mi->options) < 0) {
> +			mi->sb_flags & ~mflags, mi->options) < 0) {
>  		pr_perror("Can't mount at %s", mi->mountpoint);
>  		return -1;
>  	}
> @@ -2324,6 +2334,7 @@ static int collect_mnt_from_image(struct mount_info **pms, struct ns_id *nsid)
>  		pm->parent_mnt_id	= me->parent_mnt_id;
>  		pm->s_dev		= me->root_dev;
>  		pm->flags		= me->flags;
> +		pm->sb_flags		= me->sb_flags;
>  		pm->shared_id		= me->shared_id;
>  		pm->master_id		= me->master_id;
>  		pm->need_plugin		= me->with_plugin;
> diff --git a/proc_parse.c b/proc_parse.c
> index 1f58f7f..10fb2df 100644
> --- a/proc_parse.c
> +++ b/proc_parse.c
> @@ -1078,7 +1078,7 @@ static int parse_mountinfo_ent(char *str, struct mount_info *new, char **fsname)
>  	if (!new->options)
>  		goto err;
>  
> -	if (parse_sb_opt(opt, &new->flags, new->options))
> +	if (parse_sb_opt(opt, &new->sb_flags, new->options))
>  		goto err;
>  
>  	ret = 0;
> diff --git a/protobuf/mnt.proto b/protobuf/mnt.proto
> index 6e58e1d..c6a1787 100644
> --- a/protobuf/mnt.proto
> +++ b/protobuf/mnt.proto
> @@ -41,4 +41,6 @@ message mnt_entry {
>  
>  	optional string		fsname			= 14;
>  	optional bool		internal_sharing	= 15;
> +
> +	optional uint32		sb_flags		= 16 [(criu).hex = true];
>  }
>