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

Mkrtchyan, Tigran
2020-03-13T11:11:31Z

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?

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