[PATCH] erofs: support idmapped mounts

[PATCH] erofs: support idmapped mounts

[Chao Yu]

This patch enables idmapped mounts for erofs, since all dedicated helpers
for this functionality existsm, so, in this patch we just pass down the
user_namespace argument from the VFS methods to the relevant helpers.

Simple idmap example on erofs image:

1. mkdir dir
2. touch dir/file
3. mkfs.erofs erofs.img dir
4. mount -t erofs -o loop erofs.img  /mnt/erofs/

5. ls -ln /mnt/erofs/
total 0
-rw-rw-r-- 1 1000 1000 0 May 17 15:26 file

6. mount-idmapped --map-mount b:0:1001:1 /mnt/erofs/ /mnt/scratch_erofs/

7. ls -ln /mnt/scratch_erofs/
total 0
-rw-rw-r-- 1 65534 65534 0 May 17 15:26 file

Signed-off-by: Chao Yu <chao.yu@oppo.com>
---
 fs/erofs/inode.c | 2 +-
 fs/erofs/super.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
index e8b37ba5e9ad..5320bf52c1ce 100644
--- a/fs/erofs/inode.c
+++ b/fs/erofs/inode.c
@@ -370,7 +370,7 @@ int erofs_getattr(struct user_namespace *mnt_userns, const struct path *path,
 	stat->attributes_mask |= (STATX_ATTR_COMPRESSED |
 				  STATX_ATTR_IMMUTABLE);
 
-	generic_fillattr(&init_user_ns, inode, stat);
+	generic_fillattr(mnt_userns, inode, stat);
 	return 0;
 }
 
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 0c4b41130c2f..7dc5f2e8ddee 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -781,7 +781,7 @@ static struct file_system_type erofs_fs_type = {
 	.name           = "erofs",
 	.init_fs_context = erofs_init_fs_context,
 	.kill_sb        = erofs_kill_sb,
-	.fs_flags       = FS_REQUIRES_DEV,
+	.fs_flags       = FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
 };
 MODULE_ALIAS_FS("erofs");
 
-- 
2.25.1

[PATCH] erofs: support idmapped mounts

[Chao Yu]

This patch enables idmapped mounts for erofs, since all dedicated helpers
for this functionality existsm, so, in this patch we just pass down the
user_namespace argument from the VFS methods to the relevant helpers.

Simple idmap example on erofs image:

1. mkdir dir
2. touch dir/file
3. mkfs.erofs erofs.img dir
4. mount -t erofs -o loop erofs.img  /mnt/erofs/

5. ls -ln /mnt/erofs/
total 0
-rw-rw-r-- 1 1000 1000 0 May 17 15:26 file

6. mount-idmapped --map-mount b:0:1001:1 /mnt/erofs/ /mnt/scratch_erofs/

7. ls -ln /mnt/scratch_erofs/
total 0
-rw-rw-r-- 1 65534 65534 0 May 17 15:26 file

Signed-off-by: Chao Yu <chao.yu@oppo.com>
---
 fs/erofs/inode.c | 2 +-
 fs/erofs/super.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
index e8b37ba5e9ad..5320bf52c1ce 100644
--- a/fs/erofs/inode.c
+++ b/fs/erofs/inode.c
@@ -370,7 +370,7 @@ int erofs_getattr(struct user_namespace *mnt_userns, const struct path *path,
 	stat->attributes_mask |= (STATX_ATTR_COMPRESSED |
 				  STATX_ATTR_IMMUTABLE);
 
-	generic_fillattr(&init_user_ns, inode, stat);
+	generic_fillattr(mnt_userns, inode, stat);
 	return 0;
 }
 
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 0c4b41130c2f..7dc5f2e8ddee 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -781,7 +781,7 @@ static struct file_system_type erofs_fs_type = {
 	.name           = "erofs",
 	.init_fs_context = erofs_init_fs_context,
 	.kill_sb        = erofs_kill_sb,
-	.fs_flags       = FS_REQUIRES_DEV,
+	.fs_flags       = FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
 };
 MODULE_ALIAS_FS("erofs");
 
-- 
2.25.1

Re: [PATCH] erofs: support idmapped mounts

[Gao Xiang]

Hi Chao,

On Tue, May 17, 2022 at 03:32:10PM +0800, Chao Yu wrote:
> This patch enables idmapped mounts for erofs, since all dedicated helpers
> for this functionality existsm, so, in this patch we just pass down the
> user_namespace argument from the VFS methods to the relevant helpers.
> 
> Simple idmap example on erofs image:
> 
> 1. mkdir dir
> 2. touch dir/file
> 3. mkfs.erofs erofs.img dir
> 4. mount -t erofs -o loop erofs.img  /mnt/erofs/
> 
> 5. ls -ln /mnt/erofs/
> total 0
> -rw-rw-r-- 1 1000 1000 0 May 17 15:26 file
> 
> 6. mount-idmapped --map-mount b:0:1001:1 /mnt/erofs/ /mnt/scratch_erofs/
> 
> 7. ls -ln /mnt/scratch_erofs/
> total 0
> -rw-rw-r-- 1 65534 65534 0 May 17 15:26 file
> 
> Signed-off-by: Chao Yu <chao.yu@oppo.com>

Thanks, yeah, I think it's enough to enable idmapped mount for EROFS:
Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Thanks,
Gao Xiang

> ---
>  fs/erofs/inode.c | 2 +-
>  fs/erofs/super.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
> index e8b37ba5e9ad..5320bf52c1ce 100644
> --- a/fs/erofs/inode.c
> +++ b/fs/erofs/inode.c
> @@ -370,7 +370,7 @@ int erofs_getattr(struct user_namespace *mnt_userns, const struct path *path,
>  	stat->attributes_mask |= (STATX_ATTR_COMPRESSED |
>  				  STATX_ATTR_IMMUTABLE);
>  
> -	generic_fillattr(&init_user_ns, inode, stat);
> +	generic_fillattr(mnt_userns, inode, stat);
>  	return 0;
>  }
>  
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 0c4b41130c2f..7dc5f2e8ddee 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -781,7 +781,7 @@ static struct file_system_type erofs_fs_type = {
>  	.name           = "erofs",
>  	.init_fs_context = erofs_init_fs_context,
>  	.kill_sb        = erofs_kill_sb,
> -	.fs_flags       = FS_REQUIRES_DEV,
> +	.fs_flags       = FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
>  };
>  MODULE_ALIAS_FS("erofs");
>  
> -- 
> 2.25.1

Re: [PATCH] erofs: support idmapped mounts

[Gao Xiang]

Hi Chao,

On Tue, May 17, 2022 at 03:32:10PM +0800, Chao Yu wrote:
> This patch enables idmapped mounts for erofs, since all dedicated helpers
> for this functionality existsm, so, in this patch we just pass down the
> user_namespace argument from the VFS methods to the relevant helpers.
> 
> Simple idmap example on erofs image:
> 
> 1. mkdir dir
> 2. touch dir/file
> 3. mkfs.erofs erofs.img dir
> 4. mount -t erofs -o loop erofs.img  /mnt/erofs/
> 
> 5. ls -ln /mnt/erofs/
> total 0
> -rw-rw-r-- 1 1000 1000 0 May 17 15:26 file
> 
> 6. mount-idmapped --map-mount b:0:1001:1 /mnt/erofs/ /mnt/scratch_erofs/
> 
> 7. ls -ln /mnt/scratch_erofs/
> total 0
> -rw-rw-r-- 1 65534 65534 0 May 17 15:26 file
> 
> Signed-off-by: Chao Yu <chao.yu@oppo.com>

Thanks, yeah, I think it's enough to enable idmapped mount for EROFS:
Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Thanks,
Gao Xiang

> ---
>  fs/erofs/inode.c | 2 +-
>  fs/erofs/super.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
> index e8b37ba5e9ad..5320bf52c1ce 100644
> --- a/fs/erofs/inode.c
> +++ b/fs/erofs/inode.c
> @@ -370,7 +370,7 @@ int erofs_getattr(struct user_namespace *mnt_userns, const struct path *path,
>  	stat->attributes_mask |= (STATX_ATTR_COMPRESSED |
>  				  STATX_ATTR_IMMUTABLE);
>  
> -	generic_fillattr(&init_user_ns, inode, stat);
> +	generic_fillattr(mnt_userns, inode, stat);
>  	return 0;
>  }
>  
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 0c4b41130c2f..7dc5f2e8ddee 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -781,7 +781,7 @@ static struct file_system_type erofs_fs_type = {
>  	.name           = "erofs",
>  	.init_fs_context = erofs_init_fs_context,
>  	.kill_sb        = erofs_kill_sb,
> -	.fs_flags       = FS_REQUIRES_DEV,
> +	.fs_flags       = FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
>  };
>  MODULE_ALIAS_FS("erofs");
>  
> -- 
> 2.25.1

Re: [PATCH] erofs: support idmapped mounts

[Christian Brauner]

On Tue, May 17, 2022 at 03:32:10PM +0800, Chao Yu wrote:
> This patch enables idmapped mounts for erofs, since all dedicated helpers
> for this functionality existsm, so, in this patch we just pass down the
> user_namespace argument from the VFS methods to the relevant helpers.
> 
> Simple idmap example on erofs image:
> 
> 1. mkdir dir
> 2. touch dir/file
> 3. mkfs.erofs erofs.img dir
> 4. mount -t erofs -o loop erofs.img  /mnt/erofs/
> 
> 5. ls -ln /mnt/erofs/
> total 0
> -rw-rw-r-- 1 1000 1000 0 May 17 15:26 file
> 
> 6. mount-idmapped --map-mount b:0:1001:1 /mnt/erofs/ /mnt/scratch_erofs/
> 
> 7. ls -ln /mnt/scratch_erofs/
> total 0
> -rw-rw-r-- 1 65534 65534 0 May 17 15:26 file

Your current example maps id 0 in the filesystem to id 1001 in the
mount. But since no files with id 0 exist in the filesystem you're
illustrating that unmapped ids are correctly reported as overflow{g,u}id.

I think what you'd rather want to show is something like this:

5. ls -ln /mnt/erofs/
total 0
-rw-rw-r-- 1 1000 1000 0 May 17 15:26 file

6. mount-idmapped --map-mount b:1000:1001:1 /mnt/erofs/ /mnt/scratch_erofs/

7. ls -ln /mnt/scratch_erofs/
total 0
-rw-rw-r-- 1 1001 1001 0 May 17 15:26 file

where id 1000 in the filesystem maps to id 1001 in the mount.

> 
> Signed-off-by: Chao Yu <chao.yu@oppo.com>
> ---

Overall this is currently the smallest patch to support idmapped mounts.

Is erofs integrated with xfstests in any way?
For read-only filesystems we probably only need to verify that {g,u}id
are correctly reported. All the writable aspects are irrelevant.

Looks good,
Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>

>  fs/erofs/inode.c | 2 +-
>  fs/erofs/super.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
> index e8b37ba5e9ad..5320bf52c1ce 100644
> --- a/fs/erofs/inode.c
> +++ b/fs/erofs/inode.c
> @@ -370,7 +370,7 @@ int erofs_getattr(struct user_namespace *mnt_userns, const struct path *path,
>  	stat->attributes_mask |= (STATX_ATTR_COMPRESSED |
>  				  STATX_ATTR_IMMUTABLE);
>  
> -	generic_fillattr(&init_user_ns, inode, stat);
> +	generic_fillattr(mnt_userns, inode, stat);
>  	return 0;
>  }
>  
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 0c4b41130c2f..7dc5f2e8ddee 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -781,7 +781,7 @@ static struct file_system_type erofs_fs_type = {
>  	.name           = "erofs",
>  	.init_fs_context = erofs_init_fs_context,
>  	.kill_sb        = erofs_kill_sb,
> -	.fs_flags       = FS_REQUIRES_DEV,
> +	.fs_flags       = FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
>  };
>  MODULE_ALIAS_FS("erofs");
>  
> -- 
> 2.25.1
> 

Re: [PATCH] erofs: support idmapped mounts

[Christian Brauner]

On Tue, May 17, 2022 at 03:32:10PM +0800, Chao Yu wrote:
> This patch enables idmapped mounts for erofs, since all dedicated helpers
> for this functionality existsm, so, in this patch we just pass down the
> user_namespace argument from the VFS methods to the relevant helpers.
> 
> Simple idmap example on erofs image:
> 
> 1. mkdir dir
> 2. touch dir/file
> 3. mkfs.erofs erofs.img dir
> 4. mount -t erofs -o loop erofs.img  /mnt/erofs/
> 
> 5. ls -ln /mnt/erofs/
> total 0
> -rw-rw-r-- 1 1000 1000 0 May 17 15:26 file
> 
> 6. mount-idmapped --map-mount b:0:1001:1 /mnt/erofs/ /mnt/scratch_erofs/
> 
> 7. ls -ln /mnt/scratch_erofs/
> total 0
> -rw-rw-r-- 1 65534 65534 0 May 17 15:26 file

Your current example maps id 0 in the filesystem to id 1001 in the
mount. But since no files with id 0 exist in the filesystem you're
illustrating that unmapped ids are correctly reported as overflow{g,u}id.

I think what you'd rather want to show is something like this:

5. ls -ln /mnt/erofs/
total 0
-rw-rw-r-- 1 1000 1000 0 May 17 15:26 file

6. mount-idmapped --map-mount b:1000:1001:1 /mnt/erofs/ /mnt/scratch_erofs/

7. ls -ln /mnt/scratch_erofs/
total 0
-rw-rw-r-- 1 1001 1001 0 May 17 15:26 file

where id 1000 in the filesystem maps to id 1001 in the mount.

> 
> Signed-off-by: Chao Yu <chao.yu@oppo.com>
> ---

Overall this is currently the smallest patch to support idmapped mounts.

Is erofs integrated with xfstests in any way?
For read-only filesystems we probably only need to verify that {g,u}id
are correctly reported. All the writable aspects are irrelevant.

Looks good,
Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>

>  fs/erofs/inode.c | 2 +-
>  fs/erofs/super.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
> index e8b37ba5e9ad..5320bf52c1ce 100644
> --- a/fs/erofs/inode.c
> +++ b/fs/erofs/inode.c
> @@ -370,7 +370,7 @@ int erofs_getattr(struct user_namespace *mnt_userns, const struct path *path,
>  	stat->attributes_mask |= (STATX_ATTR_COMPRESSED |
>  				  STATX_ATTR_IMMUTABLE);
>  
> -	generic_fillattr(&init_user_ns, inode, stat);
> +	generic_fillattr(mnt_userns, inode, stat);
>  	return 0;
>  }
>  
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 0c4b41130c2f..7dc5f2e8ddee 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -781,7 +781,7 @@ static struct file_system_type erofs_fs_type = {
>  	.name           = "erofs",
>  	.init_fs_context = erofs_init_fs_context,
>  	.kill_sb        = erofs_kill_sb,
> -	.fs_flags       = FS_REQUIRES_DEV,
> +	.fs_flags       = FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
>  };
>  MODULE_ALIAS_FS("erofs");
>  
> -- 
> 2.25.1
> 

Re: [PATCH] erofs: support idmapped mounts

[Gao Xiang]

Hi Christian,

On Tue, May 17, 2022 at 11:06:22AM +0200, Christian Brauner wrote:
> On Tue, May 17, 2022 at 03:32:10PM +0800, Chao Yu wrote:
> > This patch enables idmapped mounts for erofs, since all dedicated helpers
> > for this functionality existsm, so, in this patch we just pass down the
> > user_namespace argument from the VFS methods to the relevant helpers.
> > 
> > Simple idmap example on erofs image:
> > 
> > 1. mkdir dir
> > 2. touch dir/file
> > 3. mkfs.erofs erofs.img dir
> > 4. mount -t erofs -o loop erofs.img  /mnt/erofs/
> > 
> > 5. ls -ln /mnt/erofs/
> > total 0
> > -rw-rw-r-- 1 1000 1000 0 May 17 15:26 file
> > 
> > 6. mount-idmapped --map-mount b:0:1001:1 /mnt/erofs/ /mnt/scratch_erofs/
> > 
> > 7. ls -ln /mnt/scratch_erofs/
> > total 0
> > -rw-rw-r-- 1 65534 65534 0 May 17 15:26 file
> 
> Your current example maps id 0 in the filesystem to id 1001 in the
> mount. But since no files with id 0 exist in the filesystem you're
> illustrating that unmapped ids are correctly reported as overflow{g,u}id.
> 
> I think what you'd rather want to show is something like this:
> 
> 5. ls -ln /mnt/erofs/
> total 0
> -rw-rw-r-- 1 1000 1000 0 May 17 15:26 file
> 
> 6. mount-idmapped --map-mount b:1000:1001:1 /mnt/erofs/ /mnt/scratch_erofs/
> 
> 7. ls -ln /mnt/scratch_erofs/
> total 0
> -rw-rw-r-- 1 1001 1001 0 May 17 15:26 file
> 
> where id 1000 in the filesystem maps to id 1001 in the mount.
> 
> > 
> > Signed-off-by: Chao Yu <chao.yu@oppo.com>
> > ---
> 
> Overall this is currently the smallest patch to support idmapped mounts.
> 
> Is erofs integrated with xfstests in any way?
> For read-only filesystems we probably only need to verify that {g,u}id
> are correctly reported. All the writable aspects are irrelevant.

Currently most generic xfstests test cases are unsuitable for erofs.

Instead we have regression testcases for EROFS specific since it needs
to generate images with care,
 https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/log/?h=experimental-tests

Also we have an erofsstress to do long time random stress workloads,
https://github.com/erofs/erofsstress

But yeah, it's some awkward that fstests idmapped mount testcases may
be unsuitable for EROFS for now. I will add some new testcases to build
images and test for this behavior.

> 
> Looks good,
> Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>

Thanks for your review!

Thanks,
Gao Xiang

> 
> >  fs/erofs/inode.c | 2 +-
> >  fs/erofs/super.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
> > index e8b37ba5e9ad..5320bf52c1ce 100644
> > --- a/fs/erofs/inode.c
> > +++ b/fs/erofs/inode.c
> > @@ -370,7 +370,7 @@ int erofs_getattr(struct user_namespace *mnt_userns, const struct path *path,
> >  	stat->attributes_mask |= (STATX_ATTR_COMPRESSED |
> >  				  STATX_ATTR_IMMUTABLE);
> >  
> > -	generic_fillattr(&init_user_ns, inode, stat);
> > +	generic_fillattr(mnt_userns, inode, stat);
> >  	return 0;
> >  }
> >  
> > diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> > index 0c4b41130c2f..7dc5f2e8ddee 100644
> > --- a/fs/erofs/super.c
> > +++ b/fs/erofs/super.c
> > @@ -781,7 +781,7 @@ static struct file_system_type erofs_fs_type = {
> >  	.name           = "erofs",
> >  	.init_fs_context = erofs_init_fs_context,
> >  	.kill_sb        = erofs_kill_sb,
> > -	.fs_flags       = FS_REQUIRES_DEV,
> > +	.fs_flags       = FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
> >  };
> >  MODULE_ALIAS_FS("erofs");
> >  
> > -- 
> > 2.25.1
> > 

Re: [PATCH] erofs: support idmapped mounts

[Gao Xiang]

Hi Christian,

On Tue, May 17, 2022 at 11:06:22AM +0200, Christian Brauner wrote:
> On Tue, May 17, 2022 at 03:32:10PM +0800, Chao Yu wrote:
> > This patch enables idmapped mounts for erofs, since all dedicated helpers
> > for this functionality existsm, so, in this patch we just pass down the
> > user_namespace argument from the VFS methods to the relevant helpers.
> > 
> > Simple idmap example on erofs image:
> > 
> > 1. mkdir dir
> > 2. touch dir/file
> > 3. mkfs.erofs erofs.img dir
> > 4. mount -t erofs -o loop erofs.img  /mnt/erofs/
> > 
> > 5. ls -ln /mnt/erofs/
> > total 0
> > -rw-rw-r-- 1 1000 1000 0 May 17 15:26 file
> > 
> > 6. mount-idmapped --map-mount b:0:1001:1 /mnt/erofs/ /mnt/scratch_erofs/
> > 
> > 7. ls -ln /mnt/scratch_erofs/
> > total 0
> > -rw-rw-r-- 1 65534 65534 0 May 17 15:26 file
> 
> Your current example maps id 0 in the filesystem to id 1001 in the
> mount. But since no files with id 0 exist in the filesystem you're
> illustrating that unmapped ids are correctly reported as overflow{g,u}id.
> 
> I think what you'd rather want to show is something like this:
> 
> 5. ls -ln /mnt/erofs/
> total 0
> -rw-rw-r-- 1 1000 1000 0 May 17 15:26 file
> 
> 6. mount-idmapped --map-mount b:1000:1001:1 /mnt/erofs/ /mnt/scratch_erofs/
> 
> 7. ls -ln /mnt/scratch_erofs/
> total 0
> -rw-rw-r-- 1 1001 1001 0 May 17 15:26 file
> 
> where id 1000 in the filesystem maps to id 1001 in the mount.
> 
> > 
> > Signed-off-by: Chao Yu <chao.yu@oppo.com>
> > ---
> 
> Overall this is currently the smallest patch to support idmapped mounts.
> 
> Is erofs integrated with xfstests in any way?
> For read-only filesystems we probably only need to verify that {g,u}id
> are correctly reported. All the writable aspects are irrelevant.

Currently most generic xfstests test cases are unsuitable for erofs.

Instead we have regression testcases for EROFS specific since it needs
to generate images with care,
 https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/log/?h=experimental-tests

Also we have an erofsstress to do long time random stress workloads,
https://github.com/erofs/erofsstress

But yeah, it's some awkward that fstests idmapped mount testcases may
be unsuitable for EROFS for now. I will add some new testcases to build
images and test for this behavior.

> 
> Looks good,
> Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>

Thanks for your review!

Thanks,
Gao Xiang

> 
> >  fs/erofs/inode.c | 2 +-
> >  fs/erofs/super.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
> > index e8b37ba5e9ad..5320bf52c1ce 100644
> > --- a/fs/erofs/inode.c
> > +++ b/fs/erofs/inode.c
> > @@ -370,7 +370,7 @@ int erofs_getattr(struct user_namespace *mnt_userns, const struct path *path,
> >  	stat->attributes_mask |= (STATX_ATTR_COMPRESSED |
> >  				  STATX_ATTR_IMMUTABLE);
> >  
> > -	generic_fillattr(&init_user_ns, inode, stat);
> > +	generic_fillattr(mnt_userns, inode, stat);
> >  	return 0;
> >  }
> >  
> > diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> > index 0c4b41130c2f..7dc5f2e8ddee 100644
> > --- a/fs/erofs/super.c
> > +++ b/fs/erofs/super.c
> > @@ -781,7 +781,7 @@ static struct file_system_type erofs_fs_type = {
> >  	.name           = "erofs",
> >  	.init_fs_context = erofs_init_fs_context,
> >  	.kill_sb        = erofs_kill_sb,
> > -	.fs_flags       = FS_REQUIRES_DEV,
> > +	.fs_flags       = FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
> >  };
> >  MODULE_ALIAS_FS("erofs");
> >  
> > -- 
> > 2.25.1
> > 

Re: [PATCH] erofs: support idmapped mounts

[Christian Brauner]

On Tue, May 17, 2022 at 05:15:02PM +0800, Gao Xiang wrote:
> Hi Christian,
> 
> On Tue, May 17, 2022 at 11:06:22AM +0200, Christian Brauner wrote:
> > On Tue, May 17, 2022 at 03:32:10PM +0800, Chao Yu wrote:
> > > This patch enables idmapped mounts for erofs, since all dedicated helpers
> > > for this functionality existsm, so, in this patch we just pass down the
> > > user_namespace argument from the VFS methods to the relevant helpers.
> > > 
> > > Simple idmap example on erofs image:
> > > 
> > > 1. mkdir dir
> > > 2. touch dir/file
> > > 3. mkfs.erofs erofs.img dir
> > > 4. mount -t erofs -o loop erofs.img  /mnt/erofs/
> > > 
> > > 5. ls -ln /mnt/erofs/
> > > total 0
> > > -rw-rw-r-- 1 1000 1000 0 May 17 15:26 file
> > > 
> > > 6. mount-idmapped --map-mount b:0:1001:1 /mnt/erofs/ /mnt/scratch_erofs/
> > > 
> > > 7. ls -ln /mnt/scratch_erofs/
> > > total 0
> > > -rw-rw-r-- 1 65534 65534 0 May 17 15:26 file
> > 
> > Your current example maps id 0 in the filesystem to id 1001 in the
> > mount. But since no files with id 0 exist in the filesystem you're
> > illustrating that unmapped ids are correctly reported as overflow{g,u}id.
> > 
> > I think what you'd rather want to show is something like this:
> > 
> > 5. ls -ln /mnt/erofs/
> > total 0
> > -rw-rw-r-- 1 1000 1000 0 May 17 15:26 file
> > 
> > 6. mount-idmapped --map-mount b:1000:1001:1 /mnt/erofs/ /mnt/scratch_erofs/
> > 
> > 7. ls -ln /mnt/scratch_erofs/
> > total 0
> > -rw-rw-r-- 1 1001 1001 0 May 17 15:26 file
> > 
> > where id 1000 in the filesystem maps to id 1001 in the mount.
> > 
> > > 
> > > Signed-off-by: Chao Yu <chao.yu@oppo.com>
> > > ---
> > 
> > Overall this is currently the smallest patch to support idmapped mounts.
> > 
> > Is erofs integrated with xfstests in any way?
> > For read-only filesystems we probably only need to verify that {g,u}id
> > are correctly reported. All the writable aspects are irrelevant.
> 
> Currently most generic xfstests test cases are unsuitable for erofs.
> 
> Instead we have regression testcases for EROFS specific since it needs
> to generate images with care,
>  https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/log/?h=experimental-tests
> 
> Also we have an erofsstress to do long time random stress workloads,
> https://github.com/erofs/erofsstress
> 
> But yeah, it's some awkward that fstests idmapped mount testcases may
> be unsuitable for EROFS for now. I will add some new testcases to build
> images and test for this behavior.
> 
> > 
> > Looks good,
> > Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>
> 
> Thanks for your review!

Thanks for supporting this in erofs!
Christian

Re: [PATCH] erofs: support idmapped mounts

[Christian Brauner]

On Tue, May 17, 2022 at 05:15:02PM +0800, Gao Xiang wrote:
> Hi Christian,
> 
> On Tue, May 17, 2022 at 11:06:22AM +0200, Christian Brauner wrote:
> > On Tue, May 17, 2022 at 03:32:10PM +0800, Chao Yu wrote:
> > > This patch enables idmapped mounts for erofs, since all dedicated helpers
> > > for this functionality existsm, so, in this patch we just pass down the
> > > user_namespace argument from the VFS methods to the relevant helpers.
> > > 
> > > Simple idmap example on erofs image:
> > > 
> > > 1. mkdir dir
> > > 2. touch dir/file
> > > 3. mkfs.erofs erofs.img dir
> > > 4. mount -t erofs -o loop erofs.img  /mnt/erofs/
> > > 
> > > 5. ls -ln /mnt/erofs/
> > > total 0
> > > -rw-rw-r-- 1 1000 1000 0 May 17 15:26 file
> > > 
> > > 6. mount-idmapped --map-mount b:0:1001:1 /mnt/erofs/ /mnt/scratch_erofs/
> > > 
> > > 7. ls -ln /mnt/scratch_erofs/
> > > total 0
> > > -rw-rw-r-- 1 65534 65534 0 May 17 15:26 file
> > 
> > Your current example maps id 0 in the filesystem to id 1001 in the
> > mount. But since no files with id 0 exist in the filesystem you're
> > illustrating that unmapped ids are correctly reported as overflow{g,u}id.
> > 
> > I think what you'd rather want to show is something like this:
> > 
> > 5. ls -ln /mnt/erofs/
> > total 0
> > -rw-rw-r-- 1 1000 1000 0 May 17 15:26 file
> > 
> > 6. mount-idmapped --map-mount b:1000:1001:1 /mnt/erofs/ /mnt/scratch_erofs/
> > 
> > 7. ls -ln /mnt/scratch_erofs/
> > total 0
> > -rw-rw-r-- 1 1001 1001 0 May 17 15:26 file
> > 
> > where id 1000 in the filesystem maps to id 1001 in the mount.
> > 
> > > 
> > > Signed-off-by: Chao Yu <chao.yu@oppo.com>
> > > ---
> > 
> > Overall this is currently the smallest patch to support idmapped mounts.
> > 
> > Is erofs integrated with xfstests in any way?
> > For read-only filesystems we probably only need to verify that {g,u}id
> > are correctly reported. All the writable aspects are irrelevant.
> 
> Currently most generic xfstests test cases are unsuitable for erofs.
> 
> Instead we have regression testcases for EROFS specific since it needs
> to generate images with care,
>  https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/log/?h=experimental-tests
> 
> Also we have an erofsstress to do long time random stress workloads,
> https://github.com/erofs/erofsstress
> 
> But yeah, it's some awkward that fstests idmapped mount testcases may
> be unsuitable for EROFS for now. I will add some new testcases to build
> images and test for this behavior.
> 
> > 
> > Looks good,
> > Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>
> 
> Thanks for your review!

Thanks for supporting this in erofs!
Christian

Re: [PATCH] erofs: support idmapped mounts

[Gao Xiang]

On Tue, May 17, 2022 at 11:22:03AM +0200, Christian Brauner wrote:
> On Tue, May 17, 2022 at 05:15:02PM +0800, Gao Xiang wrote:
> > Hi Christian,
> > 
> > On Tue, May 17, 2022 at 11:06:22AM +0200, Christian Brauner wrote:
> > > On Tue, May 17, 2022 at 03:32:10PM +0800, Chao Yu wrote:
> > > > This patch enables idmapped mounts for erofs, since all dedicated helpers
> > > > for this functionality existsm, so, in this patch we just pass down the
> > > > user_namespace argument from the VFS methods to the relevant helpers.
> > > > 
> > > > Simple idmap example on erofs image:
> > > > 
> > > > 1. mkdir dir
> > > > 2. touch dir/file
> > > > 3. mkfs.erofs erofs.img dir
> > > > 4. mount -t erofs -o loop erofs.img  /mnt/erofs/
> > > > 
> > > > 5. ls -ln /mnt/erofs/
> > > > total 0
> > > > -rw-rw-r-- 1 1000 1000 0 May 17 15:26 file
> > > > 
> > > > 6. mount-idmapped --map-mount b:0:1001:1 /mnt/erofs/ /mnt/scratch_erofs/
> > > > 
> > > > 7. ls -ln /mnt/scratch_erofs/
> > > > total 0
> > > > -rw-rw-r-- 1 65534 65534 0 May 17 15:26 file
> > > 
> > > Your current example maps id 0 in the filesystem to id 1001 in the
> > > mount. But since no files with id 0 exist in the filesystem you're
> > > illustrating that unmapped ids are correctly reported as overflow{g,u}id.
> > > 
> > > I think what you'd rather want to show is something like this:
> > > 
> > > 5. ls -ln /mnt/erofs/
> > > total 0
> > > -rw-rw-r-- 1 1000 1000 0 May 17 15:26 file
> > > 
> > > 6. mount-idmapped --map-mount b:1000:1001:1 /mnt/erofs/ /mnt/scratch_erofs/
> > > 
> > > 7. ls -ln /mnt/scratch_erofs/
> > > total 0
> > > -rw-rw-r-- 1 1001 1001 0 May 17 15:26 file
> > > 
> > > where id 1000 in the filesystem maps to id 1001 in the mount.

Yeah, I just manually tested, although some steps assume user 1000
and some steps assume the root user. But it works.

I will rephrase such commit messages when applying later...

Thanks,
Gao Xiang

Re: [PATCH] erofs: support idmapped mounts

[Gao Xiang]

On Tue, May 17, 2022 at 11:22:03AM +0200, Christian Brauner wrote:
> On Tue, May 17, 2022 at 05:15:02PM +0800, Gao Xiang wrote:
> > Hi Christian,
> > 
> > On Tue, May 17, 2022 at 11:06:22AM +0200, Christian Brauner wrote:
> > > On Tue, May 17, 2022 at 03:32:10PM +0800, Chao Yu wrote:
> > > > This patch enables idmapped mounts for erofs, since all dedicated helpers
> > > > for this functionality existsm, so, in this patch we just pass down the
> > > > user_namespace argument from the VFS methods to the relevant helpers.
> > > > 
> > > > Simple idmap example on erofs image:
> > > > 
> > > > 1. mkdir dir
> > > > 2. touch dir/file
> > > > 3. mkfs.erofs erofs.img dir
> > > > 4. mount -t erofs -o loop erofs.img  /mnt/erofs/
> > > > 
> > > > 5. ls -ln /mnt/erofs/
> > > > total 0
> > > > -rw-rw-r-- 1 1000 1000 0 May 17 15:26 file
> > > > 
> > > > 6. mount-idmapped --map-mount b:0:1001:1 /mnt/erofs/ /mnt/scratch_erofs/
> > > > 
> > > > 7. ls -ln /mnt/scratch_erofs/
> > > > total 0
> > > > -rw-rw-r-- 1 65534 65534 0 May 17 15:26 file
> > > 
> > > Your current example maps id 0 in the filesystem to id 1001 in the
> > > mount. But since no files with id 0 exist in the filesystem you're
> > > illustrating that unmapped ids are correctly reported as overflow{g,u}id.
> > > 
> > > I think what you'd rather want to show is something like this:
> > > 
> > > 5. ls -ln /mnt/erofs/
> > > total 0
> > > -rw-rw-r-- 1 1000 1000 0 May 17 15:26 file
> > > 
> > > 6. mount-idmapped --map-mount b:1000:1001:1 /mnt/erofs/ /mnt/scratch_erofs/
> > > 
> > > 7. ls -ln /mnt/scratch_erofs/
> > > total 0
> > > -rw-rw-r-- 1 1001 1001 0 May 17 15:26 file
> > > 
> > > where id 1000 in the filesystem maps to id 1001 in the mount.

Yeah, I just manually tested, although some steps assume user 1000
and some steps assume the root user. But it works.

I will rephrase such commit messages when applying later...

Thanks,
Gao Xiang