On 13/12/09 13:06, Giuseppe Scrivano wrote:
Hello,
Jim Meyering<j...@meyering.net> writes:
Pádraig Brady wrote:
I've just noticed that `tail -f` will not work over NFS
as changes on the remote system will not go through
the local VFS and so will not be noticed by inotify.
So what to do? I suppose we could statfs("filename")
Yes, I think something like that is required.
For even less impact, call fstatfs on the file descriptor.
When this check should be done? At initialization before enter the
tail_forever/tail_forever_inotify loop? In this case, shouldn't we take
into account that the underyling FS can be changed when "tail -F" is
used? Like:
(sleep 5s; mount -F nfs server:/foo/bar /mnt/)&
tail -F /mnt/file
I got a few minutes to look at this today,
and the attached patch seems to work with a very quick test.
It doesn't handle the above remount case though
as if I mount the parent dir of a file or bind mount the file itself
then there are no inotify notifications. This remounting issue is
independent of nfs anyway. So can inotify handle this or will we
have to periodically check with a select rather than a blocking read?
cheers,
Pádraig.
commit c2c0c5b0e5ecf1f193eb29729e3a5315b858b142
Author: Pádraig Brady <p...@draigbrady.com>
Date: Mon Dec 14 22:45:34 2009 +0000
tail: fix --follow to not use inotify on remote files
* src/tail.c (......):
diff --git a/src/tail.c b/src/tail.c
index 71f8a32..3880444 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,18 @@ 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;
+ f->ignore = true;
+ f->remote = true;
+ error (0, 0, _("%s has been replaced with a remote file. "
+ "giving up on this name"), quote (pretty_name (f)));
+ return;
+ }
+#endif
else
{
f->errnum = 0;
@@ -1153,6 +1175,68 @@ 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 filesystems. */
+
+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_CIFS:
+ case S_MAGIC_CODA:
+ case S_MAGIC_FUSECTL:
+ case S_MAGIC_LUSTRE:
+ case S_MAGIC_NFS:
+ case S_MAGIC_NFSD:
+ case S_MAGIC_SMB:
+ break;
+ default:
+ remote = false;
+ }
+ }
+#endif
+
+ return remote;
+}
+
static size_t
wd_hasher (const void *entry, size_t tabsize)
{
@@ -1310,7 +1394,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)
@@ -1649,6 +1733,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 +2099,21 @@ 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;
+ non-inotify-based tail_forever is easy and portable.
- for (i = 0; i < n_files; i++)
- if (!F[i].ignore && STREQ (F[i].name, "-"))
- stdin_cmdline_arg = true;
+ any_remote_files() checks if the user has specified any
+ files that reside on remote filesystems. inotify is not used
+ in this case because it would miss any updates to the file
+ that were not initiated from the local system. */
+ if (tailable_stdin (F, n_files) || any_remote_files (F, n_files))
+ disable_inotify = true;
- if (!disable_inotify && !stdin_cmdline_arg)
+ if (!disable_inotify)
{
int wd = inotify_init ();
if (wd < 0)
