Received-SPF: pass (google.com: best guess record for domain of linux-nfs-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
IronPort-SDR: T3gqtetQ+iQ5XBsEvHJitUs9TfbZycOKnEuz2vGsFrgiE2QONkwf+h0yxc7zEpn7Dh8xM4RQlc
 /w0lfngeaSwA==
Date:   Fri, 13 Mar 2020 16:07:02 +0000
From:   Frank van der Linden <fllinden@amazon.com>
To:     "J. Bruce Fields" <bfields@fieldses.org>
CC:     <chuck.lever@oracle.com>, <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 02/14] xattr: modify vfs_{set, remove}xattr for NFS
 server use
Message-ID: <20200313160702.GA31307@dev-dsk-fllinden-2c-c1893d73.us-west-2.amazon.com>
References: <20200311195954.27117-1-fllinden@amazon.com>
 <20200311195954.27117-3-fllinden@amazon.com>
 <20200313153549.GD12537@fieldses.org>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20200313153549.GD12537@fieldses.org>
User-Agent: Mutt/1.11.4 (2019-03-13)
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

On Fri, Mar 13, 2020 at 11:35:49AM -0400, J. Bruce Fields wrote:
> 
> 
> On Wed, Mar 11, 2020 at 07:59:42PM +0000, Frank van der Linden wrote:
> > Second, RFC 8276 (NFSv4 extended attribute support) specifies that
> > delegations should be recalled (8.4.2.4, 8.4.4.4) when a SETXATTR
> > or REMOVEXATTR operation is performed. So, like with other fs
> > operations, try to break the delegation. The _locked version of
> > these operations will not wait for the delegation to be successfully
> > broken, instead returning an error if it wasn't, so that the NFS
> > server code can return NFS4ERR_DELAY to the client (similar to
> > what e.g. vfs_link does).
> 
> Is there a preexisting bug here?  Even without NFS support for xattrs, a
> local setxattr on the filesystem should still revoke any delegations
> held by remote NFS clients.  I couldn't tell whether we're getting that
> right from a quick look at the current code.
> 
> --b.

I think there's currently a bug if that's the expected behavior, yes.
Attribute changes will call notify_change(), and other methods (unlink,
link, rename) call try_break_deleg(). But the xattr entry points
don't do that, which is why I added it.

- Frank