tail: fix --follow to not use inotify on remote files

* src/tail.c (struct File_spec): Add a flag to record if file is remote.
(recheck): If we're using inotify then check if the file has gone remote
and if so, drop it with a warning.
(any_remote_files): A new function to check for any open remote files.
(tailable_stdin): A new function to refactor the check for whether
a tailable file was specified through stdin.
(fremote): A new function to check if a file descriptor
refers to a remote file.
(tail_forever_inotify): Add some comments.
(tail_file): Record if a file is remote when initially opened.
(main): Disable inotify if any remote files specified.
Also document the caveat about remounted files not
being noticed by inotify.
* NEWS: Mention the fix.

1 files changed, 116 insertions, 14 deletions

diff --git a/src/tail.c b/src/tail.c
index 71f8a32d1..025680445 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -52,6 +52,12 @@
 # include <sys/inotify.h>
 /* `select' is used by tail_forever_inotify.  */
 # include <sys/select.h>
+
+/* inotify needs to know if a file is local.  */
+# include "fs.h"
+# if HAVE_SYS_STATFS_H
+#  include <sys/statfs.h>
+# endif
 #endif
 
 /* The official name of this program (e.g., no `g' prefix).  */
@@ -143,6 +149,9 @@ struct File_spec
 
   /* Offset in NAME of the basename part.  */
   size_t basename_start;
+
+  /* inotify doesn't work for remotely updated files.  */
+  bool remote;
 #endif
 };
 
@@ -151,6 +160,8 @@ struct File_spec
    directories.  */
 const uint32_t inotify_wd_mask = (IN_MODIFY | IN_ATTRIB | IN_DELETE_SELF
                                   | IN_MOVE_SELF);
+
+static bool fremote (int fd, const char *name);
 #endif
 
 /* Keep trying to open a file even if it is inaccessible when tail starts
@@ -201,8 +212,7 @@ static bool have_read_stdin;
    more expensive) code unconditionally. Intended solely for testing.  */
 static bool presume_input_pipe;
 
-/* If nonzero then don't use inotify even if available.
-   Intended solely for testing.  */
+/* If nonzero then don't use inotify even if available.  */
 static bool disable_inotify;
 
 /* For long options that have no equivalent short option, use a
@@ -921,6 +931,17 @@ recheck (struct File_spec *f, bool blocking)
              quote (pretty_name (f)));
       f->ignore = true;
     }
+#if HAVE_INOTIFY
+  else if (!disable_inotify && fremote (fd, pretty_name (f)))
+    {
+      ok = false;
+      f->errnum = -1;
+      error (0, 0, _("%s has been replaced with a remote file. "
+                     "giving up on this name"), quote (pretty_name (f)));
+      f->ignore = true;
+      f->remote = true;
+    }
+#endif
   else
     {
       f->errnum = 0;
@@ -1153,6 +1174,74 @@ tail_forever (struct File_spec *f, size_t n_files, double sleep_interval)
 
 #if HAVE_INOTIFY
 
+/* Return true if any of the N_FILES files in F are remote, i.e., have
+   open file descriptors and are on network file systems.  */
+
+static bool
+any_remote_files (const struct File_spec *f, size_t n_files)
+{
+  size_t i;
+
+  for (i = 0; i < n_files; i++)
+    if (0 <= f[i].fd && f[i].remote)
+      return true;
+  return false;
+}
+
+/* Return true if any of the N_FILES files in F represent
+   stdin and are tailable.  */
+
+static bool
+tailable_stdin (const struct File_spec *f, size_t n_files)
+{
+  size_t i;
+
+  for (i = 0; i < n_files; i++)
+    if (!f[i].ignore && STREQ (f[i].name, "-"))
+      return true;
+  return false;
+}
+
+static bool
+fremote (int fd, const char *name)
+{
+  bool remote = true;           /* be conservative (poll by default).  */
+
+# if HAVE_FSTATFS && HAVE_STRUCT_STATFS_F_TYPE && defined __linux__
+  struct statfs buf;
+  int err = fstatfs (fd, &buf);
+  if (err != 0)
+    {
+      error (0, errno, _("cannot determine location of %s. "
+                         "reverting to polling"), quote (name));
+    }
+  else
+    {
+      switch (buf.f_type)
+        {
+        case S_MAGIC_AFS:
+        case S_MAGIC_CIFS:
+        case S_MAGIC_CODA:
+        case S_MAGIC_FUSEBLK:
+        case S_MAGIC_FUSECTL:
+        case S_MAGIC_GFS:
+        case S_MAGIC_KAFS:
+        case S_MAGIC_LUSTRE:
+        case S_MAGIC_NCP:
+        case S_MAGIC_NFS:
+        case S_MAGIC_NFSD:
+        case S_MAGIC_OCFS2:
+        case S_MAGIC_SMB:
+          break;
+        default:
+          remote = false;
+        }
+    }
+# endif
+
+  return remote;
+}
+
 static size_t
 wd_hasher (const void *entry, size_t tabsize)
 {
@@ -1310,7 +1399,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
       struct inotify_event *ev;
 
       /* When watching a PID, ensure that a read from WD will not block
-         indefinetely.  */
+         indefinitely.  */
       if (pid)
         {
           if (writer_is_dead)
@@ -1362,7 +1451,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
       ev = (struct inotify_event *) (evbuf + evbuf_off);
       evbuf_off += sizeof (*ev) + ev->len;
 
-      if (ev->len)
+      if (ev->len) /* event on ev->name in watched directory  */
         {
           for (i = 0; i < n_files; i++)
             {
@@ -1377,6 +1466,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
           if (i == n_files)
             continue;
 
+          /* It's fine to add the same file more than once.  */
           f[i].wd = inotify_add_watch (wd, f[i].name, inotify_wd_mask);
 
           if (f[i].wd < 0)
@@ -1649,6 +1739,9 @@ tail_file (struct File_spec *f, uintmax_t n_units)
                  to avoid a race condition described by Ken Raeburn:
         http://mail.gnu.org/archive/html/bug-textutils/2003-05/msg00007.html */
               record_open_fd (f, fd, read_pos, &stats, (is_stdin ? -1 : 1));
+#if HAVE_INOTIFY
+              f->remote = fremote (fd, pretty_name (f));
+#endif
             }
         }
       else
@@ -2012,19 +2105,28 @@ main (int argc, char **argv)
   if (forever && ignore_fifo_and_pipe (F, n_files))
     {
 #if HAVE_INOTIFY
-      /* If the user specifies stdin via a command line argument of "-",
-         or implicitly by providing no arguments, we won't use inotify.
+      /* tailable_stdin() checks if the user specifies stdin via  "-",
+         or implicitly by providing no arguments. If so, we won't use inotify.
          Technically, on systems with a working /dev/stdin, we *could*,
          but would it be worth it?  Verifying that it's a real device
          and hooked up to stdin is not trivial, while reverting to
-         non-inotify-based tail_forever is easy and portable.  */
-      bool stdin_cmdline_arg = false;
-
-      for (i = 0; i < n_files; i++)
-        if (!F[i].ignore && STREQ (F[i].name, "-"))
-          stdin_cmdline_arg = true;
-
-      if (!disable_inotify && !stdin_cmdline_arg)
+         non-inotify-based tail_forever is easy and portable.
+
+         any_remote_files() checks if the user has specified any
+         files that reside on remote file systems.  inotify is not used
+         in this case because it would miss any updates to the file
+         that were not initiated from the local system.
+
+         FIXME: inotify doesn't give any notification when a new
+         (remote) file or directory is mounted on top a watched file.
+         When follow_mode == Follow_name we would ideally like to detect that.
+         Note if there is a change to the original file then we'll
+         recheck it and follow the new file, or ignore it if the
+         file has changed to being remote.  */
+      if (tailable_stdin (F, n_files) || any_remote_files (F, n_files))
+        disable_inotify = true;
+
+      if (!disable_inotify)
         {
           int wd = inotify_init ();
           if (wd < 0)