Re: [RFC PATCH v2 4/6] fuse: implementation of the FUSE_LOOKUP_HANDLE operation

[Luis Henriques <luis@igalia.com>]

On Wed, Dec 17 2025, Amir Goldstein wrote:

> On Tue, Dec 16, 2025 at 12:48 PM Luis Henriques <luis@igalia.com> wrote:
>>
>> On Mon, Dec 15 2025, Bernd Schubert wrote:
>>
>> > On 12/12/25 19:12, Luis Henriques wrote:
>> >> The implementation of LOOKUP_HANDLE modifies the LOOKUP operation to include
>> >> an extra inarg: the file handle for the parent directory (if it is
>> >> available).  Also, because fuse_entry_out now has a extra variable size
>> >> struct (the actual handle), it also sets the out_argvar flag to true.
>> >>
>> >> Most of the other modifications in this patch are a fallout from these
>> >> changes: because fuse_entry_out has been modified to include a variable size
>> >> struct, every operation that receives such a parameter have to take this
>> >> into account:
>> >>
>> >>    CREATE, LINK, LOOKUP, MKDIR, MKNOD, READDIRPLUS, SYMLINK, TMPFILE
>> >>
>> >> Signed-off-by: Luis Henriques <luis@igalia.com>
>> >> ---
>> >>   fs/fuse/dev.c             | 16 +++++++
>> >>   fs/fuse/dir.c             | 87 ++++++++++++++++++++++++++++++---------
>> >>   fs/fuse/fuse_i.h          | 34 +++++++++++++--
>> >>   fs/fuse/inode.c           | 69 +++++++++++++++++++++++++++----
>> >>   fs/fuse/readdir.c         | 10 ++---
>> >>   include/uapi/linux/fuse.h |  8 ++++
>> >>   6 files changed, 189 insertions(+), 35 deletions(-)
>> >>
>> >> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
>> >> index 629e8a043079..fc6acf45ae27 100644
>> >> --- a/fs/fuse/dev.c
>> >> +++ b/fs/fuse/dev.c
>> >> @@ -606,6 +606,22 @@ static void fuse_adjust_compat(struct fuse_conn *fc, struct fuse_args *args)
>> >>      if (fc->minor < 4 && args->opcode == FUSE_STATFS)
>> >>              args->out_args[0].size = FUSE_COMPAT_STATFS_SIZE;
>> >>
>> >> +    if (fc->minor < 45) {
>> >
>> > Could we use fc->lookup_handle here? Numbers are hard with backports
>>
>> To be honest, I'm not sure this code is correct.  I just followed the
>> pattern.  I'll need to dedicate some more time looking into this,
>> specially because the READDIRPLUS op handling is still TBD.
>>
>> <snip>
>>
>> >> @@ -505,6 +535,30 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
>> >>      if (!inode)
>> >>              return NULL;
>> >>
>> >> +    fi = get_fuse_inode(inode);
>> >> +    if (fc->lookup_handle) {
>> >> +            if ((fh == NULL) && (nodeid != FUSE_ROOT_ID)) {
>> >> +                    pr_err("NULL file handle for nodeid %llu\n", nodeid);
>> >> +                    iput(inode);
>> >> +                    return NULL;
>> >
>> > Hmm, so there are conditions like "if (fi && fi->fh) {" in lookup and I
>> > was thinking "nice, fuse-server can decide to skip the fh for some
>> > inodes like FUSE_ROOT_ID. But now it gets forbidden here. In combination
>> > with the other comment in fuse_inode_handle_alloc(), could be allocate
>> > here to the needed size and allow fuse-server to not send the handle
>> > for some files?
>>
>> I'm not sure the code is consistent with this regard, but here I'm doing
>> exactly that: allowing the fh to be NULL only for FUSE_ROOT_ID.  Or did I
>> misunderstood your comment?
>>
>
> root inode is a special case.
> The NFS server also does not encode the file handle for export root as
> far as a I know
> it just sends the special file handle type FILEID_ROOT to describe the
> root inode
> without any blob unique, so FUSE can do the same.

OK, that makes sense.

> There is not much point in "looking up" the root inode neither by nodeid
> nor by handle. unless is for making the code more generic.
>
> I am not sure if FUSE server restart is supposed to revalidate the
> root inode by file handle. That's kind of an administrative question about
> the feature. My feeling is that it is not needed.

Thanks, Amir.  Looks like there's a lot in these v2 review comments that
I'll need to go through.  I'll try to put everything together and see what
I can cook for v3.

Cheers,
-- 
Luís