> On Nov 4, 2019, at 5:58 PM, Bruce Fields <bfields@xxxxxxxxxxxx> wrote: > > On Mon, Nov 04, 2019 at 04:21:47PM +0000, Frank van der Linden wrote: >> On Mon, Nov 04, 2019 at 10:36:03AM -0500, Chuck Lever wrote: >>> >>> Following the server's local file systems' mount options seems like a >>> good way to go. In particular, is there a need to expose user xattrs >>> on the server host, but prevent NFS clients' access to them? I can't >>> think of one. >> >> Ok, that sounds fine to me - I'll remove the user_xattr export flag, >> and we had already agreed to do away with the CONFIGs. >> >> That leaves one last item with regard to enabling support: the client side >> mount option. I assume the [no]user_xattr option should work the same as >> with other filesystems. What about the default setting? > > Just checking code for other filesystems quickly; if I understand right: > > - ext4 has user_xattr and nouser_xattr options, but you get a > deprecation warning if you try to use the latter; > - xfs doesn't support either option; > - cifs supports both, with xattr support the default. > > Not necessarily my call, but just for simplicity's sake, I'd probably > leave out the option and see if anybody complains. Agree, I would leave it out to begin with. Anyone on linux-fsdevel, feel free to chime in here about why some other file systems have this mount option. History lessons welcome. >> Also, currently, my code does not fail the mount operation if user xattrs >> are asked for, but the server does not support them. It just doesn't >> set NFS_CAP_XATTR for the server, and the xattr handler entry points >> eturn -EOPNOTSUPP, as they check for NFS_CAP_XATTR. What's the preferred >> behavior there? > > getxattr(2) under ERRORS says: > > ENOTSUP > Extended attributes are not supported by the filesystem, > or are disabled. > > so I'm guessing just erroring out is clearest. IMO on the client, we want getxattr failure behavior to be consistent among: - A version of NFS that does not support xattrs at all (say, v3) - A version of NFS that can support them but doesn't (say, NFSv4.2 before these patches) - A version of NFS that can support them, but the server doesn't - A version of NFS that can support them, a server that can support them, but it's filesystem doesn't > I also see there's an EOPNOTSUPP return in the nouser_xattr case in > ext4_xattr_user_get. (errno(3) says ENOTSUP and EOPNOTSUPP are the same > value on Linux.) > > --b. -- Chuck Lever chucklever@xxxxxxxxx