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

[Pavel Emelyanov]

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?

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

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

> +{
> +	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()?

> +{
> +	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.

> +			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;
>