Re: [PATCH 03/13] NFSv4.2: query the server for extended attribute support

["Mkrtchyan, Tigran" <tigran.mkrtchyan@desy.de>]

----- Original Message -----
> From: "trondmy" <trondmy@hammerspace.com>
> To: "Frank van der Linden" <fllinden@amazon.com>, "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
> Cc: "linux-nfs" <linux-nfs@vger.kernel.org>, "Anna Schumaker" <anna.schumaker@netapp.com>
> Sent: Friday, March 13, 2020 2:50:38 PM
> Subject: Re: [PATCH 03/13] NFSv4.2: query the server for extended attribute support

> On Fri, 2020-03-13 at 12:11 +0100, Mkrtchyan, Tigran wrote:
>> Hi Frank,
>> 
>> I think the way how you have implemented is almost correct. You query
>> server for supported attributes. As result client will get all
>> attributes
>> supported bu the server and if FATTR4_XATTR_SUPPORT is returned, then
>> client
>> adds xattr capability. This the way how I read rfc8276. Do you have a
>> different
>> opinion?
>> 
> 
> 'xattr_support' seems like a protocol hack to allow the client to
> determine whether or not the xattr operations are supported.
> 
> The reason why it is a hack is that 'supported_attrs' is also a per-
> filesystem attribute, and there is no value in advertising
> 'xattr_support' there unless your filesystem also supports xattrs.
> 
> IOW: the protocol forces you to do 2 round trips to the server in order
> to figure out something that really should be obvious with 1 round
> trip.
> 


So you say  that client have to query for xattr_support every time the
fsid is changing?

Tigran.

>> Regards,
>>    Tigran.
>> 
>> ----- Original Message -----
>> > From: "Frank van der Linden" <fllinden@amazon.com>
>> > To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
>> > Cc: "Trond Myklebust" <trond.myklebust@hammerspace.com>, "Anna
>> > Schumaker" <anna.schumaker@netapp.com>, "linux-nfs"
>> > <linux-nfs@vger.kernel.org>
>> > Sent: Thursday, March 12, 2020 10:15:55 PM
>> > Subject: Re: [PATCH 03/13] NFSv4.2: query the server for extended
>> > attribute support
>> > On Thu, Mar 12, 2020 at 08:51:39PM +0000, Frank van der Linden
>> > wrote:
>> > > 1) The xattr_support attribute exists
>> > > 2) The xattr support attribute exists *and* it's true for the
>> > > root fh
>> > > 
>> > > Currently the code does 2) in one operation. That might not be
>> > > 100%
>> > > correct - the RFC does mention that (section 8.2):
>> > > 
>> > > "Before interrogating this attribute using GETATTR, a client
>> > > should
>> > >  determine whether it is a supported attribute by interrogating
>> > > the
>> > >  supported_attrs attribute."
>> > > 
>> > > That's a "should", not a "MUST", but it's still waving its finger
>> > > at you not to do this.
>> > > 
>> > > Since 8.2.1 says:
>> > > 
>> > > "However, a client may reasonably assume that a server
>> > >  (or file system) that does not support the xattr_support
>> > > attribute
>> > >  does not provide xattr support, and it acts on that basis."
>> > > 
>> > > ..I think you're right, and the code should just use the
>> > > existence
>> > > of the attribute as a signal that the server knows about xattrs -
>> > > operations should still error out correctly if it doesn't.
>> > > 
>> > > I'll make that change, thanks.
>> > 
>> > ..or, alternatively, only query xattr_support in
>> > nfs4_server_capabilities,
>> > and then its actual value, if it exists, in nfs4_fs_info.
>> > 
>> > Any opinions on this?
>> > 
>> > - Frank
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com