>From 0229b8ae7c2bac52209a7dc980b4be8ea0dad895 Mon Sep 17 00:00:00 2001 From: =?utf-8?q?P=C3=A1draig=20Brady?= Date: Mon, 14 Dec 2009 22:45:34 +0000 Subject: [PATCH] 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. --- NEWS | 5 ++ src/tail.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 121 insertions(+), 14 deletions(-) diff --git a/NEWS b/NEWS index ac5bd07..8395d91 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,11 @@ GNU coreutils NEWS -*- outline -*- and rpc_pipefs. Also Minix V3 is displayed correctly as minix3, not minux3. [bug introduced in coreutils-8.1] + tail -f (inotify-enabled) once again works with remote files. + The use of inotify with remote files meant that any changes to those + files that was not done from the local system would go unnoticed. + [bug introduced in coreutils-7.5] + touch -a once again guarantees that a file's change time is adjusted, working around a bug in current Linux kernels. [bug introduced in coreutils-8.1] diff --git a/src/tail.c b/src/tail.c index 71f8a32..0256804 100644 --- a/src/tail.c +++ b/src/tail.c @@ -52,6 +52,12 @@ # include /* `select' is used by tail_forever_inotify. */ # include + +/* inotify needs to know if a file is local. */ +# include "fs.h" +# if HAVE_SYS_STATFS_H +# include +# 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) -- 1.6.2.5