On Mon, Nov 04, 2019 at 05:58:46PM -0500, Bruce Fields wrote: > 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. Sounds good, I'll go with that. > > > 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. > > 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.) Sure. So, to recap, here's what I'll do: * Remove the CONFIG options - enable the code by default. * Server side: * Remove the export file option to not export extended attributes * Client side: * Always probe user xattr support, and use it when available (returning EOPNOTSUPP, as expected, for all xattr syscalls if it is not available). Thanks for the feedback so far - much appreciated. I'll submit the separate client and server patch sets this week. - Frank
