To summarize, there are 2 things we are discussing here: 1. Allow READ operation when OPEN is done with SHARE_ACCESS_WRITE and WRITE delegation is granted. This is not happening today and we all agree that we need to fix this. 2. In nfs4_read() function, we want to check if any OPENs from other clients exist with 'deny=read' and fail the READ(for both with and without delegation). But the point I am trying to make is, we are not checking here for other clients' deny modes but we are checking our own deny mode. So this will mean, If say client A does an OPEN with "share_access = write, deny = read", it will deny clientA to read as well instead of just denying READ ops from other clients. /* Even if file is open for write, the client * may do accidently read operation (caching). * Because of this, READ is allowed if not * explicitly denied. See page 112 in RFC 7530 * for more details. */ if (state_open->state_data.share.share_deny &. <---- this is the deny mode of the state which came in READ op. OPEN4_SHARE_DENY_READ) { /* Bad open mode, return NFS4ERR_OPENMODE */ res_READ4->status = NFS4ERR_OPENMODE; Please correct me if I am wrong in my understanding here. Regards, Deepthi On 06/02/20, 12:46 AM, "Frank Filz" <ffilzlnx@mindspring.com> wrote: > -----Original Message----- > From: Jeff Layton [mailto:jlayton@redhat.com] > Sent: Wednesday, February 5, 2020 11:09 AM > To: Soumya Koduri <skoduri@redhat.com>; Deepthi Shivaramu > <des@vmware.com>; devel@lists.nfs-ganesha.org > Cc: Frank Filz <ffilz@redhat.com> > Subject: [NFS-Ganesha-Devel] Re: NFSv4 Read not allowed on file opened with > OPEN4_SHARE_ACCESS_WRITE and got write > delegation(OPEN_DELEGATE_WRITE) > > On Thu, 2020-02-06 at 00:17 +0530, Soumya Koduri wrote: > > Hi Deepthi, > > > > On 2/5/20 4:28 PM, Deepthi Shivaramu wrote: > > > Soumya, > > > I got your point that open_state may not exist when the reads are done with > deleg_state, so you need this block of code. So the check here should have been > similar to the check done below as per RFC. > > > > > > But I did not understand the point of checking share_deny as well. When a > client performs open with share_deny set to say read, it means other clients > should be denied read but not itself right? So what is the point of checking my > own 'sdeleg->share_deny' or even 'state_open->state_data.share.share_deny' > as done in share lock check below. > > > > you are right. Those checks would have already been performed in OPEN, > > unless its special stateID and it bypasses OPEN which in case > > state_found shall be NULL and there are checks for Deleg and share > > conflicts in the subsequent block. > > Request Frank/Jeff to comment on if/why its needed for regular READ > > and if the same applies in case of delegation as well. > > > > It looks like there is a comment that explains this in the code: > > /* Even if file is open for write, the client > * may do accidently read operation (caching). > * Because of this, READ is allowed if not > * explicitly denied. See page 112 in RFC 7530 > * for more details. > */ > > IIUC -- the client can do a read on a write-only stateid -- eg to do a read-modify- > write cycle to update a full page in the pagecache. The server _should_ generally > allow that, but if someone has set DENY_READ then it should not. Thus the > check. > > I'd suggest (though I'm not sure) that you still need to check whether you have a > delegation in play or not. One would hope that sane servers would avoid passing > out delegations that might conflict with deny modes. > > In practice, it's probably easiest to just assume that an open for WRITE also > implies READ access. The RFC seems to allow for that (see p112 again). > > If you have real questions about this, you may want to email nfsv4@ietf.org and > ask for clarification there. Yea, it looks like we should allow READ on a WRITE Delegation stateid. We probably should not grant a WRITE delegation if there is a conflicting OPEN with Deny Read (which unless we should rarely see, usually you either see Deny Write or Deny Read/Write, though one can probably make a case for OPEN Write/Deny Write, OPEN Read/Deny Read for some kind of pipe like thing that will have a single producer and a single consumer. On the other hand, a delegation would be inappropriate for either side of that... Frank