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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/