Re: [PATCH v4 03/15] fs/fuse: add an idmap argument to fuse_simple_request

[Christian Brauner <brauner@kernel.org>]

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@kernel.org>
> Cc: Seth Forshee <sforshee@kernel.org>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Amir Goldstein <amir73il@gmail.com>
> Cc: Bernd Schubert <bschubert@ddn.com>
> Cc: <linux-fsdevel@vger.kernel.org>
> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
> ---

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.