[CRIU] [PATCH 2/2] mount: don't clean up propogation options for the root mount (v2)

[Pavel Emelyanov]

On 03/18/2014 11:46 PM, Andrey Vagin wrote:
> Currently we marks all mounts as private before restoring mntns. We do
> these to avoid problem with pivot_root.
> It's wrong, because the root mount can be slave for an external shared
> group. The root mount is not mounted by CRIU, so here is nothing wrong.
> 
> Now look at the pivot_root code in kernel
> if (IS_MNT_SHARED(old_mnt) ||
> 	IS_MNT_SHARED(new_mnt->mnt_parent) ||
> 	IS_MNT_SHARED(root_mnt->mnt_parent))
> 	goto out4;
> 
> So we don't need to change options for all mounts. We need to remount
> / and the parent of the new root. It's safe, because we already in another
> mntns.
> 
> v2: simplify code
> 
> Signed-off-by: Andrey Vagin <avagin at openvz.org>
> ---
>  mount.c | 39 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 32 insertions(+), 7 deletions(-)
> 
> diff --git a/mount.c b/mount.c
> index c6a110a..a0daee5 100644
> --- a/mount.c
> +++ b/mount.c
> @@ -1238,6 +1238,11 @@ static int cr_pivot_root(void)
>  		return -1;
>  	}
>  
> +	if (mount("none", put_root, "none", MS_REC|MS_PRIVATE, NULL)) {
> +		pr_perror("Can't remount root with MS_PRIVATE");
> +		return -1;
> +	}
> +
>  	if (umount2(put_root, MNT_DETACH)) {
>  		pr_perror("Can't umount %s", put_root);
>  		return -1;
> @@ -1411,11 +1416,6 @@ int prepare_mnt_ns(int ns_pid)
>  	if (!mis)
>  		goto out;
>  
> -	if (mount("none", "/", "none", MS_REC|MS_PRIVATE, NULL)) {
> -		pr_perror("Can't remount root with MS_PRIVATE");
> -		return -1;
> -	}
> -
>  	if (chdir(opts.root ? : "/")) {
>  		pr_perror("chdir(%s) failed", opts.root ? : "/");
>  		return -1;
> @@ -1426,8 +1426,33 @@ int prepare_mnt_ns(int ns_pid)
>  	 * clones from the original one. We have to umount them
>  	 * prior to recreating new ones.
>  	 */
> -	if (!opts.root && clean_mnt_ns())
> -		return -1;
> +	if (!opts.root) {
> +		if (clean_mnt_ns())
> +			return -1;
> +	} else {
> +		struct mount_info *mi;
> +
> +		/* moving a mount residing under a shared mount is invalid. */
> +		mi = mount_resolve_path(opts.root);
> +		if (mi == NULL) {
> +			pr_err("Unable to find mount point for %s\n", opts.root);
> +			return -1;
> +		}
> +		if (mi->parent == NULL) {
> +			pr_err("New root and old root are the same\n");
> +			return -1;
> +		}
> +
> +		if (!strcmp(mi->parent->mountpoint, mi->mountpoint)) {

Add some comment saying why mountpoints match means "parent of the root is unreachable".

> +			pr_err("The parent of the new root is unreachable\n");
> +			return -1;
> +		}
> +
> +		if (mount("none", mi->parent->mountpoint, "none", MS_SLAVE, NULL)) {

Last time there were two MS_SLAVE marks ;) Where did the 2nd one disappear?

> +			pr_perror("Can't remount the parent of the new root with MS_SLAVE");
> +			return -1;
> +		}
> +	}
>  
>  	free_mounts();
>  
> 


Thanks,
Pavel