Re: rename() over NFS

[Bruno Haible]

Jim Meyering wrote:
> if you see ways to improve things without impacting performance
> or maintainability, I'm all for it.

Maintainability wouldn't be impacted, because the fix would be to use
lib/rename.c, with
  #define RENAME_DEST_EXISTS_BUG 1
  #define RENAME_HARD_LINK_BUG 1

But performance would be impacted, since 2 lstat() calls would happen
before every rename().

The bug may not have a big impact, since coreutils 'mv' handles a move of a
directory to a directory differently anyway, and few other programs use
rename() on directories.

So, assuming Eric also agrees with approach (a), here's a proposal for
fixing the testsuite:


2010-11-13  Bruno Haible  <address@hidden>

        rename, renameat: Avoid test failures at NFS mounted locations.
        * tests/test-rename.h (dentry_exists, assert_nonexistent): New
        functions.
        (test_rename): Use assert_nonexistent.
        * tests/test-rename.c: Include <dirent.h>.
        * tests/test-renameat.c: Likewise.
        Reported by Gary V. Vaughan <address@hidden>.

--- tests/test-rename.h.orig    Sat Nov 13 14:35:50 2010
+++ tests/test-rename.h Sat Nov 13 14:35:23 2010
@@ -20,6 +20,49 @@
    appropriate headers are already included.  If PRINT, warn before
    skipping symlink tests with status 77.  */
 
+/* Tests whether a file, given by a file name without slashes, exists in
+   the current directory, by scanning the directory entries.  */
+static bool
+dentry_exists (const char *filename)
+{
+  bool exists = false;
+  DIR *dir = opendir (".");
+
+  ASSERT (dir != NULL);
+  for (;;)
+    {
+      struct dirent *d = readdir (dir);
+      if (d == NULL)
+        break;
+      if (strcmp (d->d_name, filename) == 0)
+        {
+          exists = true;
+          break;
+        }
+    }
+  ASSERT (closedir (dir) == 0);
+  return exists;
+}
+
+/* Asserts that a specific file, given by a file name without slashes, does
+   not exist in the current directory.  */
+static void
+assert_nonexistent (const char *filename)
+{
+  struct stat st;
+
+  /* The usual way to test the presence of a file is via stat() or lstat().  */
+  errno = 0;
+  if (stat (filename, &st) == -1)
+    ASSERT (errno == ENOENT);
+  else
+    /* But after renaming a directory over an empty directory on an NFS-mounted
+       file system, on Linux 2.6.18, for a period of 30 seconds the old
+       directory name is "present" according to stat() but "nonexistent"
+       according to dentry_exists().  */
+    ASSERT (!dentry_exists (filename));
+}
+
 static int
 test_rename (int (*func) (char const *, char const *), bool print)
 {
@@ -226,9 +269,7 @@
     }
     { /* Full onto empty.  */
       ASSERT (func (BASE "dir", BASE "dir2") == 0);
-      errno = 0;
-      ASSERT (stat (BASE "dir", &st) == -1);
-      ASSERT (errno == ENOENT);
+      assert_nonexistent (BASE "dir");
       ASSERT (stat (BASE "dir2/file", &st) == 0);
       /* Files present here:
            {BASE}file
@@ -263,9 +304,7 @@
        */
       {
         ASSERT (func (BASE "dir", BASE "dir2/") == 0);
-        errno = 0;
-        ASSERT (stat (BASE "dir", &st) == -1);
-        ASSERT (errno == ENOENT);
+        assert_nonexistent (BASE "dir");
         ASSERT (stat (BASE "dir2/file", &st) == 0);
       }
       /* Files present here:
--- tests/test-rename.c.orig    Sat Nov 13 14:35:50 2010
+++ tests/test-rename.c Sat Nov 13 14:15:34 2010
@@ -21,6 +21,7 @@
 #include "signature.h"
 SIGNATURE_CHECK (rename, int, (char const *, char const *));
 
+#include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <stdbool.h>
--- tests/test-renameat.c.orig  Sat Nov 13 14:35:50 2010
+++ tests/test-renameat.c       Sat Nov 13 14:15:30 2010
@@ -23,6 +23,7 @@
 #include "signature.h"
 SIGNATURE_CHECK (renameat, int, (int, char const *, int, char const *));
 
+#include <dirent.h>
 #include <fcntl.h>
 #include <errno.h>
 #include <stdbool.h>