[CRIU] [PATCH] mount: rework perparation for pivot_root

[Andrew Vagin]

On Fri, Aug 08, 2014 at 10:22:29AM -0500, Tycho Andersen wrote:
> On Fri, Aug 08, 2014 at 06:53:01PM +0400, Andrew Vagin wrote:
> > 
> > Is the opts.root path a mount point?
> 
> No, should have just been a regular file.
> 
> > Could you try the attached patcn?
> 
> This one gives me,
> 
> Error (mount.c:1280): Can't mount at ./proc: No such file or directory
> 
> which I guess is probably good since it is a new error?

We talked personally and found that Tycho misunderstands my patch.
It doesn't mount the root, it prepares it for pivot_root. Actually this
patch reworks the existing code. With this patch criu doesn't need to
collect mounts and look up the root mount. "criu restore" doesn't fail,
if the root mount over-mounts something else.

> 
> Tycho
> 
> > > 
> > > Tycho
> > > 
> > > > -
> > > > -		/* Our root is mounted over the parent (in the same directory) */
> > > > -		if (!strcmp(mi->parent->mountpoint, mi->mountpoint)) {
> > > > -			pr_err("The parent of the new root is unreachable\n");
> > > > +		if (mount(opts.root, opts.root, NULL, MS_BIND, NULL)) {
> > > > +			pr_perror("Unable to bind-mount %s", opts.root);
> > > >  			return -1;
> > > >  		}
> > > > -
> > > > -		if (mount("none", mi->parent->mountpoint + 1, "none", MS_SLAVE, NULL)) {
> > > > -			pr_perror("Can't remount the parent of the new root with MS_SLAVE");
> > > > +		if (chdir(opts.root)) {
> > > > +			pr_perror("chdir(%s) failed", opts.root);
> > > >  			return -1;
> > > >  		}
> > > >  	}
> > > >  
> > > > -	free_mntinfo(old);
> > > > -
> > > >  	ret = populate_mnt_ns(mis);
> > > >  	if (ret)
> > > >  		goto out;
> > > > -- 
> > > > 1.9.3
> > > > 
> 
> > From d611ebadb189c460051fac8d43d9df6d3e1262ea Mon Sep 17 00:00:00 2001
> > From: Andrey Vagin <avagin at openvz.org>
> > Date: Fri, 25 Jul 2014 16:22:06 +0400
> > Subject: [PATCH] mount: rework perparation for pivot_root
> > 
> > We can't bind-mount the required root into itself instead of
> > resolving a parent mount.
> > 
> > This patch is required to support userns, because if we want to make
> > pivot_root, the parent mount can't be locked. When we create userns
> > and mntns, all inherited mounts are marked as locked.
> > 
> > Cc: Tycho Andersen <tycho.andersen at canonical.com>
> > Signed-off-by: Andrey Vagin <avagin at openvz.org>
> > ---
> >  mount.c | 49 ++++++++++++++++++++++++-------------------------
> >  1 file changed, 24 insertions(+), 25 deletions(-)
> > 
> > diff --git a/mount.c b/mount.c
> > index 3287282..3612d02 100644
> > --- a/mount.c
> > +++ b/mount.c
> > @@ -1910,20 +1910,13 @@ int fini_mnt_ns(void)
> >  int prepare_mnt_ns(void)
> >  {
> >  	int ret = -1;
> > -	struct mount_info *mis, *old;
> > -	struct ns_id ns = { .pid = PROC_SELF, .nd = &mnt_ns_desc };
> > +	struct mount_info *mis;
> >  
> >  	if (!(root_ns_mask & CLONE_NEWNS))
> >  		return rst_collect_local_mntns();
> >  
> >  	pr_info("Restoring mount namespace\n");
> >  
> > -	old = collect_mntinfo(&ns);
> > -	if (old == NULL)
> > -		return -1;
> > -
> > -	close_proc();
> > -
> >  	mis = read_mnt_ns_img();
> >  	if (!mis)
> >  		goto out;
> > @@ -1939,36 +1932,42 @@ int prepare_mnt_ns(void)
> >  	 * prior to recreating new ones.
> >  	 */
> >  	if (!opts.root) {
> > -		if (clean_mnt_ns(ns.mnt.mntinfo_tree))
> > +		struct ns_id ns = { .pid = PROC_SELF, .nd = &mnt_ns_desc };
> > +		struct mount_info *old;
> > +		int err;
> > +
> > +		old = collect_mntinfo(&ns);
> > +		if (old == NULL)
> > +			return -1;
> > +
> > +		close_proc();
> > +
> > +		err = clean_mnt_ns(ns.mnt.mntinfo_tree);
> > +		free_mntinfo(old);
> > +		if (err)
> >  			return -1;
> >  	} else {
> > -		struct mount_info *mi;
> > +		close_proc();
> >  
> > -		/* moving a mount residing under a shared mount is invalid. */
> > -		mi = mount_resolve_path(ns.mnt.mntinfo_tree, opts.root);
> > -		if (mi == NULL) {
> > -			pr_err("Unable to find mount point for %s\n", opts.root);
> > +		if (mount(opts.root, opts.root, NULL, MS_BIND, NULL)) {
> > +			pr_perror("Unable to bind-mount %s", opts.root);
> >  			return -1;
> >  		}
> > -		if (mi->parent == NULL) {
> > -			pr_err("New root and old root are the same\n");
> > +		/* pivot_root fails, if a parent mount is shared */
> > +		if (mount("none", opts.root, "none", MS_SLAVE, NULL)) {
> > +			pr_perror("Can't remount the parent of the new root with MS_SLAVE");
> >  			return -1;
> >  		}
> > -
> > -		/* Our root is mounted over the parent (in the same directory) */
> > -		if (!strcmp(mi->parent->mountpoint, mi->mountpoint)) {
> > -			pr_err("The parent of the new root is unreachable\n");
> > +		if (mount(opts.root, opts.root, NULL, MS_BIND, NULL)) {
> > +			pr_perror("Unable to bind-mount %s", opts.root);
> >  			return -1;
> >  		}
> > -
> > -		if (mount("none", mi->parent->mountpoint + 1, "none", MS_SLAVE, NULL)) {
> > -			pr_perror("Can't remount the parent of the new root with MS_SLAVE");
> > +		if (chdir(opts.root)) {
> > +			pr_perror("chdir(%s) failed", opts.root);
> >  			return -1;
> >  		}
> >  	}
> >  
> > -	free_mntinfo(old);
> > -
> >  	ret = populate_mnt_ns(mis);
> >  	if (ret)
> >  		goto out;
> > -- 
> > 1.9.3
> > 
>