Skip to contentSkip to conversation

pynfs: cleanup and turn off some tests by default

PATCH New J. Bruce Fields 2 participants 9 messages Feb 15, 2012 – Feb 16, 2012

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

I've committed the following to my pynfs repo.

Fix a couple minor problems found while working on a bug, then turn off utf8 and pnfs tests by default (both can still be requested by adding "utf8" or "pnfs", respectively, on the commandline), and remove one incorrect test.

I'd like to get to the point where the pynfs tests all pass against a reasonable server. That means fixing the server where necessary, of course, but also not complaining about unimplemented optional features or about stuff we don't plan to fix.

--b.

#1 lore raw

PATCH 1/5 4.0 tests: kill "Starting Call Back server stub" printk

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

The printk doesn't seem especially useful, and it's just clutter if you want to use output of the scripts somehow.

---
 nfs4.0/nfs4lib.py |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
nfs4.0/nfs4lib.py +0 -1
index 0a38ebc..c6d6781 100644
@@ -191,7 +191,6 @@ class CBServer(rpc.RPCServer):
             raise "Bad name %s" % name
 
     def run(self):
-        print "Starting Call Back server stub on port %i" % self.port
         rpc.RPCServer.run(self)
 
     def handle_0(self, data, cred):
Signature · 1 line
1.7.5.4
#2 lore raw

PATCH 2/5 4.0 tests: remove 1-second wait on client creation

In reply to J. Bruce Fields

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

I can't figure out why this wait is necessary, and it slows things down. (Noticed while trying to write a race-condition reproducer requiring more precise timing.)

---
 nfs4.0/nfs4lib.py |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
nfs4.0/nfs4lib.py +0 -1
index c6d6781..7185057 100644
@@ -293,7 +293,6 @@ class NFS4Client(rpc.RPCClient, nfs4_ops.NFS4Operations):
         self.cb_control = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
         while 1:
             try:
-                time.sleep(1)
                 self.cb_control.connect(('127.0.0.1', self.cb_server.port))
                 break
             except socket.error:
Signature · 1 line
1.7.5.4
#3 lore raw

PATCH 3/5 4.1 server tests: don't run pnfs tests by default

In reply to J. Bruce Fields

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

I'd rather the pynfs tests have clean results by default against a correct 4.1 server. And it's correct for such a server not to implement optional features. So leave pnfs tests off by default; you can still get them by asking for "pnfs" tests.

Also, flags for individual pnfs operations seemed mild overkill for now, given how few tests we have.

---
 nfs4.1/server41tests/st_current_stateid.py |    2 +-
 nfs4.1/server41tests/st_getdevicelist.py   |   16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)
nfs4.1/server41tests/st_current_stateid.py +1 -1
index aeaa8b1..7804900 100644
@@ -119,7 +119,7 @@ def testOpenLayoutGet(t, env):
     """test current state id processing by having OPEN and LAYOUTGET
        in a single compound
 
-    FLAGS: currentstateid all
+    FLAGS: currentstateid pnfs
     CODE: CSID7
     """
     sess = env.c1.new_client_session(env.testname(t),
nfs4.1/server41tests/st_getdevicelist.py +8 -8
index 8d53633..7a10c6c 100644
@@ -10,7 +10,7 @@ from nfs4lib import state00
 def testGetDevList(t, env):
     """Check devlist
 
-    FLAGS: getdevicelist all
+    FLAGS: pnfs
     CODE: GETDLIST1
     """
     # Make sure E_ID returns MDS capabilities
@@ -36,7 +36,7 @@ def testGetDevList(t, env):
 def testGetDevInfo(t, env):
     """Check devlist
 
-    FLAGS: getdevicelist all
+    FLAGS: pnfs
     DEPEND: GETDLIST1
     CODE: GETDINFO1
     """
@@ -73,7 +73,7 @@ def testGetDevInfo(t, env):
 ## def xxxtestLayout(t, env):
 ##     """Verify layout handling
 
-##     FLAGS: layout all
+##     FLAGS: pnfs
 ##     CODE: GETLAYOUT1
 ##     """
 ##     # Make sure E_ID returns MDS capabilities
@@ -105,7 +105,7 @@ def testGetDevInfo(t, env):
 def testGetLayout(t, env):
     """Verify layout handling
 
-    FLAGS: getlayout all
+    FLAGS: pnfs
     CODE: GETLAYOUT1
     """
     # Make sure E_ID returns MDS capabilities
@@ -187,7 +187,7 @@ def testLayoutReturnFile(t, env):
     """
     Return a file's layout
     
-    FLAGS: layoutreturn all
+    FLAGS: pnfs
     DEPEND: GETLAYOUT1
     CODE: LAYOUTRET1
     """
@@ -220,7 +220,7 @@ def testLayoutReturnFsid(t, env):
     """
     Return all of a filesystem's layouts
     
-    FLAGS: layoutreturn all
+    FLAGS: pnfs
     DEPEND: GETLAYOUT1
     CODE: LAYOUTRET2
     """
@@ -252,7 +252,7 @@ def testLayoutReturnAll(t, env):
     """
     Return all of a client's layouts
     
-    FLAGS: layoutreturn all
+    FLAGS: pnfs
     DEPEND: GETLAYOUT1
     CODE: LAYOUTRET3
     """
@@ -283,7 +283,7 @@ def testLayoutCommit(t, env):
     """
     Do some commits
 
-    FLAGS: layoutcommit all
+    FLAGS: pnfs
     CODE: LAYOUTCOMMIT1
     """
     # Make sure E_ID returns MDS capabilities
Signature · 1 line
1.7.5.4
#4 lore raw

PATCH 4/5 4.1 server tests: don't run utf8 tests by default

In reply to J. Bruce Fields

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

The server doesn't pass the utf8 tests and probably never will, and I'm tired of people reporting those failures to me.

It's unclear to me whether those tests are even completely correct given the latest internationalization language in the spec.

---
 nfs4.0/servertests/st_compound.py   |    2 +-
 nfs4.0/servertests/st_link.py       |    2 +-
 nfs4.0/servertests/st_lookup.py     |    2 +-
 nfs4.0/servertests/st_open.py       |    2 +-
 nfs4.0/servertests/st_remove.py     |    2 +-
 nfs4.0/servertests/st_rename.py     |    4 ++--
 nfs4.0/servertests/st_secinfo.py    |    2 +-
 nfs4.0/servertests/st_setattr.py    |    2 +-
 nfs4.1/server41tests/st_compound.py |    2 +-
 nfs4.1/server41tests/st_rename.py   |    4 ++--
 10 files changed, 12 insertions(+), 12 deletions(-)
nfs4.0/servertests/st_compound.py +1 -1
index 98be3a8..d2af73b 100644
@@ -31,7 +31,7 @@ def testGoodTag(t, env):
 def testBadTags(t, env):
     """COMPOUND with invalid utf8 tags
 
-    FLAGS: compound utf8 all
+    FLAGS: compound utf8
     CODE: COMP3
     """
     c = env.c1
nfs4.0/servertests/st_link.py +1 -1
index 10d7197..e20ea77 100644
@@ -212,7 +212,7 @@ def testLongName(t, env):
 def testInvalidUtf8(t, env):
     """LINK with bad UTF-8 name strings should return NFS4ERR_INVAL
 
-    FLAGS: link utf8 all
+    FLAGS: link utf8
     DEPEND: LINKS LOOKFILE MKDIR
     CODE: LINK8
     """
nfs4.0/servertests/st_lookup.py +1 -1
index 40c5983..456fed2 100644
@@ -221,7 +221,7 @@ def testNonAccessable(t, env):
 def testInvalidUtf8(t, env):
     """LOOKUP with bad UTF-8 name strings should return NFS4ERR_INVAL
 
-    FLAGS: lookup utf8 all
+    FLAGS: lookup utf8
     DEPEND:
     CODE: LOOK7
     """
nfs4.0/servertests/st_open.py +1 -1
index 8176ad5..992e5e3 100644
@@ -247,7 +247,7 @@ def testNotDir(t, env):
 def testInvalidUtf8(t, env):
     """OPEN with bad UTF-8 name strings should return NFS4ERR_INVAL
 
-    FLAGS: open utf8 all
+    FLAGS: open utf8
     DEPEND: MKDIR
     CODE: OPEN13
     """
nfs4.0/servertests/st_remove.py +1 -1
index 76a7037..d475185 100644
@@ -195,7 +195,7 @@ def testZeroLengthTarget(t, env):
 def testNonUTF8(t, env):
     """REMOVE with non-UTF8 components should return NFS4ERR_INVAL
 
-    FLAGS: remove utf8 all
+    FLAGS: remove utf8
     DEPEND: MKDIR
     CODE: RM5
     """
nfs4.0/servertests/st_rename.py +2 -2
index fe6339b..16e4ce5 100644
@@ -311,7 +311,7 @@ def testZeroLengthNewname(t, env):
 def testBadutf8Oldname(t, env):
     """RENAME with non-UTF8 oldname should return NFS4ERR_INVAL
 
-    FLAGS: rename utf8 all
+    FLAGS: rename utf8
     DEPEND: MKDIR
     CODE: RNM8
     """
@@ -327,7 +327,7 @@ def testBadutf8Oldname(t, env):
 def testBadutf8Newname(t, env):
     """RENAME with non-UTF8 newname should return NFS4ERR_INVAL
 
-    FLAGS: rename utf8 all
+    FLAGS: rename utf8
     DEPEND: MKDIR
     CODE: RNM9
     """
nfs4.0/servertests/st_secinfo.py +1 -1
index 793fdfd..83f713b 100644
@@ -80,7 +80,7 @@ def testZeroLenName(t, env):
 def testInvalidUtf8(t, env):
     """SECINFO with bad UTF-8 name strings should return NFS4ERR_INVAL
 
-    FLAGS: secinfo utf8 all
+    FLAGS: secinfo utf8
     DEPEND: SEC1
     CODE: SEC6
     """
nfs4.0/servertests/st_setattr.py +1 -1
index 686ce87..936778f 100644
@@ -412,7 +412,7 @@ def testNonUTF8(t, env):
     OWNER and GROUP are subject to too many restrictions to use.
     Similarly for ACL.
 
-    FLAGS: setattr utf8 all
+    FLAGS: setattr utf8
     DEPEND: MKFILE
     CODE: SATT9
     """
nfs4.1/server41tests/st_compound.py +1 -1
index 4dedca8..d38f621 100644
@@ -39,7 +39,7 @@ def testGoodTag(t, env):
 def testBadTags(t, env):
     """COMPOUND with invalid utf8 tags
 
-    FLAGS: compound utf8 all
+    FLAGS: compound utf8
     CODE: COMP3
     """
     c = env.c1
nfs4.1/server41tests/st_rename.py +2 -2
index bddcb35..ed5dbb5 100644
@@ -314,7 +314,7 @@ def testZeroLengthNewname(t, env):
 def testBadutf8Oldname(t, env):
     """RENAME with non-UTF8 oldname should return NFS4ERR_INVAL
 
-    FLAGS: rename utf8 all
+    FLAGS: rename utf8
     CODE: RNM8
     """
     name = env.testname(t)
@@ -330,7 +330,7 @@ def testBadutf8Oldname(t, env):
 def testBadutf8Newname(t, env):
     """RENAME with non-UTF8 newname should return NFS4ERR_INVAL
 
-    FLAGS: rename utf8 all
+    FLAGS: rename utf8
     CODE: RNM9
     """
     name = env.testname(t)
Signature · 1 line
1.7.5.4
#5 lore raw

PATCH 5/5 4.0 server tests: remove RD12 (incorrect test)

In reply to J. Bruce Fields

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

Quoted 1 line

From 3530 section 8.1.4:

In the case of WRITE-type operations (i.e., WRITEs and SETATTRs which set size), the server must verify that the access mode allows writing and return an NFS4ERR_OPENMODE error if it does not. In the case, of READ, the server may perform the corresponding check on the access mode, or it may choose to allow READ on opens for WRITE only, to accommodate clients whose write implementation may unavoidably do reads (e.g., due to buffer cache constraints).

---
 nfs4.0/servertests/st_read.py |   14 --------------
 1 files changed, 0 insertions(+), 14 deletions(-)
nfs4.0/servertests/st_read.py +0 -14
index 639b436..cce9a03 100644
@@ -219,17 +219,3 @@ def testOldStateid(t, env):
     fh, stateid = c.confirm(t.code, res)
     res = c.read_file(fh, stateid=oldstateid)
     check(res, NFS4ERR_OLD_STATEID, "READ with old stateid")
-
-def testOpenMode(t, env):
-    """READ with file opened in WRITE mode should return NFS4_OK or NFS4ERR_OPENMODE
-
-    FLAGS: read all
-    DEPEND: MKFILE
-    CODE: RD12
-    """
-    c = env.c1
-    c.init_connection()
-    fh, stateid = c.create_confirm(t.code, access=OPEN4_SHARE_ACCESS_WRITE)
-    res = c.read_file(fh, stateid=stateid)
-    check(res, NFS4ERR_OPENMODE, "READ with file opened in WRITE mode",
-          [NFS4_OK])
Signature · 1 line
1.7.5.4
#6 lore raw
In reply to J. Bruce Fields
Quoted 7 lines · J. Bruce Fields

I've committed the following to my pynfs repo.

Fix a couple minor problems found while working on a bug, then turn off utf8 and pnfs tests by default (both can still be requested by adding "utf8" or "pnfs", respectively, on the commandline), and remove one incorrect test.

Quoted 5 lines

I'd like to get to the point where the pynfs tests all pass against a reasonable server. That means fixing the server where necessary, of course, but also not complaining about unimplemented optional features or about stuff we don't plan to fix.

Can't we "auto detect" the pnfs option. Request a pnfs access if failed proceed to the next set of tests, if successful continue to test "pnfs"

Quoted 2 lines

--b.

Thanks
Boaz

#7 lore raw
Quoted 8 lines · Boaz Harrosh
Quoted 7 lines · J. Bruce Fields

I've committed the following to my pynfs repo.

Fix a couple minor problems found while working on a bug, then turn off utf8 and pnfs tests by default (both can still be requested by adding "utf8" or "pnfs", respectively, on the commandline), and remove one incorrect test.

Quoted 8 lines
Quoted 5 lines

I'd like to get to the point where the pynfs tests all pass against a reasonable server. That means fixing the server where necessary, of course, but also not complaining about unimplemented optional features or about stuff we don't plan to fix.

Can't we "auto detect" the pnfs option. Request a pnfs access if failed proceed to the next set of tests, if successful continue to test "pnfs"

Good idea! I think you'd still want "pnfs" and "nopnfs" options: "pnfs" for "I expect this server to support pnfs, and if it doesn't report that as a bug!", and "nopnfs" for: "this server claims to support pnfs but please just don't even bother telling me how buggy it is, I already know".

But just turning it off by default was a minute's worth of search-and-replace for me, and I don't have the time for more right now. So that's a project looking for a volunteer.

--b.

#8 lore raw
Quoted 15 lines · J. Bruce Fields
Quoted 14 lines · Boaz Harrosh
Quoted 12 lines · J. Bruce Fields

I've committed the following to my pynfs repo.

Fix a couple minor problems found while working on a bug, then turn off utf8 and pnfs tests by default (both can still be requested by adding "utf8" or "pnfs", respectively, on the commandline), and remove one incorrect test.

I'd like to get to the point where the pynfs tests all pass against a reasonable server. That means fixing the server where necessary, of course, but also not complaining about unimplemented optional features or about stuff we don't plan to fix.

Quoted 9 lines
Quoted 2 lines

Can't we "auto detect" the pnfs option. Request a pnfs access if failed proceed to the next set of tests, if successful continue to test "pnfs"

Good idea! I think you'd still want "pnfs" and "nopnfs" options: "pnfs" for "I expect this server to support pnfs, and if it doesn't report that as a bug!", and "nopnfs" for: "this server claims to support pnfs but please just don't even bother telling me how buggy it is, I already know".

Sounds good

Quoted 4 lines

But just turning it off by default was a minute's worth of search-and-replace for me, and I don't have the time for more right now. So that's a project looking for a volunteer.

NP, thanks for the fixes so far. We'll probably add pynfs testing to our Ganesha testing rig. When we do we'll look into it. That is if no one bits us to it, (Hopefully ;-) )

Quoted 1 line

--b.

Thanks
Boaz