Re: [PATCH v4 03/15] fs/fuse: add an idmap argument to fuse_simple_request
From: Christian Brauner
Date: Wed Sep 04 2024 - 11:26:23 EST
On Tue, Sep 03, 2024 at 05:16:14PM GMT, Alexander Mikhalitsyn wrote:
> If idmap == NULL *and* filesystem daemon declared idmapped mounts
> support, then uid/gid values in a fuse header will be -1.
>
> No functional changes intended.
>
> Cc: Christian Brauner <brauner@xxxxxxxxxx>
> Cc: Seth Forshee <sforshee@xxxxxxxxxx>
> Cc: Miklos Szeredi <miklos@xxxxxxxxxx>
> Cc: Amir Goldstein <amir73il@xxxxxxxxx>
> Cc: Bernd Schubert <bschubert@xxxxxxx>
> Cc: <linux-fsdevel@xxxxxxxxxxxxxxx>
> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@xxxxxxxxxxxxx>
> ---
Huha, you end up extending fuse_simple_request() with an idmap argument
and passing NULL 38 times and non-NULL only 4 times at the end of this
patch series. That's not pretty. Also, I really dislike passing NULL as
an argument to the idmap helpers. All of the idmapping code uses
nop_mnt_idmap for this case and I think we should the same just with
invalid_mnt_idmap constant.
So I would propose two changes:
(1) Add an extern invalid_mnt_idmap into mnt_idmapping.h and
define it in fs/mnt_idmapping.c so that will always yield
INVALID_VFSUID/INVALID_VFSGID. Basically, it's the same definition
as for nop_mnt_idmap.
(2) Instead of extending fuse_simple_request() with an additional
argument rename fuse_simple_request() to __fuse_simple_request()
and extend __fuse_simple_request() with a struct mnt_idmap argument.
* make fuse_simple_request() a static inline helper that calls
__fuse_simple_request() with invalid_mnt_idmap in the fuse_i.h
header.
* add fuse_idmap_request() that also calls __fuse_simple_request()
but just passes through the idmap argument.