>From f7378ea3c118633a4a5a925700a9d9a7f15af3be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Fri, 13 Sep 2013 17:31:24 +0200 Subject: [PATCH] tail: improve inotify handling of symlinks Previous behavior failed to read contents of a (re)appearing file, when symlinked by tail's watched file. Also we now diagnose other edge cases when running in inotify mode, where an initially missing or regular file changes to a symlink. * src/tail.c (main): If any arg is a symlink, use polling mode. (recheck): Diagnose the edge case where a symlink appears during inotify processing. * tests/tail-2/symlink.sh: Test the fix. Mention the edge cases. * tests/local.mk: Reference the new test. * NEWS: Mention the fix. Reported by: Ondrej Oprala --- NEWS | 7 +++- src/tail.c | 39 +++++++++++++++++++++++- tests/local.mk | 3 +- tests/tail-2/symlink.sh | 78 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 4 deletions(-) create mode 100755 tests/tail-2/symlink.sh diff --git a/NEWS b/NEWS index b6dab62..cf40f50 100644 --- a/NEWS +++ b/NEWS @@ -43,8 +43,11 @@ GNU coreutils NEWS -*- outline -*- by the alignment bug introduced in coreutils-6.0] tail --retry -f now waits for the files specified to appear. Before, tail - would immediately exit when such a file is inaccessible during the initial - open. + would immediately exit when such a file is initially inaccessible. + [This bug was introduced when inotify support was added in coreutils-7.5] + + tail -F had improved handling of symlinks. Previously tail didn't respond + to the symlink target (re)appearing after being (re)created. [This bug was introduced when inotify support was added in coreutils-7.5] ** New features diff --git a/src/tail.c b/src/tail.c index e7fefda..c781dc2 100644 --- a/src/tail.c +++ b/src/tail.c @@ -945,7 +945,20 @@ recheck (struct File_spec *f, bool blocking) then mark the file as not tailable. */ f->tailable = !(reopen_inaccessible_files && fd == -1); - if (fd == -1 || fstat (fd, &new_stats) < 0) + if (! disable_inotify && ! lstat (f->name, &new_stats) + && S_ISLNK (new_stats.st_mode)) + { + /* Diagnose the edge case where a regular file is changed + to a symlink. We avoid inotify with symlinks since + it's awkward to match between symlink name and target. */ + ok = false; + f->errnum = -1; + f->ignore = true; + + error (0, 0, _("%s has been replaced with a symbolic link. " + "giving up on this name"), quote (pretty_name (f))); + } + else if (fd == -1 || fstat (fd, &new_stats) < 0) { ok = false; f->errnum = errno; @@ -1251,6 +1264,23 @@ any_remote_file (const struct File_spec *f, size_t n_files) return false; } +/* Return true if any of the N_FILES files in F is a symlink. + Note we don't worry about the edge case where "-" exists, + since that will have the same consequences for inotify, + which is the only context this function is currently used. */ + +static bool +any_symlinks (const struct File_spec *f, size_t n_files) +{ + size_t i; + + struct stat st; + for (i = 0; i < n_files; i++) + if (lstat (f[i].name, &st) == 0 && S_ISLNK (st.st_mode)) + return true; + return false; +} + /* Return true if any of the N_FILES files in F represents stdin and is tailable. */ @@ -1531,6 +1561,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, int new_wd = inotify_add_watch (wd, f[j].name, inotify_wd_mask); if (new_wd < 0) { + /* Can get ENOENT for a dangling symlink for example. */ error (0, errno, _("cannot watch %s"), quote (f[j].name)); continue; } @@ -2206,6 +2237,11 @@ main (int argc, char **argv) in this case because it would miss any updates to the file that were not initiated from the local system. + any_symlinks() checks if the user has specified any symbolic links. + inotify is not used in this case because it returns updated _targets_ + which would not match the specified names. If we tried to always + use the target names, then we would miss changes to the symlink itself. + ok is false when one of the files specified could not be opened for reading. In this case and when following by descriptor, tail_forever_inotify() cannot be used (in its current implementation). @@ -2222,6 +2258,7 @@ main (int argc, char **argv) follow_mode == Follow_name */ if (!disable_inotify && (tailable_stdin (F, n_files) || any_remote_file (F, n_files) + || any_symlinks (F, n_files) || (!ok && follow_mode == Follow_descriptor))) disable_inotify = true; diff --git a/tests/local.mk b/tests/local.mk index 3c92425..0a82221 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -388,7 +388,8 @@ all_tests = \ tests/misc/uniq-perf.sh \ tests/misc/xattr.sh \ tests/tail-2/wait.sh \ - tests/tail-2/retry.sh \ + tests/tail-2/retry.sh \ + tests/tail-2/symlink.sh \ tests/chmod/c-option.sh \ tests/chmod/equal-x.sh \ tests/chmod/equals.sh \ diff --git a/tests/tail-2/symlink.sh b/tests/tail-2/symlink.sh new file mode 100755 index 0000000..8b220b2 --- /dev/null +++ b/tests/tail-2/symlink.sh @@ -0,0 +1,78 @@ +#!/bin/sh +# Ensure tail tracks symlinks properly + +# Copyright (C) 2013 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 + +# Function to check the expected line count in 'out'. +# Called via retry_delay_(). Sleep some time - see retry_delay_() - if the +# line count is still smaller than expected. +wait4lines_ () +{ + local delay=$1 + local elc=$2 # Expected line count. + [ "$( wc -l < out )" -ge "$elc" ] || { sleep $delay; return 1; } +} + +# Ensure changing targets of cli specified symlinks are handled +# Prior to v8.22, inotify would fail to recognize changes in the targets +# Clear 'out' so that we can check its contents without races +:>out || framework_failure_ +ln -nsf target symlink || framework_failure_ +timeout 10 tail -s.1 -F symlink >out 2>&1 & pid=$! +retry_delay_ wait4lines_ .1 6 1 || fail=1 # Wait for "cannot open..." +echo "X" > target || fail=1 +retry_delay_ wait4lines_ .1 6 3 || fail=1 # Wait for the expected output. +kill $pid +wait $pid +# Expect 3 lines in the output file. +[ $( wc -l < out ) = 3 ] || { fail=1; cat out; } +grep -F 'cannot open' out || { fail=1; cat out; } +grep -F 'has appeared' out || { fail=1; cat out; } +grep '^X$' out || { fail=1; cat out; } +rm -f target out || framework_failure_ + +# Ensure we correctly handle the source symlink itself changing. +# I.E. that we don't operate solely on the targets. +# Clear 'out' so that we can check its contents without races +:>out || framework_failure_ +touch target1 || framework_failure_ +ln -nsf target1 symlink || framework_failure_ +timeout 10 tail -s.1 -F symlink >out 2>&1 & pid=$! +echo "X1" > target1 +retry_delay_ wait4lines_ .1 6 1 || fail=1 # Wait for the expected output. +ln -nsf target2 symlink || framework_failure_ +retry_delay_ wait4lines_ .1 6 2 || fail=1 # Wait for "become inaccess..." +echo "X2" > target2 || fail=1 +retry_delay_ wait4lines_ .1 6 4 || fail=1 # Wait for the expected output. +kill $pid +wait $pid +# Expect 3 lines in the output file. +[ $( wc -l < out ) = 4 ] || { fail=1; cat out; } +grep -F 'become inaccessible' out || { fail=1; cat out; } +grep -F 'has appeared' out || { fail=1; cat out; } +grep '^X[12]$' out || { fail=1; cat out; } +rm -f target1 target2 out || framework_failure_ + +# Note other symlink edge cases are currently just diagnosed +# rather than being handled. I.E. if you specify a missing item, +# or existing file that later change to a symlink, if inotify +# is in use, you'll get a diagnostic saying that link will +# no longer be tailed. + +Exit $fail -- 1.7.7.6