On 2/15/23 3:05 PM, dai.ngo@xxxxxxxxxx wrote:
Hi Jeff, Chuck,
On 2/3/23 10:18 AM, Jeff Layton wrote:
The nested if statements here make no sense, as you can never reach
"else" branch in the nested statement. Fix the error handling for
when there is a courtesy client that holds a conflicting deny mode.
Fixes: 3d69427151806 (NFSD: add support for share reservation
conflict to courteous server)
Reported-by: 張智諺 <cc85nod@xxxxxxxxx>
Cc: Dai Ngo <dai.ngo@xxxxxxxxxx>
Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
---
fs/nfsd/nfs4state.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c39e43742dd6..af22dfdc6fcc 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5282,16 +5282,17 @@ nfs4_upgrade_open(struct svc_rqst *rqstp,
struct nfs4_file *fp,
/* test and set deny mode */
spin_lock(&fp->fi_lock);
status = nfs4_file_check_deny(fp, open->op_share_deny);
- if (status == nfs_ok) {
- if (status != nfserr_share_denied) {
- set_deny(open->op_share_deny, stp);
- fp->fi_share_deny |=
- (open->op_share_deny & NFS4_SHARE_DENY_BOTH);
- } else {
- if (nfs4_resolve_deny_conflicts_locked(fp, false,
- stp, open->op_share_deny, false))
- status = nfserr_jukebox;
- }
+ switch (status) {
+ case nfs_ok:
+ set_deny(open->op_share_deny, stp);
+ fp->fi_share_deny |=
+ (open->op_share_deny & NFS4_SHARE_DENY_BOTH);
+ break;
+ case nfserr_share_denied:
+ if (nfs4_resolve_deny_conflicts_locked(fp, false,
+ stp, open->op_share_deny, false))
While trying to write a pynfs test case to exercise this code path,
I realize that we don't need to call nfs4_resolve_deny_conflicts_locked
here since this is an open upgrade so it must comes from the same client
hence there is no conflict to resolve. Same behavior as OPEN_DOWNGRADE.
never mind, I found the scenario where this code path is executed:
Client1 opens fileX with WRITE, DENY_NONE
Client2 opens fileX with READ, DENY_NONE
Client2 opens fileX with READ, DENY_WRITE
-Dai
-Dai
+ status = nfserr_jukebox;
+ break;
}
spin_unlock(&fp->fi_lock);
