Re: inotify back end for tail -f on linux

[Jim Meyering]

Giuseppe Scrivano wrote:
> Jim Meyering <address@hidden> writes:
>
>> Note what happens whey you truncate a tracked file.
>> With your version, nothing.
>>
>> With the original, it reports this:
>>
>>   tail: F: file truncated
>>
>> Likewise, when tailing with e.g., "tail -F FILE &"
>> if you unlink FILE, you get this:
>>
>>   tail: `FILE' has become inaccessible: No such file or directory
>>
>> and if you then recreate it:
>>
>>   tail: `FILE' has appeared;  following end of new file
>>
>> You get none of that with your patch.
>
> Thank you for the quick review.  I added some messages that are possible
> in `Follow_descriptor' mode but I had to rollback to the old polling
> mechanism when `Follow_name' is used.  It is not straightforward to
> emulate it using inotify because it is not possible to watch a file that
> doesn't exist yet or a file that is removed and re-created.
> It is possible to watch the parent directory if new files are created
> but then the problem is moved on the parent directory that can be
> removed too and so on (or am I misunderstanding something with
> inotify?).

If we add inotify support, I'd like it to work for
both tail-by-FD and tail-by-name.

You can watch the parent directory for e.g., rename and creation events.
Log rotation is so common, that it'd be a shame not to be able to take
advantage of inotify when using "tail -F ...".

For starters, you needn't worry about recovering from parent-dir-removal.
I think that parent removal is rare enough in practice that we won't
need to deal with it for some time.

> I tried `inotify_add_watch' with a file that doesn't exist and a file
> that exists, removed and re-created, registering them with
> IN_ALL_EVENTS.  In the former case I get an error, in the latter I don't
> get any event when it is re-created as it is silently dropped from the
> watch list when the file is removed.
>
> Is there a way to monitor a path that doesn't exist yet and be alerted
> when it is created without polling?

Eventually, the code could monitor the longest parent prefix that *does* exist.
When the missing subdir reappears, monitor $parent/$subdir, etc.
However, there's no need to do that now.

This added complexity, and the additional features
we could gain via inotify, are why I was encouraging people
to add the feature via a new program.  And one not written
(at least not initially) in C.

I haven't looked closely at the code below or even tried it,
but here are a few superficial comments:

> @@ -45,6 +45,10 @@
>  #include "xstrtol.h"
>  #include "xstrtod.h"
>
> +#ifdef HAVE_INOTIFY
> +#include <sys/inotify.h>
> +#endif

Indent cpp directives to reflect nesting:

  #ifdef HAVE_INOTIFY
  # include <sys/inotify.h>
  #endif

>  /* The official name of this program (e.g., no `g' prefix).  */
>  #define PROGRAM_NAME "tail"
>
> @@ -1116,6 +1120,114 @@ tail_forever (struct File_spec *f, int nfiles, double 
> sleep_interval)
>      }
>  }
>
> +#ifdef HAVE_INOTIFY
> +/* Tail NFILES files forever, or until killed.
> +   Check modifications using the inotify events system.  */
> +static void
> +tail_forever_inotify (int wd, struct File_spec *f, int nfiles)
> +{
> +  /* Create an index to access File_spec by its watch descriptor
> +   * in costant time.  */
> +  struct File_spec **wd_index;
> +  int i;
> +  int *watch_fd = xmalloc (sizeof (int) * nfiles);
> +  int min_wd = -1;
> +  int max_wd = -1;
> +  int watched = 0;
> +  size_t last;
> +
> +  for (i = 0; i < nfiles; i++)
> +    {
> +      watch_fd[i] = -1;
> +
> +      if (!f[i].ignore)
> +        {
> +          watch_fd[i] = inotify_add_watch (wd, f[i].name, 
> IN_MODIFY|IN_ATTRIB);
> +
> +          if (watch_fd[i] < 0)
> +            {
> +              error (0, errno, _("cannot watch %s"), quote (f[i].name));
> +              continue;
> +            }
> +
> +          watched++;
> +
> +          if (watch_fd[i] > max_wd || max_wd < 0)
> +              max_wd = watch_fd[i];
> +
> +          if (watch_fd[i] < min_wd || min_wd < 0)
> +              min_wd = watch_fd[i];
> +        }
> +    }
> +
> +  if (!watched)
> +    return;
> +
> +  wd_index = xmalloc (sizeof (struct File_spec**) * (max_wd - min_wd + 1));

Use sizeof VAR, not sizeof TYPE.

     wd_index = xmalloc (sizeof *wd_index * (max_wd - min_wd + 1));

> +  for (i = 0; i < nfiles; i++)
> +    if (watch_fd[i] > 0)

Oops.  Off-by-one.  Don't skip a file descriptor of 0, however unlikely.
Also, remember I prefer "<" and "<=" over > and >=:

       if (0 <= watch_fd[i])

> +      wd_index[watch_fd[i] - min_wd] = &(f[i]);
> +
> +  free (watch_fd);
> +
> +  last = max_wd + 1;
> +
> +  while (1)
> +    {
> +      size_t len;
> +      struct inotify_event ev;
> +      char const *name;
> +      struct File_spec *fspec;
> +      uintmax_t bytes_read;
> +      struct stat stats;
> +
> +      len = read (wd, &ev, sizeof (struct inotify_event));

use sizeof VAR here, too

> +      if (!len && errno == EINTR)
> +        continue;
> +
> +      if (!len)
> +        error (EXIT_FAILURE, errno, _("error reading inotify event"));

This function must not exit upon a failure that is specific
to a single file.  Just diagnose it and continue with any other files.
Of course, if there are no other usable file descriptors, *then*
you will need to exit the loop -- since this is currently used only
when tailing-by-FD.

You would not exit a similar loop when tailing by name.