Re: [PATCH 3/4] libfuse: add statx support to the lower level library

["Darrick J. Wong" <djwong@kernel.org>]

On Fri, Jul 18, 2025 at 03:28:25PM +0200, Amir Goldstein wrote:
> On Fri, Jul 18, 2025 at 1:39 AM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > From: Darrick J. Wong <djwong@kernel.org>
> >
> > Add statx support to the lower level fuse library.
> 
> This looked familiar.
> Merged 3 days ago:
> https://github.com/libfuse/libfuse/pull/1026
> 
> >
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > ---
> >  include/fuse_lowlevel.h |   37 ++++++++++++++++++
> >  lib/fuse_lowlevel.c     |   97 +++++++++++++++++++++++++++++++++++++++++++++++
> >  lib/fuse_versionscript  |    2 +
> >  3 files changed, 136 insertions(+)

<snip>

> > diff --git a/lib/fuse_lowlevel.c b/lib/fuse_lowlevel.c
> > index ec30ebc4cdd074..8eeb6a8547da91 100644
> > --- a/lib/fuse_lowlevel.c
> > +++ b/lib/fuse_lowlevel.c
> > @@ -144,6 +144,43 @@ static void convert_attr(const struct fuse_setattr_in *attr, struct stat *stbuf)
> >         ST_CTIM_NSEC_SET(stbuf, attr->ctimensec);
> >  }
> >
> > +#ifdef STATX_BASIC_STATS
> > +static int convert_statx(struct fuse_statx *stbuf, const struct statx *stx,
> > +                        size_t size)
> > +{
> > +       if (sizeof(struct statx) != size)
> > +               return EOPNOTSUPP;
> > +
> > +       stbuf->mask = stx->stx_mask & (STATX_BASIC_STATS | STATX_BTIME);
> > +       stbuf->blksize          = stx->stx_blksize;
> > +       stbuf->attributes       = stx->stx_attributes;
> > +       stbuf->nlink            = stx->stx_nlink;
> > +       stbuf->uid              = stx->stx_uid;
> > +       stbuf->gid              = stx->stx_gid;
> > +       stbuf->mode             = stx->stx_mode;
> > +       stbuf->ino              = stx->stx_ino;
> > +       stbuf->size             = stx->stx_size;
> > +       stbuf->blocks           = stx->stx_blocks;
> > +       stbuf->attributes_mask  = stx->stx_attributes_mask;
> > +       stbuf->rdev_major       = stx->stx_rdev_major;
> > +       stbuf->rdev_minor       = stx->stx_rdev_minor;
> > +       stbuf->dev_major        = stx->stx_dev_major;
> > +       stbuf->dev_minor        = stx->stx_dev_minor;
> > +
> > +       stbuf->atime.tv_sec     = stx->stx_atime.tv_sec;
> > +       stbuf->btime.tv_sec     = stx->stx_btime.tv_sec;
> > +       stbuf->ctime.tv_sec     = stx->stx_ctime.tv_sec;
> > +       stbuf->mtime.tv_sec     = stx->stx_mtime.tv_sec;
> > +
> > +       stbuf->atime.tv_nsec    = stx->stx_atime.tv_nsec;
> > +       stbuf->btime.tv_nsec    = stx->stx_btime.tv_nsec;
> > +       stbuf->ctime.tv_nsec    = stx->stx_ctime.tv_nsec;
> > +       stbuf->mtime.tv_nsec    = stx->stx_mtime.tv_nsec;
> > +
> > +       return 0;
> > +}
> > +#endif
> > +
> 
> Why is this conversion not needed in the merged version?
> What am I missing?

The patch in upstream memcpy's struct statx to struct fuse_statx:

	memset(&arg, 0, sizeof(arg));
	arg.flags = flags;
	arg.attr_valid = calc_timeout_sec(attr_timeout);
	arg.attr_valid_nsec = calc_timeout_nsec(attr_timeout);
	memcpy(&arg.stat, statx, sizeof(arg.stat));

As long as the fields in the two are kept exactly in sync, this isn't a
problem and no explicit struct conversion is necessary.

I also noticed that the !HAVE_STATX variant of _do_statx doesn't call
fuse_reply_err(req, ENOSYS).  I think that means a new kernel calling
an old userspace would never receive a reply to a FUSE_STATX command
and ... time out?

My version also has explicit sizing of struct statx, but I concede that
if that struct ever gets bigger we're going to have to rev the whole
syscall anyway.  I was being perhaps a bit paranoid.

BTW, where are libfuse patches reviewed?  I guess all the review are
done via github PRs?

--D

> Thanks,
> Amir.
> 
> >  static size_t iov_length(const struct iovec *iov, size_t count)
> >  {
> >         size_t seg;
> > @@ -2653,6 +2690,64 @@ static void do_syncfs(fuse_req_t req, const fuse_ino_t nodeid, const void *inarg
> >         _do_syncfs(req, nodeid, inarg, NULL);
> >  }
> >
> > +#ifdef STATX_BASIC_STATS
> > +int fuse_reply_statx(fuse_req_t req, const struct statx *statx, size_t size,
> > +                    double attr_timeout)
> > +{
> > +       struct fuse_statx_out arg = {
> > +               .attr_valid = calc_timeout_sec(attr_timeout),
> > +               .attr_valid_nsec = calc_timeout_nsec(attr_timeout),
> > +       };
> > +
> > +       int err = convert_statx(&arg.stat, statx, size);
> > +       if (err) {
> > +               fuse_reply_err(req, err);
> > +               return err;
> > +       }
> > +
> > +       return send_reply_ok(req, &arg, sizeof(arg));
> > +}
> > +
> > +static void _do_statx(fuse_req_t req, const fuse_ino_t nodeid,
> > +                     const void *op_in, const void *in_payload)
> > +{
> > +       (void)in_payload;
> > +       const struct fuse_statx_in *arg = op_in;
> > +       struct fuse_file_info *fip = NULL;
> > +       struct fuse_file_info fi;
> > +
> > +       if (arg->getattr_flags & FUSE_GETATTR_FH) {
> > +               memset(&fi, 0, sizeof(fi));
> > +               fi.fh = arg->fh;
> > +               fip = &fi;
> > +       }
> > +
> > +       if (req->se->op.statx)
> > +               req->se->op.statx(req, nodeid, arg->sx_flags, arg->sx_mask,
> > +                                 fip);
> > +       else
> > +               fuse_reply_err(req, ENOSYS);
> > +}
> > +#else
> > +int fuse_reply_statx(fuse_req_t req, const struct statx *statx,
> > +                    double attr_timeout)
> > +{
> > +       fuse_reply_err(req, ENOSYS);
> > +       return -ENOSYS;
> > +}
> > +
> > +static void _do_statx(fuse_req_t req, const fuse_ino_t nodeid,
> > +                     const void *op_in, const void *in_payload)
> > +{
> > +       fuse_reply_err(req, ENOSYS);
> > +}
> > +#endif /* STATX_BASIC_STATS */
> > +
> > +static void do_statx(fuse_req_t req, const fuse_ino_t nodeid, const void *inarg)
> > +{
> > +       _do_statx(req, nodeid, inarg, NULL);
> > +}
> > +
> >  static bool want_flags_valid(uint64_t capable, uint64_t want)
> >  {
> >         uint64_t unknown_flags = want & (~capable);
> > @@ -3627,6 +3722,7 @@ static struct {
> >         [FUSE_COPY_FILE_RANGE] = { do_copy_file_range, "COPY_FILE_RANGE" },
> >         [FUSE_LSEEK]       = { do_lseek,       "LSEEK"       },
> >         [FUSE_SYNCFS]      = { do_syncfs,       "SYNCFS"     },
> > +       [FUSE_STATX]       = { do_statx,       "STATX"       },
> >         [FUSE_IOMAP_CONFIG]= { do_iomap_config, "IOMAP_CONFIG" },
> >         [FUSE_IOMAP_BEGIN] = { do_iomap_begin,  "IOMAP_BEGIN" },
> >         [FUSE_IOMAP_END]   = { do_iomap_end,    "IOMAP_END" },
> > @@ -3686,6 +3782,7 @@ static struct {
> >         [FUSE_COPY_FILE_RANGE]  = { _do_copy_file_range, "COPY_FILE_RANGE" },
> >         [FUSE_LSEEK]            = { _do_lseek,          "LSEEK" },
> >         [FUSE_SYNCFS]           = { _do_syncfs,         "SYNCFS" },
> > +       [FUSE_STATX]            = { _do_statx,          "STATX" },
> >         [FUSE_IOMAP_CONFIG]     = { _do_iomap_config,   "IOMAP_CONFIG" },
> >         [FUSE_IOMAP_BEGIN]      = { _do_iomap_begin,    "IOMAP_BEGIN" },
> >         [FUSE_IOMAP_END]        = { _do_iomap_end,      "IOMAP_END" },
> > diff --git a/lib/fuse_versionscript b/lib/fuse_versionscript
> > index dc9fa2428b5325..a67b1802770335 100644
> > --- a/lib/fuse_versionscript
> > +++ b/lib/fuse_versionscript
> > @@ -223,6 +223,8 @@ FUSE_3.18 {
> >                 fuse_reply_iomap_config;
> >                 fuse_lowlevel_notify_iomap_upsert;
> >                 fuse_lowlevel_notify_iomap_inval;
> > +
> > +               fuse_reply_statx;
> >  } FUSE_3.17;
> >
> >  # Local Variables:
> >
> >
>