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

Trond Myklebust
2020-03-13T13:50:45Z

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.

> 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