On Mon, 2023-05-15 at 13:11 -0400, Jeff Layton wrote:
> On Mon, 2023-05-15 at 11:50 +0000, Ondrej Valousek wrote:
> > Hi Paul,
> >
> > Ok first of all, thanks for taking initiative on this, I am unable
> > to proceed on this on my own at the moment.
> > I see few problems with this:
> >
> > 1. The calculation of the 'listbufsize' is incorrect in your patch.
> > It will _not_work as you expected and won't limit the number of
> > syscalls (which is why we came up with this patch, right?). Check
> > with my original proposal, we really need to check for
> > 'system.nfs4' xattr name presence here
> > 2. It mistakenly detects an ACL presence on files which do not have
> > any ACL on NFSv4 filesystem. Digging further it seems that kernel
> > in F39 behaves differently to the previous kernels:
> >
> > F38:
> > # getfattr -m . /path_to_nfs4_file
> > # file: path_to_nfs4_file
> > system.nfs4_acl <---- only
> > single xattr detected
> >
> > F39:
> > # getfattr -m . /path_to_nfs4_file
> > # file: path_to_nfs4_file
> > system.nfs4_acl
> > system.posix_acl_default
> > /* SOMETIMES even shows this */
> > system.posix_acl_default
>
> (cc'ing Christian and relevant kernel lists)
>
> I assume the F39 kernel is v6.4-rc based? If so, then I think that's
> a
> regression. NFSv4 client inodes should _not_ report a POSIX ACL
> attribute since the protocol doesn't support them.
>
> In fact, I think the rationale in the kernel commit below is wrong.
> NFSv4 has a listxattr operation, but doesn't support POSIX ACLs.
>
> Christian, do we need to revert this?
>
> commit e499214ce3ef50c50522719e753a1ffc928c2ec1
> Author: Christian Brauner <brauner@xxxxxxxxxx>
> Date: Wed Feb 1 14:15:01 2023 +0100
>
> acl: don't depend on IOP_XATTR
>
>
No. The problem is commit f2620f166e2a ("xattr: simplify listxattr
helpers") which helpfully inserts posix acl handlers into
generic_listxattr(), and makes it impossible to call from
nfs4_listxattr().
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
