[CRIU] [PATCH 5/6] [v2] mnt: try to split a mount tree to restore over-mounted mounts

[Andrei Vagin]

On Mon, Dec 05, 2016 at 12:34:58PM +0300, Pavel Emelyanov wrote:
> On 09/22/2016 09:16 AM, Andrei Vagin wrote:
> > From: Andrei Vagin <avagin at virtuozzo.com>
> > 
> > If a mount overmounts something else, we can try to restore it
> > separetly and then move it to the right places after restoring
> > all mounts.
> > 
> > In this patch if we see that a mount is overmounts something,
> > we create a new directory in the root yard and restore this
> > mount and its sub-tree in this directory.
> > 
> > https://bugs.openvz.org/browse/OVZ-6778
> > 
> > v2: add more comments
> >     rename roots_mp into roots_yard_mp
> > 
> > Signed-off-by: Andrei Vagin <avagin at virtuozzo.com>
> > ---
> >  criu/mount.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 153 insertions(+), 1 deletion(-)
> > 
> > diff --git a/criu/mount.c b/criu/mount.c
> > index a64671f..ccbe1eb 100644
> > --- a/criu/mount.c
> > +++ b/criu/mount.c
> > @@ -734,7 +734,8 @@ static int validate_mounts(struct mount_info *info, bool for_dump)
> >  			}
> >  		}
> >  skip_fstype:
> > -		if (does_mnt_overmount(m)) {
> > +		if (does_mnt_overmount(m) &&
> > +		    !list_empty(&m->parent->mnt_share)) {
> 
> So 'shared overmounts' are still not supported, do I get this hunk right?

Yes, you do

> 
> >  			pr_err("Unable to handle mounts under %d:%s\n",
> >  					m->mnt_id, m->mountpoint);
> >  			return -1;
> > @@ -2913,6 +2914,143 @@ static int do_umount_one(struct mount_info *mi)
> >  	return 0;
> >  }
> >  
> > +/*
> > + * If a mount overmounts other mounts, it is restored  separetly in the roots
> > + * yard and then moved to the right place.
> > + *
> > + * mnt_remap_entry is created for each such mount and it's added into
> > + * mnt_remap_list. The origin mount point is replaced on a new one in
> > + * roots_yard where it will be restored. The remapped mount will be
> > + * moved to the right places after restoring all mounts.
> > + */
> > +
> > +static inline int print_ns_root(struct ns_id *ns, int remap_id, char *buf, int bs);
> > +static int get_mp_mountpoint(char *mountpoint, struct mount_info *mi, char *root, int root_len);
> > +
> > +static LIST_HEAD(mnt_remap_list);
> > +static int remap_id;
> > +
> > +struct mnt_remap_entry {
> > +	struct mount_info *child; /* child is remaped into the root yards */
> 
> So child is the mi to be remapped, isn't it?

yes

> 
> > +	struct mount_info *parent; /* the origin parent for the child*/
> > +	struct list_head node;
> > +};
> > +
> > +static int do_mnt_remap(struct mount_info *m)
> > +{
> > +	int len;
> > +
> > +	if (m->nsid->type == NS_OTHER) {
> > +		/*
> > +		 * m->mountpoint already contains a roots_yard prefix and
> > +		 * it has a fixed size, so it can be just replaced.
> > +		 */
> > +		len = print_ns_root(m->nsid, remap_id, m->mountpoint, PATH_MAX);
> > +		m->mountpoint[len] = '/';
> > +	} else if (m->nsid->type == NS_ROOT) {
> > +		char root[PATH_MAX], *mp, *ns_mp;
> > +		int len, ret;
> > +
> > +		/*
> > +		 * Allocate a new path in the roots yard. m->mountpoint in the
> > +		 * root namespace doesn't have a roots_yard prefix, so its
> > +		 * size has to be changed and a new storage has to be
> > +		 * allocated.
> > +		 */
> > +		mp = m->mountpoint; ns_mp = m->ns_mountpoint;
> > +
> > +		len = print_ns_root(m->nsid, remap_id, root, PATH_MAX);
> > +
> > +		ret = get_mp_mountpoint(ns_mp, m, root, len);
> > +		if (ret < 0)
> > +			return ret;
> > +		xfree(mp);
> > +	} else
> > +		BUG();
> > +	return 0;
> > +}
> > +
> > +static int remap_mnt(struct mount_info *m)
> 
> check_mnt_remap()? try_remap_mount()?
> 
> And the do_mnt_remap(0 should be symmetrically renamed :) E.g.
> 
> check_mnt_remap/do_mnt_remap
> try_remap_mount/do_remap_mount

Ok

> 
> > +{
> > +	struct mnt_remap_entry *r;
> > +
> > +	if (!does_mnt_overmount(m))
> > +		return 0;
> > +
> > +	BUG_ON(!m->parent || !list_empty(&m->parent->mnt_share));
> > +
> > +	r = xmalloc(sizeof(struct mnt_remap_entry));
> > +	if (!r)
> > +		return -1;
> > +
> > +	r->child = m;
> > +	list_add(&r->node, &mnt_remap_list);
> > +
> > +	return 0;
> > +}
> > +
> > +static int handle_overmounts(struct mount_info *root)
> 
> setup_mnt_remaps()? or remap_overmounts()?

Ok

> 
> > +{
> > +	struct mnt_remap_entry *r;
> > +	struct mount_info *m;
> > +
> > +	/* It's imposiable to change a tree without interrupting
> > +	 * enumeration, so on the first step mounts are added
> > +	 * into mnt_remap_list and then they are connected to root_yard_mp.
> > +	 */
> > +	if (mnt_tree_for_each(root, remap_mnt))
> > +		return -1;
> > +
> > +	/* Move remapped mounts to root_yard */
> > +	list_for_each_entry(r, &mnt_remap_list, node) {
> > +		m = r->child;
> > +		r->parent = m->parent;
> > +		m->parent = root_yard_mp;
> > +		list_del(&m->siblings);
> > +		list_add(&m->siblings, &root_yard_mp->children);
> > +
> > +		remap_id++;
> > +		mnt_tree_for_each(m, do_mnt_remap);
> > +		pr_debug("Restore the %d mount in %s\n", m->mnt_id, m->mountpoint);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/* Move remapped mounts to places where they have to be */
> > +static int move_back_mnt_remaps()
> > +{
> > +	struct mnt_remap_entry *r;
> > +
> > +	list_for_each_entry(r, &mnt_remap_list, node) {
> > +		struct mount_info *m = r->child;
> > +		char path[PATH_MAX];
> > +		int len;
> > +
> > +		if (m->nsid->type == NS_ROOT) {
> > +			path[0] = '.';
> > +			strncpy(path + 1, m->ns_mountpoint, PATH_MAX - 1);
> > +		} else {
> > +			strncpy(path, m->mountpoint, PATH_MAX);
> > +			len = print_ns_root(m->nsid, 0, path, PATH_MAX);
> > +			path[len] = '/';
> > +		}
> > +
> > +		pr_debug("Move mount %s -> %s\n", m->mountpoint, path);
> > +		if (mount(m->mountpoint, path, NULL, MS_MOVE, NULL)) {
> > +			pr_perror("Unable to move mount %s -> %s", m->mountpoint, path);
> > +			return -1;
> > +		}
> > +
> > +		/* Insert child back to its place in the tree */
> > +		list_del(&r->child->siblings);
> > +		list_add(&r->child->siblings, &r->parent->children);
> > +		r->child->parent = r->parent;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int cr_pivot_root(char *root)
> >  {
> >  	char tmp_dir_tmpl[] = "crtools-put-root.XXXXXX";
> > @@ -3394,6 +3532,7 @@ void fini_restore_mntns(void)
> >   */
> >  static int populate_roots_yard(void)
> >  {
> > +	struct mnt_remap_entry *r;
> >  	char path[PATH_MAX];
> >  	struct ns_id *nsid;
> >  
> > @@ -3414,6 +3553,13 @@ static int populate_roots_yard(void)
> >  		}
> >  	}
> >  
> > +	list_for_each_entry(r, &mnt_remap_list, node) {
> > +		if (mkdirpat(AT_FDCWD, r->child->mountpoint)) {
> 
> Please, add a code comment saying where the mnt_remap_list is populated.

Ok

> 
> > +			pr_perror("Unable to create %s", r->child->mountpoint);
> > +			return -1;
> > +		}
> > +	}
> > +
> >  	return 0;
> >  }
> >  
> > @@ -3462,6 +3608,9 @@ static int populate_mnt_ns(void)
> >  	if (validate_mounts(mntinfo, false))
> >  		return -1;
> >  
> > +	if (handle_overmounts(pms))
> > +		return -1;
> > +
> >  	/*
> >  	 * Set properties for the root before mounting a root yard,
> >  	 * otherwise the root yard can be propagated into the host
> > @@ -3479,6 +3628,9 @@ static int populate_mnt_ns(void)
> >  	ret = mnt_tree_for_each(pms, do_mount_one);
> >  	mnt_tree_for_each(pms, do_close_one);
> >  
> > +	if (ret == 0 && move_back_mnt_remaps())
> > +		return -1;
> > +
> >  	if (umount_clean_path())
> >  		return -1;
> >  	return ret;
> > 
>