All of lore.kernel.org
 help / color / mirror / Atom feed
From: Trond Myklebust <trondmy@hammerspace.com>
To: "fllinden@amazon.com" <fllinden@amazon.com>,
	"tigran.mkrtchyan@desy.de" <tigran.mkrtchyan@desy.de>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
	"anna.schumaker@netapp.com" <anna.schumaker@netapp.com>
Subject: Re: [PATCH 03/13] NFSv4.2: query the server for extended attribute support
Date: Fri, 13 Mar 2020 13:50:38 +0000	[thread overview]
Message-ID: <6792d6a6012a241b8bd1555eea8c592ff318a444.camel@hammerspace.com> (raw)
In-Reply-To: <948465413.4651196.1584097887947.JavaMail.zimbra@desy.de>

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



  reply	other threads:[~2020-03-13 13:50 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-11 19:56 [PATCH 00/13] client side user xattr (RFC8276) support Frank van der Linden
2020-03-11 19:56 ` [PATCH 01/13] nfs,nfsd: NFSv4.2 extended attribute protocol definitions Frank van der Linden
2020-03-11 19:56 ` [PATCH 02/13] nfs: add client side only definitions for user xattrs Frank van der Linden
2020-03-11 19:56 ` [PATCH 03/13] NFSv4.2: query the server for extended attribute support Frank van der Linden
2020-03-12 16:15   ` Mkrtchyan, Tigran
2020-03-12 20:51     ` Frank van der Linden
2020-03-12 21:15       ` Frank van der Linden
2020-03-13 11:11         ` Mkrtchyan, Tigran
2020-03-13 13:50           ` Trond Myklebust [this message]
2020-03-13 14:19             ` Mkrtchyan, Tigran
2020-03-13 17:10               ` Trond Myklebust
2020-03-13 17:55             ` Frank van der Linden
2020-03-11 19:56 ` [PATCH 04/13] NFSv4.2: define limits and sizes for user xattr handling Frank van der Linden
2020-03-12 20:35   ` Schumaker, Anna
2020-03-11 19:56 ` [PATCH 05/13] NFSv4.2: add client side XDR handling for extended attributes Frank van der Linden
2020-03-12 20:49   ` Schumaker, Anna
2020-03-11 19:56 ` [PATCH 06/13] nfs: define nfs_access_get_cached function Frank van der Linden
2020-03-11 19:56 ` [PATCH 07/13] NFSv4.2: query the extended attribute access bits Frank van der Linden
2020-03-11 19:56 ` [PATCH 08/13] nfs: modify update_changeattr to deal with regular files Frank van der Linden
2020-03-11 19:56 ` [PATCH 09/13] nfs: define and use the NFS_INO_INVALID_XATTR flag Frank van der Linden
2020-03-24  6:02   ` [nfs] c5654df66d: stress-ng.msg.ops_per_sec 15.5% improvement kernel test robot
2020-03-24 16:21     ` Frank van der Linden
2020-03-11 19:56 ` [PATCH 10/13] nfs: make the buf_to_pages_noslab function available to the nfs code Frank van der Linden
2020-03-12 20:36   ` Schumaker, Anna
2020-03-11 19:56 ` [PATCH 11/13] NFSv4.2: add the extended attribute proc functions Frank van der Linden
2020-03-11 19:56 ` [PATCH 12/13] NFSv4.2: hook in the user extended attribute handlers Frank van der Linden
2020-03-11 19:56 ` [PATCH 13/13] NFSv4.2: add client side xattr caching Frank van der Linden
2020-03-12 20:39   ` Schumaker, Anna
2020-03-12 20:48   ` Schumaker, Anna
2020-03-12 19:06 ` [PATCH 00/13] client side user xattr (RFC8276) support Mkrtchyan, Tigran
2020-03-12 20:09 ` Anna Schumaker
2020-03-16 15:50   ` Frank van der Linden
2020-03-17 23:03   ` Frank van der Linden
2020-03-19 14:39     ` J. Bruce Fields

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6792d6a6012a241b8bd1555eea8c592ff318a444.camel@hammerspace.com \
    --to=trondmy@hammerspace.com \
    --cc=anna.schumaker@netapp.com \
    --cc=fllinden@amazon.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=tigran.mkrtchyan@desy.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.