From 6878ae412d8e7fb65ad77a3d68af986377d9d977 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Tue, 9 Jun 2015 00:53:35 +0100 Subject: [PATCH 1/2] tail: display file headers correctly with inotify * src/tail.c (tail_forever_inotify): Use the fspec pointer to distinguish previously output files, rather than a descriptor from the inotify event. That event descriptor was that of the parent directory when files were created or renamed etc. (check_fspec): Adjust for the new comparison. Also show the header when the file is truncated, since we show data in this case also. * tests/tail-2/F-headers.sh: A new test case. * tests/local.mk: Reference the new test. * NEWS: Mention the bug fix. --- NEWS | 4 ++++ src/tail.c | 22 ++++++++++-------- tests/local.mk | 1 + tests/tail-2/F-headers.sh | 59 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 10 deletions(-) create mode 100755 tests/tail-2/F-headers.sh diff --git a/NEWS b/NEWS index c2576c5..4c0f6e4 100644 --- a/NEWS +++ b/NEWS @@ -56,6 +56,10 @@ GNU coreutils NEWS -*- outline -*- tail --follow consistently outputs all data for a truncated file. [bug introduced in the beginning] + tail --follow=name correctly outputs headers for multiple files + when those files are being created or renamed. + [bug introduced in coreutils-7.5] + ** New features chroot accepts the new --skip-chdir option to not change the working directory diff --git a/src/tail.c b/src/tail.c index c9736ca..4d6b28a 100644 --- a/src/tail.c +++ b/src/tail.c @@ -1046,7 +1046,9 @@ recheck (struct File_spec *f, bool blocking) { /* This happens when one iteration finds the file missing, then the preceding pair is reused as the - file is recreated. */ + file is recreated. Note this also means the file is redisplayed + in --follow=name mode if renamed away from and back to + a monitored name. */ new_file = true; } else @@ -1321,7 +1323,7 @@ wd_comparator (const void *e1, const void *e2) /* Output (new) data for FSPEC->fd. */ static void -check_fspec (struct File_spec *fspec, int wd, int *prev_wd) +check_fspec (struct File_spec *fspec, struct File_spec **prev_fspec) { struct stat stats; char const *name; @@ -1347,7 +1349,6 @@ check_fspec (struct File_spec *fspec, int wd, int *prev_wd) if (S_ISREG (fspec->mode) && stats.st_size < fspec->size) { error (0, 0, _("%s: file truncated"), name); - *prev_wd = wd; xlseek (fspec->fd, 0, SEEK_SET, name); fspec->size = 0; } @@ -1355,11 +1356,11 @@ check_fspec (struct File_spec *fspec, int wd, int *prev_wd) && timespec_cmp (fspec->mtime, get_stat_mtime (&stats)) == 0) return; - if (wd != *prev_wd) + if (fspec != *prev_fspec) { if (print_headers) write_header (name); - *prev_wd = wd; + *prev_fspec = fspec; } uintmax_t bytes_read = dump_remainder (name, fspec->fd, COPY_TO_EOF); @@ -1391,7 +1392,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, bool found_unwatchable_dir = false; bool no_inotify_resources = false; bool writer_is_dead = false; - int prev_wd; + struct File_spec *prev_fspec; size_t evlen = 0; char *evbuf; size_t evbuf_off = 0; @@ -1492,7 +1493,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, if (follow_mode == Follow_descriptor && !found_watchable_file) return false; - prev_wd = f[n_files - 1].wd; + prev_fspec = &(f[n_files - 1]); /* Check files again. New files or data can be available since last time we checked and before they are watched by inotify. */ @@ -1524,7 +1525,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, } /* check for new data. */ - check_fspec (&f[i], f[i].wd, &prev_wd); + check_fspec (&f[i], &prev_fspec); } } @@ -1703,7 +1704,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, continue; } - check_fspec (fspec, ev->wd, &prev_wd); + check_fspec (fspec, &prev_fspec); } } #endif @@ -2316,7 +2317,8 @@ main (int argc, char **argv) FIXME-maybe: inotify has a watch descriptor per inode, and hence with our current hash implementation will only --follow data for one - of the names when multiple hardlinked files are specified. */ + of the names when multiple hardlinked files are specified, or + for one name when a name is specified multiple times. */ if (!disable_inotify && (tailable_stdin (F, n_files) || any_remote_file (F, n_files) || any_symlinks (F, n_files) diff --git a/tests/local.mk b/tests/local.mk index bb78796..7b8c91e 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -172,6 +172,7 @@ all_tests = \ tests/tail-2/inotify-hash-abuse2.sh \ tests/tail-2/F-vs-missing.sh \ tests/tail-2/F-vs-rename.sh \ + tests/tail-2/F-headers.sh \ tests/tail-2/descriptor-vs-rename.sh \ tests/tail-2/inotify-rotate.sh \ tests/tail-2/inotify-rotate-resources.sh \ diff --git a/tests/tail-2/F-headers.sh b/tests/tail-2/F-headers.sh new file mode 100755 index 0000000..31d0fa6 --- /dev/null +++ b/tests/tail-2/F-headers.sh @@ -0,0 +1,59 @@ +#!/bin/sh +# Ensure tail -F distinguishes output with the correct headers +# Between coreutils 7.5 and 8.23 inclusive, 'tail -F ...' would +# not output headers for or created/renamed files in certain cases. + +# Copyright (C) 2015 Free Software Foundation, Inc. + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src +print_ver_ tail + +check_tail_output() +{ + local delay="$1" + grep "$tail_re" out || + { sleep $delay; return 1; } +} + +# Terminate any background tail process +cleanup_() { kill $pid 2>/dev/null && wait $pid; } + +# Speedup the non inotify case +fastpoll='-s.1 --max-unchanged-stats=1' + +for mode in '' '---disable-inotify'; do + rm -f a b out + + tail $mode -F $fastpoll a b > out 2>&1 & pid=$! + + # Wait up to 12.7s for tail to start. + tail_re="cannot open 'b'" retry_delay_ check_tail_output .1 7 || + { cat out; fail=1; } + + echo x > a + # Wait up to 12.7s for a's header to appear in the output: + tail_re='==> a <==' retry_delay_ check_tail_output .1 7 || + { echo "$0: a: unexpected delay?"; cat out; fail=1; } + + echo y > b + # Wait up to 12.7s for b's header to appear in the output: + tail_re='==> b <==' retry_delay_ check_tail_output .1 7 || + { echo "$0: b: unexpected delay?"; cat out; fail=1; } + + cleanup_ +done + +Exit $fail -- 2.4.1 From aee982627874e9719bc151007b7e0144edd3963e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Tue, 9 Jun 2015 11:33:44 +0100 Subject: [PATCH 2/2] tail: display consistent diagnostics upon file replacement * src/tail.c (recheck): Display diagnostices for replaced files even with reused inodes which is a common case. * tests/tail-2/F-vs-missing.sh: Use correct diagnostic in comment. * tests/tail-2/F-vs-rename.sh: Likewise. --- src/tail.c | 55 ++++++++++++++++++++------------------------ tests/tail-2/F-vs-missing.sh | 2 +- tests/tail-2/F-vs-rename.sh | 2 +- 3 files changed, 27 insertions(+), 32 deletions(-) diff --git a/src/tail.c b/src/tail.c index 4d6b28a..c062d40 100644 --- a/src/tail.c +++ b/src/tail.c @@ -1019,42 +1019,37 @@ recheck (struct File_spec *f, bool blocking) assert (f->fd == -1); error (0, 0, _("%s has become accessible"), quote (pretty_name (f))); } + else if (f->fd == -1) + { + /* A new file even when inodes haven't changed as + pairs can be reused, and we know the file was missing + on the previous iteration. Note this also means the file + is redisplayed in --follow=name mode if renamed away from + and back to a monitored name. */ + new_file = true; + + error (0, 0, + _("%s has appeared; following new file"), + quote (pretty_name (f))); + } else if (f->ino != new_stats.st_ino || f->dev != new_stats.st_dev) { + /* File has been replaced (e.g., via log rotation) -- + tail the new one. */ new_file = true; - if (f->fd == -1) - { - error (0, 0, - _("%s has appeared; following new file"), - quote (pretty_name (f))); - } - else - { - /* Close the old one. */ - close_fd (f->fd, pretty_name (f)); - - /* File has been replaced (e.g., via log rotation) -- - tail the new one. */ - error (0, 0, - _("%s has been replaced; following new file"), - quote (pretty_name (f))); - } + + error (0, 0, + _("%s has been replaced; following new file"), + quote (pretty_name (f))); + + /* Close the old one. */ + close_fd (f->fd, pretty_name (f)); + } else { - if (f->fd == -1) - { - /* This happens when one iteration finds the file missing, - then the preceding pair is reused as the - file is recreated. Note this also means the file is redisplayed - in --follow=name mode if renamed away from and back to - a monitored name. */ - new_file = true; - } - else - { - close_fd (fd, pretty_name (f)); - } + /* No changes detected, so close new fd. */ + close_fd (fd, pretty_name (f)); } /* FIXME: When a log is rotated, daemons tend to log to the diff --git a/tests/tail-2/F-vs-missing.sh b/tests/tail-2/F-vs-missing.sh index be20ee3..54fe30a 100755 --- a/tests/tail-2/F-vs-missing.sh +++ b/tests/tail-2/F-vs-missing.sh @@ -48,7 +48,7 @@ for mode in '' '---disable-inotify'; do (cd missing && echo x > file) || framework_failure_ # Wait up to 12.7s for this to appear in the output: - # "tail: '...' has appeared; following end of new file" + # "tail: '...' has appeared; following new file" tail_re='has appeared' retry_delay_ check_tail_output .1 7 || { echo "$0: file: unexpected delay?"; cat out; fail=1; } diff --git a/tests/tail-2/F-vs-rename.sh b/tests/tail-2/F-vs-rename.sh index ee61a54..06733fb 100755 --- a/tests/tail-2/F-vs-rename.sh +++ b/tests/tail-2/F-vs-rename.sh @@ -53,7 +53,7 @@ for mode in '' '---disable-inotify'; do echo x > a # Wait up to 12.7s for this to appear in the output: - # "tail: '...' has appeared; following end of new file" + # "tail: '...' has appeared; following new file" tail_re='has appeared' retry_delay_ check_tail_output .1 7 || { echo "$0: a: unexpected delay?"; cat out; fail=1; } -- 2.4.1