Re: [PATCH v3 19/19] ovl: support idmapped layers

[Vivek Goyal <vgoyal@redhat.com>]

On Thu, Mar 31, 2022 at 01:23:17PM +0200, Christian Brauner wrote:
> Now that overlay is able to take a layers idmapping into account allow
> overlay mounts to be created on top of idmapped mounts.
> 
> Cc: <linux-unionfs@vger.kernel.org>
> Tested-by: Giuseppe Scrivano <gscrivan@redhat.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
> ---
> /* v2 */
> - Turn on support for idmapped mounts in ovl_upper_idmap() helper here
>   after we've introduced it earlier in the series and made it return the
>   initial idmapping.
> 
> /* v3 */
> unchanged
> ---
>  fs/overlayfs/ovl_entry.h | 2 +-
>  fs/overlayfs/super.c     | 4 ----
>  2 files changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> index 79b612cfbe52..898b002a5c6f 100644
> --- a/fs/overlayfs/ovl_entry.h
> +++ b/fs/overlayfs/ovl_entry.h
> @@ -92,7 +92,7 @@ static inline struct vfsmount *ovl_upper_mnt(struct ovl_fs *ofs)
>  
>  static inline struct user_namespace *ovl_upper_idmap(struct ovl_fs *ofs)

Same minor nit here. Will ovl_upper_mnt_userns() be better for
readability. If it is too long, may be ovl_upper_mnt_uns().

I have this general comment here and other places
where "idmap" has been used. "idmap" is just one property vfs
derives from user namespace associated with mnt.

Vivek

>  {
> -       return &init_user_ns;
> +	return mnt_user_ns(ovl_upper_mnt(ofs));
>  }
>  
>  static inline struct ovl_fs *OVL_FS(struct super_block *sb)
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 9a656a24f7b1..d4cc07f7a2ef 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -874,10 +874,6 @@ static int ovl_mount_dir_noesc(const char *name, struct path *path)
>  		pr_err("filesystem on '%s' not supported\n", name);
>  		goto out_put;
>  	}
> -	if (is_idmapped_mnt(path->mnt)) {
> -		pr_err("idmapped layers are currently not supported\n");
> -		goto out_put;
> -	}
>  	if (!d_is_dir(path->dentry)) {
>  		pr_err("'%s' not a directory\n", name);
>  		goto out_put;
> -- 
> 2.32.0
>