On 7/8/24 7:18 AM, Chuck Lever III wrote:
On Jul 7, 2024, at 7:06 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
On Sun, 2024-07-07 at 01:42 +0300, Sagi Grimberg wrote:
Many applications open files with O_WRONLY, fully intending to write
into the opened file. There is no reason why these applications should
not enjoy a write delegation handed to them.
Cc: Dai Ngo <dai.ngo@xxxxxxxxxx>
Signed-off-by: Sagi Grimberg <sagi@xxxxxxxxxxx>
---
Note: I couldn't find any reason to why the initial implementation chose
to offer write delegations only to NFS4_SHARE_ACCESS_BOTH, but it seemed
like an oversight to me. So I figured why not just send it out and see who
objects...
fs/nfsd/nfs4state.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a20c2c9d7d45..69d576b19eb6 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5784,15 +5784,15 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
* "An OPEN_DELEGATE_WRITE delegation allows the client to handle,
* on its own, all opens."
*
- * Furthermore the client can use a write delegation for most READ
- * operations as well, so we require a O_RDWR file here.
- *
- * Offer a write delegation in the case of a BOTH open, and ensure
- * we get the O_RDWR descriptor.
+ * Offer a write delegation in the case of a BOTH open (ensure
+ * a O_RDWR descriptor) Or WRONLY open (with a O_WRONLY descriptor).
*/
if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_BOTH) {
nf = find_rw_file(fp);
dl_type = NFS4_OPEN_DELEGATE_WRITE;
+ } else if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) {
+ nf = find_writeable_file(fp);
+ dl_type = NFS4_OPEN_DELEGATE_WRITE;
}
/*
Thanks Sagi, I'm glad to see this posting!
I *think* the main reason we limited this before is because a write
delegation is really a read/write delegation. There is no such thing as
a write-only delegation.
I recall (quite dimly) that Dai found some bad behavior
in a subtle corner case, so we decided to leave this on
the table as a possible future enhancement. Adding Dai.
If I remember correctly, granting write delegation on OPEN with
NFS4_SHARE_ACCESS_WRITE without additional changes causes the git
test to fail.
The cause of the failure is because the client does read-modify-write
using the write delegation stateid. This happens when an application
does partial write the client side, the Linux client reads the whole
page from the server, modifies a section and writes the whole page
back. I think this is the case with the t0000-basic test in the git
test suite.
I think this behavior is allowed in section 9.1.2 of RFC 8881.
-Dai
Suppose the user is prevented from doing reads against the inode (by
permission bits or ACLs). The server gives out a WRITE delegation on a
O_WRONLY open. Will the client allow cached opens for read regardless
of the server's permissions? Or, does it know to check vs. the server
if the client tries to do an open for read in this situation?
My understanding is that a write delegation is no more
than a promise by the server to tell the client if another
client wants access to the file. So granting a write
delegation on a read-only or write-only OPEN should be
fine to do (at the discretion of the server, of course).
The issue about the ACE is moot for NFSD right now because
NFSD returns an empty ACE. That should require the client to
continue to send ACCESS operations to the server as needed.
--
Chuck Lever
