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

Submitted by Andrei Vagin on Sept. 13, 2016, 4:19 a.m.

Details

Message ID 1473740388-3797-6-git-send-email-avagin@openvz.org
State Rejected
Series "mnt: try to split a mount tree to restore over-mounted mounts"
Headers show

Commit Message

Andrei Vagin Sept. 13, 2016, 4:19 a.m.
From: Andrei Vagin <avagin@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
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
---
 criu/include/mount.h |   1 +
 criu/mount.c         | 160 +++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 157 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/include/mount.h b/criu/include/mount.h
index a266cd0..d9b1521 100644
--- a/criu/include/mount.h
+++ b/criu/include/mount.h
@@ -65,6 +65,7 @@  struct mount_info {
 	bool			need_plugin;
 	bool			is_ns_root;
 	bool			deleted;
+	bool			remap;
 	struct mount_info	*next;
 	struct ns_id		*nsid;
 
diff --git a/criu/mount.c b/criu/mount.c
index e568249..6d61219 100644
--- a/criu/mount.c
+++ b/criu/mount.c
@@ -2718,7 +2718,146 @@  static int do_umount_one(struct mount_info *mi)
 	return 0;
 }
 
-static int cr_pivot_root(char *root)
+static struct mount_info *roots_mp = NULL;
+
+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 *parent, *child;
+	struct list_head node;
+};
+
+/*
+ * If a mount overmounts other mounts, it is restored
+ * separetly and then moved to the right place.
+ * All these mounts are moved into the root yard.
+ */
+static int do_mnt_remap(struct mount_info *m)
+{
+	int len;
+
+	if (m->nsid->type == NS_OTHER) {
+		/* A path in root_yard has a fixed size, so it can be replaced. */
+		len = print_ns_root(m->nsid, remap_id, m->mountpoint, PATH_MAX);
+		m->mountpoint[len] = '/';
+	} else {
+		char root[PATH_MAX], *mp, *ns_mp;
+		int len, ret;
+
+		/* Add a root_yard path */
+		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);
+	}
+	return 0;
+}
+
+static int remap_mnt(struct mount_info *m)
+{
+	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)
+{
+	struct mnt_remap_entry *r;
+	struct mount_info *m;
+
+	/*
+	 * Mark mounts which have to be restored separetly,
+	 * because it's imposiable to remove them from
+	 * a tree without interrupting enumeration.
+	 */
+	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 = roots_mp;
+		list_del(&m->siblings);
+		list_add(&m->siblings, &roots_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 handle_mnt_remaps(int nsid, char *put_root)
+{
+	struct mnt_remap_entry *r;
+	int cwd, exit_code = -1;
+
+	cwd = open(".", O_PATH);
+	if (cwd < 0) {
+		pr_perror("Unable to open \".\"\n");
+		return -1;
+	}
+
+	if (nsid != root_item->ids->mnt_ns_id && chdir(put_root)) {
+		pr_perror("Unable to change working directory");
+		return -1;
+	}
+
+	list_for_each_entry(r, &mnt_remap_list, node) {
+		struct mount_info *m = r->child;
+
+		if (r->child->nsid->id != nsid)
+			continue;
+
+		pr_debug("Move mount %s -> %s\n", m->mountpoint, m->ns_mountpoint);
+		if (mount(m->mountpoint, m->ns_mountpoint, NULL, MS_MOVE, NULL)) {
+			pr_perror("Unable to move mount %s -> %s", m->mountpoint, m->ns_mountpoint);
+			goto err;
+		}
+
+		list_del(&r->child->siblings);
+		list_add(&r->child->siblings, &r->parent->children);
+		r->child->parent = r->parent;
+	}
+
+	exit_code = 0;
+err:
+	if (fchdir(cwd)) {
+		pr_perror("Unable to change working directory");
+		close(cwd);
+		return -1;
+	}
+	close(cwd);
+
+	return exit_code;
+}
+
+
+static int cr_pivot_root(char *root, int nsid)
 {
 	char tmp_dir_tmpl[] = "crtools-put-root.XXXXXX";
 	bool tmp_dir = false;
@@ -2759,6 +2898,9 @@  static int cr_pivot_root(char *root)
 		goto err_tmpfs;
 	}
 
+	if (nsid > 0 && handle_mnt_remaps(nsid, put_root))
+		return -1;
+
 	if (mount("none", put_root, "none", MS_REC|MS_SLAVE, NULL)) {
 		pr_perror("Can't remount root with MS_PRIVATE");
 		return -1;
@@ -3196,6 +3338,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;
 
@@ -3216,6 +3359,13 @@  static int populate_roots_yard(void)
 		}
 	}
 
+	list_for_each_entry(r, &mnt_remap_list, node) {
+		if (mkdirpat(AT_FDCWD, r->child->mountpoint)) {
+			pr_perror("Unable to create %s", r->child->mountpoint);
+			return -1;
+		}
+	}
+
 	return 0;
 }
 
@@ -3223,7 +3373,6 @@  static int populate_mnt_ns(void)
 {
 	struct mount_info *pms;
 	struct ns_id *nsid;
-	struct mount_info *roots_mp = NULL;
 	int ret;
 
 	if (mnt_roots) {
@@ -3258,6 +3407,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
@@ -3462,7 +3614,7 @@  int prepare_mnt_ns(void)
 
 	ret = populate_mnt_ns();
 	if (!ret && opts.root)
-		ret = cr_pivot_root(NULL);
+		ret = cr_pivot_root(NULL, root_item->ids->mnt_ns_id);
 	if (ret)
 		return -1;
 
@@ -3495,7 +3647,7 @@  int prepare_mnt_ns(void)
 		/* Set its root */
 		path[0] = '/';
 		print_ns_root(nsid, 0, path + 1, sizeof(path) - 1);
-		if (cr_pivot_root(path))
+		if (cr_pivot_root(path, nsid->id))
 			goto err;
 
 		/* Pin one with a file descriptor */

Comments

Pavel Emelianov Sept. 21, 2016, 7:37 a.m.
On 09/13/2016 07:19 AM, Andrei Vagin wrote:
> From: Andrei Vagin <avagin@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.

Hm... Why not simply tune can_mount_new() to refuse mounting the
top-mounts until sub-mounts are?

> 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
> Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
> ---
>  criu/include/mount.h |   1 +
>  criu/mount.c         | 160 +++++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 157 insertions(+), 4 deletions(-)
> 
> diff --git a/criu/include/mount.h b/criu/include/mount.h
> index a266cd0..d9b1521 100644
> --- a/criu/include/mount.h
> +++ b/criu/include/mount.h
> @@ -65,6 +65,7 @@ struct mount_info {
>  	bool			need_plugin;
>  	bool			is_ns_root;
>  	bool			deleted;
> +	bool			remap;

Unused.

>  	struct mount_info	*next;
>  	struct ns_id		*nsid;
>  
> diff --git a/criu/mount.c b/criu/mount.c
> index e568249..6d61219 100644
> --- a/criu/mount.c
> +++ b/criu/mount.c
> @@ -2718,7 +2718,146 @@ static int do_umount_one(struct mount_info *mi)
>  	return 0;
>  }
>  
> -static int cr_pivot_root(char *root)
> +static struct mount_info *roots_mp = NULL;

This thing was moved from populate_mnt_ns(). This name is OK for the
on-stack variable, but for the file-wide one it's no longer nice.

> +
> +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 *parent, *child;
> +	struct list_head node;

Please, document the parent and child fields of this struct.

> +};
> +
> +/*
> + * If a mount overmounts other mounts, it is restored
> + * separetly and then moved to the right place.
> + * All these mounts are moved into the root yard.
> + */
> +static int do_mnt_remap(struct mount_info *m)
> +{
> +	int len;
> +
> +	if (m->nsid->type == NS_OTHER) {
> +		/* A path in root_yard has a fixed size, so it can be replaced. */
> +		len = print_ns_root(m->nsid, remap_id, m->mountpoint, PATH_MAX);
> +		m->mountpoint[len] = '/';
> +	} else {

Else what? type == NS_ROOT? And why is root's mounts are __that__ special?

> +		char root[PATH_MAX], *mp, *ns_mp;
> +		int len, ret;
> +
> +		/* Add a root_yard path */

Please, write a MORE descriptive comment showing what is changed into what.

> +		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);
> +	}
> +	return 0;
> +}
> +
> +static int remap_mnt(struct mount_info *m)
> +{
> +	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)
> +{
> +	struct mnt_remap_entry *r;
> +	struct mount_info *m;
> +
> +	/*
> +	 * Mark mounts which have to be restored separetly,
> +	 * because it's imposiable to remove them from
> +	 * a tree without interrupting enumeration.
> +	 */
> +	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 = roots_mp;
> +		list_del(&m->siblings);
> +		list_add(&m->siblings, &roots_mp->children);

list_move()?

Why not do all of the above in remap_mnt()?

> +
> +		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 handle_mnt_remaps(int nsid, char *put_root)

Two handle_ routines in one patch :) Too generic names, can we come up with better ones?

> +{
> +	struct mnt_remap_entry *r;
> +	int cwd, exit_code = -1;
> +
> +	cwd = open(".", O_PATH);
> +	if (cwd < 0) {
> +		pr_perror("Unable to open \".\"\n");
> +		return -1;
> +	}
> +
> +	if (nsid != root_item->ids->mnt_ns_id && chdir(put_root)) {

What for?

> +		pr_perror("Unable to change working directory");
> +		return -1;
> +	}
> +
> +	list_for_each_entry(r, &mnt_remap_list, node) {
> +		struct mount_info *m = r->child;
> +
> +		if (r->child->nsid->id != nsid)
> +			continue;

Where's the guarantee that the mnt_remap_list() is empty at the end?

> +
> +		pr_debug("Move mount %s -> %s\n", m->mountpoint, m->ns_mountpoint);
> +		if (mount(m->mountpoint, m->ns_mountpoint, NULL, MS_MOVE, NULL)) {
> +			pr_perror("Unable to move mount %s -> %s", m->mountpoint, m->ns_mountpoint);
> +			goto err;
> +		}
> +
> +		list_del(&r->child->siblings);
> +		list_add(&r->child->siblings, &r->parent->children);
> +		r->child->parent = r->parent;

Worth having 'reparent_mount()' helper? The same code is in handle_overmounts().

> +	}
> +
> +	exit_code = 0;
> +err:
> +	if (fchdir(cwd)) {
> +		pr_perror("Unable to change working directory");
> +		close(cwd);
> +		return -1;
> +	}
> +	close(cwd);
> +
> +	return exit_code;
> +}
> +
> +
> +static int cr_pivot_root(char *root, int nsid)
>  {
>  	char tmp_dir_tmpl[] = "crtools-put-root.XXXXXX";
>  	bool tmp_dir = false;
> @@ -2759,6 +2898,9 @@ static int cr_pivot_root(char *root)
>  		goto err_tmpfs;
>  	}
>  
> +	if (nsid > 0 && handle_mnt_remaps(nsid, put_root))
> +		return -1;
> +
>  	if (mount("none", put_root, "none", MS_REC|MS_SLAVE, NULL)) {
>  		pr_perror("Can't remount root with MS_PRIVATE");
>  		return -1;
> @@ -3196,6 +3338,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;
>  
> @@ -3216,6 +3359,13 @@ static int populate_roots_yard(void)
>  		}
>  	}
>  
> +	list_for_each_entry(r, &mnt_remap_list, node) {
> +		if (mkdirpat(AT_FDCWD, r->child->mountpoint)) {

You mkdir() with names from image for all the namespaces found. What if names conflict?

> +			pr_perror("Unable to create %s", r->child->mountpoint);
> +			return -1;
> +		}
> +	}
> +
>  	return 0;
>  }
>  
> @@ -3223,7 +3373,6 @@ static int populate_mnt_ns(void)
>  {
>  	struct mount_info *pms;
>  	struct ns_id *nsid;
> -	struct mount_info *roots_mp = NULL;
>  	int ret;
>  
>  	if (mnt_roots) {
> @@ -3258,6 +3407,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
> @@ -3462,7 +3614,7 @@ int prepare_mnt_ns(void)
>  
>  	ret = populate_mnt_ns();
>  	if (!ret && opts.root)
> -		ret = cr_pivot_root(NULL);
> +		ret = cr_pivot_root(NULL, root_item->ids->mnt_ns_id);
>  	if (ret)
>  		return -1;
>  
> @@ -3495,7 +3647,7 @@ int prepare_mnt_ns(void)
>  		/* Set its root */
>  		path[0] = '/';
>  		print_ns_root(nsid, 0, path + 1, sizeof(path) - 1);
> -		if (cr_pivot_root(path))
> +		if (cr_pivot_root(path, nsid->id))
>  			goto err;
>  
>  		/* Pin one with a file descriptor */
>
Andrey Vagin Sept. 21, 2016, 7:56 p.m.
On Wed, Sep 21, 2016 at 10:37:10AM +0300, Pavel Emelyanov wrote:
> On 09/13/2016 07:19 AM, Andrei Vagin wrote:
> > From: Andrei Vagin <avagin@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.
> 
> Hm... Why not simply tune can_mount_new() to refuse mounting the
> top-mounts until sub-mounts are?

because top-mounts can be from shared groups which are required to
restore sub-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
> > Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
> > ---
> >  criu/include/mount.h |   1 +
> >  criu/mount.c         | 160 +++++++++++++++++++++++++++++++++++++++++++++++++--
> >  2 files changed, 157 insertions(+), 4 deletions(-)
> > 
> > diff --git a/criu/include/mount.h b/criu/include/mount.h
> > index a266cd0..d9b1521 100644
> > --- a/criu/include/mount.h
> > +++ b/criu/include/mount.h
> > @@ -65,6 +65,7 @@ struct mount_info {
> >  	bool			need_plugin;
> >  	bool			is_ns_root;
> >  	bool			deleted;
> > +	bool			remap;
> 
> Unused.
> 
> >  	struct mount_info	*next;
> >  	struct ns_id		*nsid;
> >  
> > diff --git a/criu/mount.c b/criu/mount.c
> > index e568249..6d61219 100644
> > --- a/criu/mount.c
> > +++ b/criu/mount.c
> > @@ -2718,7 +2718,146 @@ static int do_umount_one(struct mount_info *mi)
> >  	return 0;
> >  }
> >  
> > -static int cr_pivot_root(char *root)
> > +static struct mount_info *roots_mp = NULL;
> 
> This thing was moved from populate_mnt_ns(). This name is OK for the
> on-stack variable, but for the file-wide one it's no longer nice.
> 
> > +
> > +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 *parent, *child;
> > +	struct list_head node;
> 
> Please, document the parent and child fields of this struct.
> 
> > +};
> > +
> > +/*
> > + * If a mount overmounts other mounts, it is restored
> > + * separetly and then moved to the right place.
> > + * All these mounts are moved into the root yard.
> > + */
> > +static int do_mnt_remap(struct mount_info *m)
> > +{
> > +	int len;
> > +
> > +	if (m->nsid->type == NS_OTHER) {
> > +		/* A path in root_yard has a fixed size, so it can be replaced. */

^^^^^^^^^^^^^ the answer for the next question is here
> > +		len = print_ns_root(m->nsid, remap_id, m->mountpoint, PATH_MAX);
> > +		m->mountpoint[len] = '/';
> > +	} else {
> 
> Else what? type == NS_ROOT? And why is root's mounts are __that__ special?

Because the length of m->mountpoint is changed in this case
and we need to reallocate it.
> 
> > +		char root[PATH_MAX], *mp, *ns_mp;
> > +		int len, ret;
> > +
> > +		/* Add a root_yard path */
> 
> Please, write a MORE descriptive comment showing what is changed into what.
> 
> > +		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);
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int remap_mnt(struct mount_info *m)
> > +{
> > +	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)
> > +{
> > +	struct mnt_remap_entry *r;
> > +	struct mount_info *m;
> > +
> > +	/*
> > +	 * Mark mounts which have to be restored separetly,
> > +	 * because it's imposiable to remove them from
> > +	 * a tree without interrupting enumeration.
> > +	 */

This is an answer on your next question.

> > +	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 = roots_mp;
> > +		list_del(&m->siblings);
> > +		list_add(&m->siblings, &roots_mp->children);
> 
> list_move()?

ok

> 
> Why not do all of the above in remap_mnt()?

I wrote a comment to answer on this question.
> 
> > +
> > +		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 handle_mnt_remaps(int nsid, char *put_root)
> 
> Two handle_ routines in one patch :) Too generic names, can we come up with better ones?
> 
> > +{
> > +	struct mnt_remap_entry *r;
> > +	int cwd, exit_code = -1;
> > +
> > +	cwd = open(".", O_PATH);
> > +	if (cwd < 0) {
> > +		pr_perror("Unable to open \".\"\n");
> > +		return -1;
> > +	}
> > +
> > +	if (nsid != root_item->ids->mnt_ns_id && chdir(put_root)) {
> 
> What for?

Because the root yard is there. I had to write a comment here, but
I reworked this code for the next series.
> 
> > +		pr_perror("Unable to change working directory");
> > +		return -1;
> > +	}
> > +
> > +	list_for_each_entry(r, &mnt_remap_list, node) {
> > +		struct mount_info *m = r->child;
> > +
> > +		if (r->child->nsid->id != nsid)
> > +			continue;
> 
> Where's the guarantee that the mnt_remap_list() is empty at the end?

We enumirate all namespaces, so it has to be clean.

In the next version this code will be reworked too.

> 
> > +
> > +		pr_debug("Move mount %s -> %s\n", m->mountpoint, m->ns_mountpoint);
> > +		if (mount(m->mountpoint, m->ns_mountpoint, NULL, MS_MOVE, NULL)) {
> > +			pr_perror("Unable to move mount %s -> %s", m->mountpoint, m->ns_mountpoint);
> > +			goto err;
> > +		}
> > +
> > +		list_del(&r->child->siblings);
> > +		list_add(&r->child->siblings, &r->parent->children);
> > +		r->child->parent = r->parent;
> 
> Worth having 'reparent_mount()' helper? The same code is in handle_overmounts().

It isn't the same. There we move mount to the remap list and here we
remove the mount from the remap list.
> 
> > +	}
> > +
> > +	exit_code = 0;
> > +err:
> > +	if (fchdir(cwd)) {
> > +		pr_perror("Unable to change working directory");
> > +		close(cwd);
> > +		return -1;
> > +	}
> > +	close(cwd);
> > +
> > +	return exit_code;
> > +}
> > +
> > +
> > +static int cr_pivot_root(char *root, int nsid)
> >  {
> >  	char tmp_dir_tmpl[] = "crtools-put-root.XXXXXX";
> >  	bool tmp_dir = false;
> > @@ -2759,6 +2898,9 @@ static int cr_pivot_root(char *root)
> >  		goto err_tmpfs;
> >  	}
> >  
> > +	if (nsid > 0 && handle_mnt_remaps(nsid, put_root))
> > +		return -1;
> > +
> >  	if (mount("none", put_root, "none", MS_REC|MS_SLAVE, NULL)) {
> >  		pr_perror("Can't remount root with MS_PRIVATE");
> >  		return -1;
> > @@ -3196,6 +3338,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;
> >  
> > @@ -3216,6 +3359,13 @@ static int populate_roots_yard(void)
> >  		}
> >  	}
> >  
> > +	list_for_each_entry(r, &mnt_remap_list, node) {
> > +		if (mkdirpat(AT_FDCWD, r->child->mountpoint)) {
> 
> You mkdir() with names from image for all the namespaces found. What if names conflict?

We add remap_id into a name, which is uniq

> 
> > +			pr_perror("Unable to create %s", r->child->mountpoint);
> > +			return -1;
> > +		}
> > +	}
> > +
> >  	return 0;
> >  }
> >  
> > @@ -3223,7 +3373,6 @@ static int populate_mnt_ns(void)
> >  {
> >  	struct mount_info *pms;
> >  	struct ns_id *nsid;
> > -	struct mount_info *roots_mp = NULL;
> >  	int ret;
> >  
> >  	if (mnt_roots) {
> > @@ -3258,6 +3407,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
> > @@ -3462,7 +3614,7 @@ int prepare_mnt_ns(void)
> >  
> >  	ret = populate_mnt_ns();
> >  	if (!ret && opts.root)
> > -		ret = cr_pivot_root(NULL);
> > +		ret = cr_pivot_root(NULL, root_item->ids->mnt_ns_id);
> >  	if (ret)
> >  		return -1;
> >  
> > @@ -3495,7 +3647,7 @@ int prepare_mnt_ns(void)
> >  		/* Set its root */
> >  		path[0] = '/';
> >  		print_ns_root(nsid, 0, path + 1, sizeof(path) - 1);
> > -		if (cr_pivot_root(path))
> > +		if (cr_pivot_root(path, nsid->id))
> >  			goto err;
> >  
> >  		/* Pin one with a file descriptor */
> > 
>