[Libguestfs] [nbdkit PATCH] log: Allow user option of appending to log

[Richard W.M. Jones]

On Thu, Nov 08, 2018 at 08:17:05AM -0600, Eric Blake wrote:
> Always truncating a log can wipe out useful history. Add an
> option to change this.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
> 
> Hmm - logappend=false enables appending. Maybe we need a common
> utility function for parsing bool values (parse 0 and case-insensitive
> "off" or "false" as off, all others as true, or even report values
> that can't be parsed reasonably)

You can copy and relicense this function if you need it:

https://github.com/libguestfs/libguestfs/blob/dd665e4bbbbb1b298a9e8046d6c4123441fb9cc0/common/utils/utils.c#L345

I wrote this, although it's based on the (documentation of) the
equivalent Tcl function.  It would be good to have something like this
in core nbdkit, analogous to ‘nbdkit_parse_size’.

I looked at other plugins and filters, but I couldn't see any other
cases where we would use it.  src/main.c does some boolean-style
parsing for the ‘--tls’ option, but the function above is not
immediately appropriate there.

The rest of the patch is fine, so ACK with a suitable change to the
boolean evaluation.

Rich.

>  filters/log/nbdkit-log-filter.pod | 5 +++--
>  filters/log/log.c                 | 8 +++++++-
>  tests/test-log.sh                 | 6 +++++-
>  3 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/filters/log/nbdkit-log-filter.pod b/filters/log/nbdkit-log-filter.pod
> index 0903329..88b9f9b 100644
> --- a/filters/log/nbdkit-log-filter.pod
> +++ b/filters/log/nbdkit-log-filter.pod
> @@ -4,7 +4,7 @@ nbdkit-log-filter - nbdkit log filter
> 
>  =head1 SYNOPSIS
> 
> - nbdkit --filter=log plugin logfile=FILE [plugin-args...]
> + nbdkit --filter=log plugin logfile=FILE [logappend=VAL] [plugin-args...]
> 
>  =head1 DESCRIPTION
> 
> @@ -22,7 +22,8 @@ work even when nbdkit is run as a daemon.
> 
>  The nbdkit-log-filter requires a single parameter C<logfile> which
>  specifies the path of the file to use for logging.  If the file
> -already exists, it will be truncated.
> +already exists, it will be truncated unless the C<logappend> parameter
> +was specified with any non-empty value other than "0".
> 
>  =head1 EXAMPLES
> 
> diff --git a/filters/log/log.c b/filters/log/log.c
> index 2079084..e6ed3b4 100644
> --- a/filters/log/log.c
> +++ b/filters/log/log.c
> @@ -43,6 +43,7 @@
>  #include <pthread.h>
>  #include <sys/time.h>
>  #include <assert.h>
> +#include <stdbool.h>
> 
>  #include <nbdkit-filter.h>
> 
> @@ -51,6 +52,7 @@
>  static uint64_t connections;
>  static char *logfilename;
>  static FILE *logfile;
> +static bool append;
>  static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
> 
>  static void
> @@ -75,6 +77,10 @@ log_config (nbdkit_next_config *next, void *nxdata,
>      }
>      return 0;
>    }
> +  if (strcmp (key, "logappend") == 0) {
> +    append = value[0] && (value[0] != '0' || value[1]);
> +    return 0;
> +  }
>    return next (nxdata, key, value);
>  }
> 
> @@ -86,7 +92,7 @@ log_config_complete (nbdkit_next_config_complete *next, void *nxdata)
>      nbdkit_error ("missing logfile= parameter for the log filter");
>      return -1;
>    }
> -  logfile = fopen (logfilename, "w");
> +  logfile = fopen (logfilename, append ? "a" : "w");
>    if (!logfile) {
>      nbdkit_error ("fopen: %m");
>      return -1;
> diff --git a/tests/test-log.sh b/tests/test-log.sh
> index f0eacb7..b30d20e 100755
> --- a/tests/test-log.sh
> +++ b/tests/test-log.sh
> @@ -45,7 +45,9 @@ if ! qemu-io -f raw -c 'w 1M 2M' log.img; then
>  fi
> 
>  # Run nbdkit with logging enabled to file.
> -start_nbdkit -P log.pid -U log.sock --filter=log file log.img logfile=log.log
> +echo '# My log' > log.log
> +start_nbdkit -P log.pid -U log.sock --filter=log file log.img \
> +	     logfile=log.log logappend=1
> 
>  # For easier debugging, dump the final log files before removing them
>  # on exit.
> @@ -61,6 +63,8 @@ cleanup_fn cleanup
>  qemu-io -f raw -c 'w -P 11 1M 2M' 'nbd+unix://?socket=log.sock'
>  qemu-io -r -f raw -c 'r -P 11 2M 1M' 'nbd+unix://?socket=log.sock'
> 
> +# The log should have been appended, preserving our marker.
> +grep '# My log' log.log
>  # The log should show a write on connection 1, and read on connection 2.
>  grep 'connection=1 Write id=1 offset=0x100000 count=0x200000 ' log.log
>  grep 'connection=2 Read id=1 offset=0x200000 count=0x100000 ' log.log
> -- 
> 2.17.2
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top