Skip to contentSkip to conversation

pynfs fixes from cthon

PATCH New J. Bruce Fields 1 participant 6 messages Feb 24, 2012

Subsystems (2)
NFS client NFS server
lore
Share & tools
lore

J. Bruce Fields

author
#0 lore raw

This is a few fixes found while trying pynfs tests against servers at connectathon.

--b.

author
#1 lore raw

PATCH 1/5 4.1 server tests: rename tests incorrectly using root dir

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

Several of the rename tests were incorrectly creating objects in the export root.

Signed-off-by J. Bruce Fields <[email protected]>
---
 nfs4.1/server41tests/st_rename.py |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)
nfs4.1/server41tests/st_rename.py +6 -6
index ed5dbb5..f706a1c 100644
@@ -185,7 +185,7 @@ def testCfhFile(t, env):
     """
     name = env.testname(t)
     sess = env.c1.new_client_session(name)
-    res = create_obj(sess, [name])
+    res = create_obj(sess, env.c1.homedir + [name])
     check(res)
     res = rename_obj(sess, env.c1.homedir + [name], env.opts.usefile + [name])
     check(res, NFS4ERR_NOTDIR, "RENAME with non-dir <cfh>")

@@ -198,7 +198,7 @@ def testCfhLink(t, env):
     """
     name = env.testname(t)
     sess = env.c1.new_client_session(name)
-    res = create_obj(sess, [name])
+    res = create_obj(sess, env.c1.homedir + [name])
     check(res)
     res = rename_obj(sess, env.c1.homedir + [name], env.opts.uselink + [name])
     check(res, NFS4ERR_NOTDIR, "RENAME with non-dir <cfh>")

@@ -211,7 +211,7 @@ def testCfhBlock(t, env):
     """
     name = env.testname(t)
     sess = env.c1.new_client_session(name)
-    res = create_obj(sess, [name])
+    res = create_obj(sess, env.c1.homedir + [name])
     check(res)
     res = rename_obj(sess, env.c1.homedir + [name], env.opts.useblock + [name])
     check(res, NFS4ERR_NOTDIR, "RENAME with non-dir <cfh>")

@@ -224,7 +224,7 @@ def testCfhChar(t, env):
     """
     name = env.testname(t)
     sess = env.c1.new_client_session(name)
-    res = create_obj(sess, [name])
+    res = create_obj(sess, env.c1.homedir + [name])
     check(res)
     res = rename_obj(sess, env.c1.homedir + [name], env.opts.usechar + [name])
     check(res, NFS4ERR_NOTDIR, "RENAME with non-dir <cfh>")

@@ -237,7 +237,7 @@ def testCfhFifo(t, env):
     """
     name = env.testname(t)
     sess = env.c1.new_client_session(name)
-    res = create_obj(sess, [name])
+    res = create_obj(sess, env.c1.homedir + [name])
     check(res)
     res = rename_obj(sess, env.c1.homedir + [name], env.opts.usefifo + [name])
     check(res, NFS4ERR_NOTDIR, "RENAME with non-dir <cfh>")

@@ -250,7 +250,7 @@ def testCfhSocket(t, env):
     """
     name = env.testname(t)
     sess = env.c1.new_client_session(name)
-    res = create_obj(sess, [name])
+    res = create_obj(sess, env.c1.homedir + [name])
     check(res)
     res = rename_obj(sess, env.c1.homedir + [name], env.opts.usesocket + [name])
     check(res, NFS4ERR_NOTDIR, "RENAME with non-dir <cfh>")
Signature · 1 line
1.7.7.6
author
#2 lore raw

PATCH 2/5 4.1 server tests: maketree should not leave files open

In reply to J. Bruce Fields

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

In particular this causes spurious failures against any server that does not allow renames of open files.

There are probably many more places where we should really be doing closes.

Note: it might also be interesting to do a purely informational test to determine whether a server allows renames of open files.

Signed-off-by J. Bruce Fields <[email protected]>
---
 nfs4.1/server41tests/environment.py |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)
nfs4.1/server41tests/environment.py +13 -1
index e9d2237..2566274 100644
@@ -550,6 +550,18 @@ def create_confirm(sess, owner, path=None, attrs={FATTR4_MODE: 0644},
     return fh, res.resarray[-2].stateid
     return fh, openstateid
 
+def create_close(sess, owner, path=None, attrs={FATTR4_MODE: 0644},
+                   access=OPEN4_SHARE_ACCESS_BOTH,
+                   deny=OPEN4_SHARE_DENY_NONE,
+                   mode=GUARDED4):
+    """Create (using open) a regular file, confirm the open, and close
+
+    Returns the fhandle
+    """
+    fh, stateid = create_confirm(sess, owner, path, attrs, access, deny, mode)
+    close_file(sess, fh, stateid=stateid)
+    return fh;
+
 def _getname(owner, path):
     if path is None:
         return owner

@@ -580,7 +592,7 @@ def maketree(sess, tree, root=None, owner=None):
             check(res)
             maketree(sess, obj[1:], root + [obj[0]], owner)
         else:
-            create_confirm(sess, owner, root + [obj])
+            create_close(sess, owner, root + [obj])
 
 def lookup_obj(sess, path):
     compound = [op.putrootfh()]
Signature · 1 line
1.7.7.6
author
#3 lore raw

PATCH 3/5 4.1 server tests: add citation for COMP4b

In reply to J. Bruce Fields

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

Signed-off-by J. Bruce Fields <[email protected]>
---
 nfs4.1/server41tests/st_compound.py |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
nfs4.1/server41tests/st_compound.py +1 -1
index d38f621..c6a5ea9 100644
@@ -64,7 +64,7 @@ def testInvalidMinor(t, env):
 def testInvalidMinor2(t, env):
     """COMPOUND with invalid minor version returns NFS4ERR_MINOR_VERS_MISMATCH
 
-    even if using illegal opcode
+    even if using illegal opcode (rfc 5661 p. 399)
 
     FLAGS: compound all
     CODE: COMP4b
Signature · 1 line
1.7.7.6
author
#4 lore raw

PATCH 4/5 4.1 server tests: allow SYMLINK error on rename

In reply to J. Bruce Fields

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

Yes, 5661 does say to return NOTDIR in this case. But I can't figure out why. Elsewhere it's generally preferred to return the more specific SYMLINK error. This is a buggy-client case (since the source and target dirs are referred to by filehandle, the client should generally know whether it's dealing with a directory or not), so I don't see the point of being really picky here.

Signed-off-by J. Bruce Fields <[email protected]>
---
 nfs4.1/server41tests/st_rename.py |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
nfs4.1/server41tests/st_rename.py +2 -1
index f706a1c..cde8ffc 100644
@@ -201,7 +201,8 @@ def testCfhLink(t, env):
     res = create_obj(sess, env.c1.homedir + [name])
     check(res)
     res = rename_obj(sess, env.c1.homedir + [name], env.opts.uselink + [name])
-    check(res, NFS4ERR_NOTDIR, "RENAME with non-dir <cfh>")
+    checklist(res, [NFS4ERR_NOTDIR, NFS4ERR_SYMLINK],
+                                "RENAME with non-dir <cfh>")
 
 def testCfhBlock(t, env):
     """RENAME with non-dir (cfh) should return NFS4ERR_NOTDIR
Signature · 1 line
1.7.7.6
author
#5 lore raw

PATCH 5/5 4.1 server tests: don't insist on 0-seqid trick in setup

In reply to J. Bruce Fields

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

The 4.1 spec requires servers to handle stateid's with seqid field 0 (they have the special meaning of "use the latest stateid in this stateid stream").

We've seen buggy or incomplete servers that don't handle this yet.

We *should* add a test for that. But insisting on it (for no real reason) during setup just prevents all the other tests from running unnecessarily.

Signed-off-by J. Bruce Fields <[email protected]>
---
 nfs4.1/server41tests/environment.py |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
nfs4.1/server41tests/environment.py +1 -1
index 2566274..24e82cd 100644
@@ -190,7 +190,7 @@ class Environment(testmod.Environment):
                 log.warning("could not create /%s" % '/'.join(path))
         # Make file-object in /tree
         fh, stateid = create_confirm(sess, 'maketree', tree + ['file'])
-        stateid.seqid = 0
+        # stateid.seqid = 0
         ops = [op.putfh(fh),
                op.write(stateid, 0, FILE_SYNC4, self.filedata)]
         res = sess.compound(ops)
Signature · 1 line
1.7.7.6