[Libguestfs] [PATCH v3 1/2] New tool: virt-tail.

[Pino Toscano]

On Monday, 3 October 2016 15:07:10 CEST Richard W.M. Jones wrote:
> This follows (tails) a log file within a guest, rather like
> the regular 'tail -f' command.  For example:
> 
>   virt-tail -d guest /var/log/messages
> ---

Mostly LGTM, just a few notes.

> +      guestfs_push_error_handler (g, NULL, NULL);
> +      stat = guestfs_statns (g, filename);
> +      guestfs_pop_error_handler (g);
> +      if (stat == NULL) {
> +        /* There's an error.  Treat ENOENT as if the file was empty size. */
> +        if (guestfs_last_errno (g) == ENOENT) {
> +          time (&t);
> +          file[i].mtime = t;
> +          file[i].size = 0;

I'd set size as -1, otherwise this is considering the file as existing.
If virt-tail (wants to) behaves like `tail -f`, I'd:
- print an error/message line (not fatal) to warn that a file does not
  exist
- when it appears, print that and start following it

> +          /* If we get here, the file changed and we're going to display
> +           * something.  If there is more than one file, and the file
> +           * displayed is different from previously, then display the
> +           * filename banner.
> +           */
> +          if (i != prev_file_displayed)
> +            printf ("\n\n--- %s ---\n\n", filename);
> +          prev_file_displayed = i;

I'd simplify the check as "if (argc > 1)", and remove the
"prev_file_displayed" variable (not used otherwise).

> +    /* Do nothing until something happens on the disk image.  Even if
> +     * the drive changes, always wait min. 30 seconds.  For libvirt
> +     * (-d) and remote sources we cannot check this so we have to use
> +     * a fixed (5 minute) delay instead.  Also we recheck every so
> +     * often even if nothing seems to have changed.  (XXX Can we do
> +     * better?)

Should -s/--sleep-interval be available too, then?

Thanks,
-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20161011/ca647a6d/attachment.sig>