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

[Daniel Veillard]

On Mon, Mar 14, 2011 at 09:34:12PM -0600, Eric Blake wrote:
> 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.

  Hum, no it passes, really !
I believe it checks only for translatable strings (that's normal) but
I didn't add any :-)

> > @@ -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.

Well we need to allocate size +1 anyway, but yeah maybe we can shave a
couple of nanoseconds on some debug message :-)

> 
> > +        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.

  Okay, will do except for the po since it's okay here.

   Thanks !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/