Re: [RFC PATCH 00/35] user xattr support (RFC8276)

From: Chuck Lever <chucklever@gmail.com>
To: Bruce Fields <bfields@fieldses.org>,
	Frank van der Linden <fllinden@amazon.com>
Cc: Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [RFC PATCH 00/35] user xattr support (RFC8276)
Date: Tue, 5 Nov 2019 10:44:06 -0500
Message-ID: <D1661F81-0232-4A35-B6BD-5857BF2D65A3@gmail.com> (raw)
In-Reply-To: <20191104225846.GA13469@fieldses.org>

> On Nov 4, 2019, at 5:58 PM, Bruce Fields <bfields@fieldses.org> 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@gmail.com