Eric Blake
2018-Nov-08 14:17 UTC
[Libguestfs] [nbdkit PATCH] log: Allow user option of appending to log
Always truncating a log can wipe out useful history. Add an
option to change this.
Signed-off-by: Eric Blake <eblake@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)
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
Richard W.M. Jones
2018-Nov-08 14:38 UTC
Re: [Libguestfs] [nbdkit PATCH] log: Allow user option of appending to log
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@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@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