----- Original Message ----- > From: "trondmy" <trondmy@xxxxxxxxxxxxxxx> > To: "Frank van der Linden" <fllinden@xxxxxxxxxx>, "Tigran Mkrtchyan" <tigran.mkrtchyan@xxxxxxx> > Cc: "linux-nfs" <linux-nfs@xxxxxxxxxxxxxxx>, "Anna Schumaker" <anna.schumaker@xxxxxxxxxx> > 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@xxxxxxxxxx> >> > To: "Tigran Mkrtchyan" <tigran.mkrtchyan@xxxxxxx> >> > Cc: "Trond Myklebust" <trond.myklebust@xxxxxxxxxxxxxxx>, "Anna >> > Schumaker" <anna.schumaker@xxxxxxxxxx>, "linux-nfs" >> > <linux-nfs@xxxxxxxxxxxxxxx> >> > 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@xxxxxxxxxxxxxxx