[PATCH v2] Test renaming onto an open file

[PATCH v2] Test renaming onto an open file

[Chuck Lever]

There has been a recent report of RENAME onto an open file resulting
in a bogus ESTALE when that open (and now deleted) file is closed.
Build in some testing with and without SEQUENCE to ensure a server
is handling this case as expected.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

Changes since v1:
- None. This is a just a repost.

---
 nfs4.0/servertests/st_rename.py   |   16 ++++++++++++++++
 nfs4.1/server41tests/st_rename.py |   19 ++++++++++++++++++-
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/nfs4.0/servertests/st_rename.py b/nfs4.0/servertests/st_rename.py
index afe9a7fe0a58..3302608719e3 100644
--- a/nfs4.0/servertests/st_rename.py
+++ b/nfs4.0/servertests/st_rename.py
@@ -524,6 +524,22 @@ def testLinkRename(t, env):
         t.fail("RENAME of file into its hard link should do nothing, "
                "but cinfo was changed")
 
+def testStaleRename(t, env):
+    """RENAME file over an open file should allow CLOSE
+
+    FLAGS: rename all
+    CODE: RNM21
+    """
+    c = env.c1
+    c.init_connection()
+    basedir = c.homedir + [t.word()]
+    c.maketree([t.word(), b'file'])
+    fh, stateid = c.create_confirm(t.word(), path=basedir + [b'file2'])
+    res = c.rename_obj(basedir + [b'file'], basedir + [b'file2'])
+    check(res)
+    res = c.close_file(t.word(), fh, stateid)
+    check(res, msg="CLOSE after RENAME deletes target returns ESTALE")
+
 ###########################################
 
     def testNamingPolicy(t, env):
diff --git a/nfs4.1/server41tests/st_rename.py b/nfs4.1/server41tests/st_rename.py
index 066df749379f..26ed4d275067 100644
--- a/nfs4.1/server41tests/st_rename.py
+++ b/nfs4.1/server41tests/st_rename.py
@@ -1,5 +1,5 @@
 from xdrdef.nfs4_const import *
-from .environment import check, fail, maketree, rename_obj, get_invalid_utf8strings, create_obj, create_confirm, link, use_obj, create_file
+from .environment import check, fail, maketree, rename_obj, get_invalid_utf8strings, create_obj, create_confirm, link, use_obj, create_file, close_file
 import nfs_ops
 op = nfs_ops.NFS4ops()
 from xdrdef.nfs4_type import *
@@ -529,3 +529,20 @@ def testLinkRename(t, env):
     if scinfo.before != scinfo.after or tcinfo.before != tcinfo.after:
         t.fail("RENAME of file into its hard link should do nothing, "
                "but cinfo was changed")
+
+def testStaleRename(t, env):
+    """RENAME file over an open file should allow CLOSE
+
+    FLAGS: rename all
+    CODE: RNM21
+    """
+    name = env.testname(t)
+    owner = b"owner_%s" % name
+    sess = env.c1.new_client_session(name)
+    maketree(sess, [name, b'file'])
+    basedir = env.c1.homedir + [name]
+    fh, stateid = create_confirm(sess, owner, path=basedir + [b'file2'])
+    res = rename_obj(sess, basedir + [b'file'], basedir + [b'file2'])
+    check(res)
+    res = close_file(sess, fh, stateid)
+    check(res, msg="CLOSE after RENAME deletes target returns ESTALE")

Re: [PATCH v2] Test renaming onto an open file

[J. Bruce Fields]

Applied, thanks.--b.

On Fri, May 27, 2022 at 03:44:55PM -0400, Chuck Lever wrote:
> There has been a recent report of RENAME onto an open file resulting
> in a bogus ESTALE when that open (and now deleted) file is closed.
> Build in some testing with and without SEQUENCE to ensure a server
> is handling this case as expected.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> 
> Changes since v1:
> - None. This is a just a repost.
> 
> ---
>  nfs4.0/servertests/st_rename.py   |   16 ++++++++++++++++
>  nfs4.1/server41tests/st_rename.py |   19 ++++++++++++++++++-
>  2 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/nfs4.0/servertests/st_rename.py b/nfs4.0/servertests/st_rename.py
> index afe9a7fe0a58..3302608719e3 100644
> --- a/nfs4.0/servertests/st_rename.py
> +++ b/nfs4.0/servertests/st_rename.py
> @@ -524,6 +524,22 @@ def testLinkRename(t, env):
>          t.fail("RENAME of file into its hard link should do nothing, "
>                 "but cinfo was changed")
>  
> +def testStaleRename(t, env):
> +    """RENAME file over an open file should allow CLOSE
> +
> +    FLAGS: rename all
> +    CODE: RNM21
> +    """
> +    c = env.c1
> +    c.init_connection()
> +    basedir = c.homedir + [t.word()]
> +    c.maketree([t.word(), b'file'])
> +    fh, stateid = c.create_confirm(t.word(), path=basedir + [b'file2'])
> +    res = c.rename_obj(basedir + [b'file'], basedir + [b'file2'])
> +    check(res)
> +    res = c.close_file(t.word(), fh, stateid)
> +    check(res, msg="CLOSE after RENAME deletes target returns ESTALE")
> +
>  ###########################################
>  
>      def testNamingPolicy(t, env):
> diff --git a/nfs4.1/server41tests/st_rename.py b/nfs4.1/server41tests/st_rename.py
> index 066df749379f..26ed4d275067 100644
> --- a/nfs4.1/server41tests/st_rename.py
> +++ b/nfs4.1/server41tests/st_rename.py
> @@ -1,5 +1,5 @@
>  from xdrdef.nfs4_const import *
> -from .environment import check, fail, maketree, rename_obj, get_invalid_utf8strings, create_obj, create_confirm, link, use_obj, create_file
> +from .environment import check, fail, maketree, rename_obj, get_invalid_utf8strings, create_obj, create_confirm, link, use_obj, create_file, close_file
>  import nfs_ops
>  op = nfs_ops.NFS4ops()
>  from xdrdef.nfs4_type import *
> @@ -529,3 +529,20 @@ def testLinkRename(t, env):
>      if scinfo.before != scinfo.after or tcinfo.before != tcinfo.after:
>          t.fail("RENAME of file into its hard link should do nothing, "
>                 "but cinfo was changed")
> +
> +def testStaleRename(t, env):
> +    """RENAME file over an open file should allow CLOSE
> +
> +    FLAGS: rename all
> +    CODE: RNM21
> +    """
> +    name = env.testname(t)
> +    owner = b"owner_%s" % name
> +    sess = env.c1.new_client_session(name)
> +    maketree(sess, [name, b'file'])
> +    basedir = env.c1.homedir + [name]
> +    fh, stateid = create_confirm(sess, owner, path=basedir + [b'file2'])
> +    res = rename_obj(sess, basedir + [b'file'], basedir + [b'file2'])
> +    check(res)
> +    res = close_file(sess, fh, stateid)
> +    check(res, msg="CLOSE after RENAME deletes target returns ESTALE")
>