Re: [libvirt] [PATCH v2] Allow to dynamically set the size of the debug buffer

On 03/09/2011 02:08 AM, Daniel Veillard wrote: > Allow to dynamically set the size of the debug buffer > > This is the part allowing to dynamically resize the debug log > buffer from it's default 64kB size. The buffer is now dynamically > allocated. > It adds a new API virLogSetBufferSize() which resizes the buffer > If passed a zero size, the buffer is deallocated and we do the small > optimization of not formatting messages which are not output anymore. > On the daemon side, it just adds a new option log_buffer_size to > libvirtd.conf and call virLogSetBufferSize() if needed > * src/util/logging.h src/util/logging.c src/libvirt_private.syms: > make buffer dynamic and add virLogSetBufferSize() internal API > * daemon/libvirtd.conf: document the new log_buffer_size option > * daemon/libvirtd.c: read and use the new log_buffer_size option > > diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c > index 452566c..ad3274f 100644 > --- a/daemon/libvirtd.c > +++ b/daemon/libvirtd.c > @@ -2713,18 +2713,22 @@ remoteReadSaslAllowedUsernameList (virConfPtr conf ATTRIBUTE_UNUSED, > * Set up the logging environment > * By default if daemonized all errors go to the logfile libvirtd.log, > * but if verbose or error debugging is asked for then also output > - * informations or debug. > + * informations or debug. Default size if 64 kB. s/informations or debug/informational and debug messages/ > +++ b/src/util/logging.c > @@ -36,6 +36,7 @@ > #endif > > #include "ignore-value.h" > +#include "virterror_internal.h" > #include "logging.h" > #include "memory.h" > #include "util.h" > @@ -43,6 +44,8 @@ > #include "threads.h" > #include "files.h" > > +#define VIR_FROM_THIS VIR_FROM_NONE 'make syntax-check' won't like you; you forgot to edit po/POTFILES.in.

> @@ -192,15 +197,85 @@ int virLogStartup(void) { > > virLogInitialized = 1; > virLogLock(); > + if (VIR_ALLOC_N(virLogBuffer, virLogSize) < 0) { > + /* > + * The debug buffer is not a critical component, allow startup > + * even in case of failure to allocate it in case of a > + * configuration mistake. > + */ > + virLogSize = 64000; Do we really want the non-power-of-2 size here? I guess it doesn't hurt, since the user can also give us a non-power-of-2, but sometimes enforcing the power-of-2 makes for nicer wrapping of a circular buffer.

> + if (VIR_ALLOC_N(virLogBuffer, virLogSize) < 0) { > + pbm = "Failed to allocate debug buffer: desactivating debug log\n"; s/desactivating/deactivating/ ACK with those nits fixed.