[CRIU] [PATCH] mnt: Carry run-time device ID in mount_info

[Cyrill Gorcunov]

On Thu, Dec 03, 2015 at 03:01:11PM +0300, Pavel Emelyanov wrote:
> > +	/*
> > +	 * To prevent overmount clash we use two entries
> > +	 * @s_dev and @s_dev_rt.  On dump @s_dev = @s_dev_rt
> > +	 * obtained from procfs parsing but on restore @s_dev_rt
> > +	 * may differ with one obtained from the image: the device
> > +	 * ID may be new on provided by the kernel itself on new
> > +	 * mount.
> > +	 */
> > +	if (dev != pm->s_dev && dev != pm->s_dev_rt) {
> 
> The first comparison is wrong, a "new" device can by chance coincide with
> some "old" one and we'll have erroneous "open_mountpoint is OK".

How is that? There is two points where @s_dev and @s_dev_rt are
assigned

1) When we parse procfs => @s_dev = @s_dev_rt always
2) When we do mount the device and fetch new run-time @s_dev_rt
   from the stat call, so I really don't see how coincide may happen/

> Also here should go BUG_ON(pm->s_dev_rt == NON_INITIALIZED).

sure

> 
> > +		pr_err("The file system %#x %#x (%#x) %s %s is inaccessible\n",
> > +		       pm->s_dev, pm->s_dev_rt, (int)dev,
> > +		       pm->fstype->name, pm->ns_mountpoint);
> >  		goto err;
> >  	}
> >  
> > @@ -1795,9 +1804,21 @@ static char *resolve_source(struct mount_info *mi)
> >  
> >  static int restore_shared_options(struct mount_info *mi, bool private, bool shared, bool slave)
> >  {
> > +	struct stat st;
> > +
> >  	pr_debug("%d:%s private %d shared %d slave %d\n",
> >  			mi->mnt_id, mi->mountpoint, private, shared, slave);
> >  
> > +	/*
> > +	 * Fetch runtime device once we're mounted for mount
> > +	 * resolving when device has been migrated.
> > +	 */
> > +	if (stat(mi->mountpoint, &st)) {
> > +		pr_perror("Can't stat on %s\n", mi->mountpoint);
> > +		return -1;
> > +	}
> > +	mi->s_dev_rt = MKKDEV(major(st.st_dev), minor(st.st_dev));
> 
> This stat() should in in do_mount_one() after the 

Why? Every mount point created go through restore_shared_options, don't they?

> 
>         if (!mi->parent) {
>                 /* do_mount_root() is called from populate_mnt_ns() */
>                 mi->mounted = true;
>                 ret = 0;
>         } else if (!mi->bind && !mi->need_plugin && !mi->external)
>                 ret = do_new_mount(mi);
>         else
>                 ret = do_bind_mount(mi);
> 
> and also propagate_mount() should copy the s_dev_rt on all the auto-propagated
> mountpoints.