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

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?