From: Pádraig Brady Date: Tue, 12 Oct 2010 00:39:58 +0000 (+0100) Subject: tail: fix checking of currently unavailable directories X-Git-Tag: v8.6~9 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=61b77891c2d9af299063850a0c4d1d721340cfff;p=platform%2Fupstream%2Fcoreutils.git tail: fix checking of currently unavailable directories * src/tail.c (tail_forever_inotify): Handle the case where tail --follow=name with inotify, is not able to add a watch on a specified directory. This may happen due to inotify resource limits or if the directory is currently missing or inaccessible. In all these cases, revert to polling which will try to reopen the file later. Note inotify returns ENOSPC when it runs out of resources, and instead we report a particular error message, lest users think one of their file systems is full. (main): Document another caveat with using inotify, where we currently don't recheck directories recreated after the initial watch is setup. * tests/tail-2/F-vs-rename: Fix the endless loop triggered by the above issue. * tests/tail-2/inotify-hash-abuse: Likewise. * tests/tail-2/wait: Don't fail in the resource exhaustion case. * tests/tail-2/F-vs-missing: A new test for this failure mode which was until now just triggered on older buggy linux kernels which returned ENOSPC constantly from inotify_add_watch(). * NEWS: Mention the fix. --- diff --git a/NEWS b/NEWS index 7ee18cd..3f81077 100644 --- a/NEWS +++ b/NEWS @@ -21,6 +21,10 @@ GNU coreutils NEWS -*- outline -*- tac would perform a double-free when given an input line longer than 16KiB. [bug introduced in coreutils-8.3] + tail -F once again notices changes in a currently unavailable directory, + and works around a Linux kernel bug where inotify runs out of resources. + [bugs introduced in coreutils-7.5] + tr now consistently handles case conversion character classes. In some locales, valid conversion specifications caused tr to abort, while in all locales, some invalid specifications were undiagnosed. diff --git a/src/tail.c b/src/tail.c index 75e9d53..68cc819 100644 --- a/src/tail.c +++ b/src/tail.c @@ -1300,9 +1300,10 @@ check_fspec (struct File_spec *fspec, int wd, int *prev_wd) error (EXIT_FAILURE, errno, _("write error")); } -/* Tail N_FILES files forever, or until killed. - Check modifications using the inotify events system. */ -static void +/* Attempt to tail N_FILES files forever, or until killed. + Check modifications using the inotify events system. + Return false on error, or true to revert to polling. */ +static bool tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, double sleep_interval) { @@ -1311,7 +1312,9 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, /* Map an inotify watch descriptor to the name of the file it's watching. */ Hash_table *wd_to_name; - bool found_watchable = false; + bool found_watchable_file = false; + bool found_unwatchable_dir = false; + bool no_inotify_resources = false; bool writer_is_dead = false; int prev_wd; size_t evlen = 0; @@ -1355,9 +1358,15 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, if (f[i].parent_wd < 0) { - error (0, errno, _("cannot watch parent directory of %s"), - quote (f[i].name)); - continue; + if (errno != ENOSPC) /* suppress confusing error. */ + error (0, errno, _("cannot watch parent directory of %s"), + quote (f[i].name)); + else + error (0, 0, _("inotify resources exhausted")); + found_unwatchable_dir = true; + /* We revert to polling below. Note invalid uses + of the inotify API will still be diagnosed. */ + break; } } @@ -1365,7 +1374,12 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, if (f[i].wd < 0) { - if (errno != f[i].errnum) + if (errno == ENOSPC) + { + no_inotify_resources = true; + error (0, 0, _("inotify resources exhausted")); + } + else if (errno != f[i].errnum) error (0, errno, _("cannot watch %s"), quote (f[i].name)); continue; } @@ -1373,12 +1387,22 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, if (hash_insert (wd_to_name, &(f[i])) == NULL) xalloc_die (); - found_watchable = true; + found_watchable_file = true; } } - if (follow_mode == Follow_descriptor && !found_watchable) - return; + /* Linux kernel 2.6.24 at least has a bug where eventually, ENOSPC is always + returned by inotify_add_watch. In any case we should revert to polling + when there are no inotify resources. Also a specified directory may not + be currently present or accessible, so revert to polling. */ + if (no_inotify_resources || found_unwatchable_dir) + { + /* FIXME: release hash and inotify resources allocated above. */ + errno = 0; + return true; + } + if (follow_mode == Follow_descriptor && !found_watchable_file) + return false; prev_wd = f[n_files - 1].wd; @@ -2140,16 +2164,18 @@ main (int argc, char **argv) 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. */ + file has changed to being remote. + + FIXME: when using inotify, and a directory for a watched file + is recreated, then we don't recheck any new file when + follow_mode == Follow_name */ if (tailable_stdin (F, n_files) || any_remote_file (F, n_files)) disable_inotify = true; if (!disable_inotify) { int wd = inotify_init (); - if (wd < 0) - error (0, errno, _("inotify cannot be used, reverting to polling")); - else + if (0 <= wd) { /* Flush any output from tail_file, now, since tail_forever_inotify flushes only after writing, @@ -2157,11 +2183,10 @@ main (int argc, char **argv) if (fflush (stdout) != 0) error (EXIT_FAILURE, errno, _("write error")); - tail_forever_inotify (wd, F, n_files, sleep_interval); - - /* The only way the above returns is upon failure. */ - exit (EXIT_FAILURE); + if (!tail_forever_inotify (wd, F, n_files, sleep_interval)) + exit (EXIT_FAILURE); } + error (0, errno, _("inotify cannot be used, reverting to polling")); } #endif tail_forever (F, n_files, sleep_interval); diff --git a/tests/Makefile.am b/tests/Makefile.am index a6a0594..41e0cbc 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -84,6 +84,7 @@ TESTS = \ cp/link-heap \ tail-2/inotify-hash-abuse \ tail-2/inotify-hash-abuse2 \ + tail-2/F-vs-missing \ tail-2/F-vs-rename \ tail-2/inotify-rotate \ chmod/no-x \ diff --git a/tests/tail-2/F-vs-missing b/tests/tail-2/F-vs-missing new file mode 100755 index 0000000..8c1d018 --- /dev/null +++ b/tests/tail-2/F-vs-missing @@ -0,0 +1,49 @@ +#!/bin/sh +# demonstrate that tail -F works for currently missing dirs +# Before coreutils-8.6, tail -F missing/file would not +# notice any subsequent availability of the missing/file. + +# Copyright (C) 2010 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=.}/init.sh"; path_prepend_ ../src +test "$VERBOSE" = yes && tail --version + +debug='---disable-inotify' +debug= +tail $debug -F -s.1 missing/file > out 2>&1 & pid=$! + +check_tail_output() +{ + local delay="$1" + grep "$tail_re" out > /dev/null || + { sleep $delay; return 1; } +} + +# Wait up to 6.3s for tail to start with diagnostic: +# tail: cannot open `missing/file' for reading: No such file or directory +tail_re='cannot open' retry_delay_ check_tail_output .1 7 || fail=1 + +mkdir missing || fail=1 +(cd missing && echo x > file) + +# Wait up to 6.3s for this to appear in the output: +# "tail: `...' has appeared; following end of new file" +tail_re='has appeared' retry_delay_ check_tail_output .1 7 || + { echo "$0: file: unexpected delay?"; cat out; fail=1; } + +kill -HUP $pid + +Exit $fail diff --git a/tests/tail-2/F-vs-rename b/tests/tail-2/F-vs-rename index 74db801..98b36e7 100755 --- a/tests/tail-2/F-vs-rename +++ b/tests/tail-2/F-vs-rename @@ -18,39 +18,36 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -if test "$VERBOSE" = yes; then - set -x - tail --version -fi - -. $srcdir/test-lib.sh +. "${srcdir=.}/init.sh"; path_prepend_ ../src +test "$VERBOSE" = yes && tail --version touch a b || framework_failure -debug='---disable-inotify -s .01' +debug='---disable-inotify' debug= -tail $debug -F a b > out 2>&1 & pid=$! +tail $debug -F -s.1 a b > out 2>&1 & pid=$! + +check_tail_output() +{ + local delay="$1" + grep "$tail_re" out > /dev/null || + { sleep $delay; return 1; } +} -# Wait until tail has started... +# Wait up to 6.3s for tail to start echo x > a -until grep '^x$' out >/dev/null 2>&1; do :; done +tail_re='^x$' retry_delay_ check_tail_output .1 7 || fail=1 mv a b || fail=1 -# Wait for the diagnostic: +# Wait 6.3s for this diagnostic: # tail: `a' has become inaccessible: No such file or directory -until grep inaccessible out >/dev/null 2>&1; do :; done +tail_re='inaccessible' retry_delay_ check_tail_output .1 7 || fail=1 echo x > a # Wait up to 6.3s for this to appear in the output: # "tail: `...' has appeared; following end of new file" -tail_f_vs_rename_1() -{ - local delay="$1" - grep 'has appeared;' out > /dev/null || - { sleep $delay; return 1; } -} -retry_delay_ tail_f_vs_rename_1 .1 7 || +tail_re='has appeared' retry_delay_ check_tail_output .1 7 || { echo "$0: a: unexpected delay?"; cat out; fail=1; } echo y >> b diff --git a/tests/tail-2/inotify-hash-abuse b/tests/tail-2/inotify-hash-abuse index 65a3412..d45668a 100755 --- a/tests/tail-2/inotify-hash-abuse +++ b/tests/tail-2/inotify-hash-abuse @@ -16,30 +16,33 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -if test "$VERBOSE" = yes; then - set -x - tail --version -fi - -. $srcdir/test-lib.sh +. "${srcdir=.}/init.sh"; path_prepend_ ../src +test "$VERBOSE" = yes && tail --version # 9 is a magic number, related to internal details of tail.c and hash.c n=9 seq $n | xargs touch || framework_failure -debug='---disable-inotify -s .01' +debug='---disable-inotify' debug= -tail $debug -qF $(seq $n) > out 2>&1 & pid=$! +tail $debug -s.1 -qF $(seq $n) > out 2>&1 & pid=$! + +check_tail_output() +{ + local delay="$1" + grep "$tail_re" out > /dev/null || + { sleep $delay; return 1; } +} -# Wait until tail has started... +# Wait up to 6.3s for tail to start echo x > $n -until grep '^x$' out >/dev/null 2>&1; do :; done +tail_re='^x$' retry_delay_ check_tail_output .1 7 || fail=1 mv 1 f || fail=1 -# Wait for this diagnostic: +# Wait 6.3s for this diagnostic: # tail: `1' has become inaccessible: No such file or directory -until grep inaccessible out >/dev/null 2>&1; do :; done +tail_re='inaccessible' retry_delay_ check_tail_output .1 7 || fail=1 # Trigger the bug. Before the fix, this would provoke the abort. echo a > 1 || fail=1 diff --git a/tests/tail-2/wait b/tests/tail-2/wait index 332cfcc..a963691 100755 --- a/tests/tail-2/wait +++ b/tests/tail-2/wait @@ -53,7 +53,7 @@ for inotify in ---disable-inotify ''; do timeout 1 tail -s0.1 -F $inotify not_here test $? = 124 || fail=1 - grep -v 'tail: inotify cannot be used, reverting to polling:' tail.err > x + grep -Ev 'inotify (resources exhausted|cannot be used)' tail.err > x mv x tail.err test -s tail.err && fail=1 :>tail.err