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 06:54:44PM +0200, Bernd Schubert wrote:
> 
> 
> On 7/18/25 18:27, Darrick J. Wong wrote:
> > 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?
> 
> Yeah, typical procedure is github PR. If preferred for these complex
> patches fine with me to post them here. Especially if others like Amir
> are going to review :)

Ok.  I prefer emailing fsdevel for the broader reach and the
LF-maintained permanent archives of the discussions.

(I really dislike email for actually sending things that are ready for
merging and would rather send PRs though.  In XFSland the PRs are more
or less a formality that comes _after_ months of arguing. :P)

--D

> 
> Thanks,
> Bernd
> 
> 
> 
>