[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] cleanup of direct stderr logging



On Wed, May 20, 2009 at 12:40:36PM +0100, Daniel P. Berrange wrote:
> On Tue, May 19, 2009 at 05:40:15PM +0200, Daniel Veillard wrote:
> >   In a number of places we still had direct output of logs, warnings or
> > errors, instead of using the existing error and logging infrastructure.
> > This patches tries to clean this up,
> 
> It all looks good side from  a couple bits which I think can be
> simplified still further...

[...]

> >      pool->configFile = strdup(path);
> >      if (pool->configFile == NULL) {
> > -        virStorageLog("Failed to load storage pool config '%s': out of memory", path);
> > +        virStorageError(conn, VIR_ERR_INTERNAL_ERROR,
> > +            "Failed to load storage pool config '%s': out of memory", path);
> >          virStoragePoolDefFree(def);
> >          return NULL;
> >      }
> >      pool->autostartLink = strdup(autostartLink);
> >      if (pool->autostartLink == NULL) {
> > -        virStorageLog("Failed to load storage pool config '%s': out of memory", path);
> > +        virStorageError(conn, VIR_ERR_INTERNAL_ERROR,
> > +            "Failed to load storage pool config '%s': out of memory", path);
> >          virStoragePoolDefFree(def);
> >          return NULL;
> >      }
> 
> These two should just be virReportOOM()  calls.

  I had kept them because they gave more contextual informations, but
  basically if there isn't enough memory, it's not worth bothering
  really.

So I made all the changes and commited, thanks !

One thing we noticed is that if someone disable debug with
--disable-debug then all errors/warnings/info which are reported
directly via VIR_ERROR/VIR_WARN/VIR_INFO disapear as the logging
code is not compiled in. We need to change this, logging code should
not be made optional and --disable-debug should just redefine the
VIR_DEBUG* macros to noop, but the rest of the logging infrastructure
should still be activated.
This will have to be addressed in a separate patch,

Daniel

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]