On Fri, 2020-03-13 at 15:19 +0100, Mkrtchyan, Tigran wrote: > > ----- 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? > According to the spec it is a per-filesystem attribute, just like supported_attrs... Cheers Trond > 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 -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@xxxxxxxxxxxxxxx