Skip to contentSkip to conversation

TESTS: Allow server to reject commits with large offsets

PATCH In review J. Bruce Fields 1 participant 3 messages Aug 17, 2010

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/2 TESTS: Allow server to reject commits with large offsets

From: Muneet Parhar <[email protected]>

These two tests insist that a server must accept commits with offset 2^64-1 and 2^64-2. I can find no justification in the spec for this requirement.

The linux server was recently changed to reject (with INVAL) offsets over 2^63-1, which is the maximum that the vfs commit routine can accept. That behavior is consistent with the NFSv3 commit implementation and with the NFSv4 write implementation.

The maximum offset allowed may vary depending on filesystem or server, so we should allow either success or INVAL here.

---
 lib/nfs4/servertests/st_commit.py |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)
lib/nfs4/servertests/st_commit.py +5 -6
index 2955d79..b7f536e 100644
@@ -1,9 +1,9 @@
 from nfs4.nfs4_const import *
-from environment import check
+from environment import check, checklist
 
 _text = "Random data to write"
 
-def _commit(t, c, offset=0, count=0):
+def _commit(t, c, offset=0, count=0, statlist=[NFS4_OK]):
     """COMMIT
 
     FLAGS: commit all
@@ -15,7 +15,7 @@ def _commit(t, c, offset=0, count=0):
     res = c.write_file(fh, _text, 0, stateid, how=UNSTABLE4)
     check(res, msg="WRITE with how=UNSTABLE4")
     res = c.commit_file(fh, offset, count)
-    check(res, msg="COMMIT with offset=%x, count=%x" % (offset, count))
+    checklist(res, statlist, msg="COMMIT with offset=%x, count=%x" % (offset, count))
 
 def testCommitOffset0(t, env):
     """COMMIT
@@ -42,7 +42,7 @@ def testCommitOffsetMax1(t, env):
     DEPEND: MKFILE
     CODE: CMT1c
     """
-    _commit(t, env.c1, 0xffffffffffffffffL)
+    _commit(t, env.c1, 0xffffffffffffffffL, statlist=[NFS4_OK, NFS4ERR_INVAL])
 
 def testCommitOffsetMax2(t, env):
     """COMMIT
@@ -51,7 +51,7 @@ def testCommitOffsetMax2(t, env):
     DEPEND: MKFILE
     CODE: CMT1d
     """
-    _commit(t, env.c1, 0xfffffffffffffffeL)
+    _commit(t, env.c1, 0xfffffffffffffffeL, statlist=[NFS4_OK, NFS4ERR_INVAL])
 
 def testCommitCount1(t, env):
     """COMMIT
@@ -71,7 +71,6 @@ def testCommitCountMax(t, env):
     """
     _commit(t, env.c1, 0, 0xffffffffL)
 
-
 def testLink(t, env):
     """COMMIT
 
Signature · 1 line
1.7.0.4
#1 lore raw

PATCH 2/2 remove open share mode file permission check

From: J. Bruce Fields <[email protected]>

First, the ability to deny reads is generally considered stronger than the ability to deny writes, so if anything should require special permissions it is the former.

Second, this test contradicts observed Windows behavior. Testing shows that Windows prohits an opener without write permissions from denying reads. It will allow such an open, but will then also allow concurrent read opens, failing to enforce the DENY_WRITE.

---
 lib/nfs4/servertests/st_open.py |   21 ---------------------
 1 files changed, 0 insertions(+), 21 deletions(-)
lib/nfs4/servertests/st_open.py +0 -21
index 64deb9f..1e5ec70 100644
@@ -347,27 +347,6 @@ def testShareConflict1(t, env):
           "Trying to open a file with deny=WRITE "
           "that was already opened with access=WRITE")
 
-# FRED - is this right response? or should it be allowed?
-def testShareConflict2(t, env):
-    """OPEN with deny=write when don't have write permissions
-
-    FLAGS: open all
-    DEPEND: MKFILE MODE
-    CODE: OPEN19
-    """
-    c1 = env.c1
-    c1.init_connection()
-    c1.create_confirm(t.code, attrs={FATTR4_MODE:0644},
-                      access=OPEN4_SHARE_ACCESS_READ,
-                      deny=OPEN4_SHARE_DENY_NONE)
-    c2 = env.c2
-    c2.init_connection()
-    res = c2.open_file(t.code, access=OPEN4_SHARE_ACCESS_READ,
-                       deny=OPEN4_SHARE_DENY_WRITE)
-    check(res, NFS4ERR_ACCESS,
-          "Trying to deny write permissions to others when "
-          "don't have write permissions")
-
 def testFailedOpen(t, env):
     """MULTIPLE: failed open should not mess up other clients' filehandles
 
Signature · 1 line
1.7.0.4
#2 lore raw

Re: [PATCH 2/2] remove open share mode file permission check

Both of these are also available from

git://linux-nfs.org/~bfields/pynfs.git

--b.

Quoted 51 lines · J. Bruce Fields

From: J. Bruce Fields <[email protected]>

First, the ability to deny reads is generally considered stronger than the ability to deny writes, so if anything should require special permissions it is the former.

Second, this test contradicts observed Windows behavior. Testing shows that Windows prohits an opener without write permissions from denying reads. It will allow such an open, but will then also allow concurrent read opens, failing to enforce the DENY_WRITE.

---
 lib/nfs4/servertests/st_open.py |   21 ---------------------
 1 files changed, 0 insertions(+), 21 deletions(-)
lib/nfs4/servertests/st_open.py +0 -21
index 64deb9f..1e5ec70 100644
@@ -347,27 +347,6 @@ def testShareConflict1(t, env):
           "Trying to open a file with deny=WRITE "
           "that was already opened with access=WRITE")
 
-# FRED - is this right response? or should it be allowed?
-def testShareConflict2(t, env):
-    """OPEN with deny=write when don't have write permissions
-
-    FLAGS: open all
-    DEPEND: MKFILE MODE
-    CODE: OPEN19
-    """
-    c1 = env.c1
-    c1.init_connection()
-    c1.create_confirm(t.code, attrs={FATTR4_MODE:0644},
-                      access=OPEN4_SHARE_ACCESS_READ,
-                      deny=OPEN4_SHARE_DENY_NONE)
-    c2 = env.c2
-    c2.init_connection()
-    res = c2.open_file(t.code, access=OPEN4_SHARE_ACCESS_READ,
-                       deny=OPEN4_SHARE_DENY_WRITE)
-    check(res, NFS4ERR_ACCESS,
-          "Trying to deny write permissions to others when "
-          "don't have write permissions")
-
 def testFailedOpen(t, env):
     """MULTIPLE: failed open should not mess up other clients' filehandles
 
Signature · 1 line
1.7.0.4