Re: [libvirt] [PATCH] storage: Replace storageLog with VIR_ERROR

On Thu, Feb 04, 2010 at 04:44:29PM +0100, Matthias Bolte wrote: > --- >  src/storage/storage_driver.c |   23 ++++++++++------------- >  1 files changed, 10 insertions(+), 13 deletions(-) > > diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c > index 50fcbe2..37be77d 100644 > --- a/src/storage/storage_driver.c > +++ b/src/storage/storage_driver.c > @@ -40,11 +40,10 @@ >  #include "storage_conf.h" >  #include "memory.h" >  #include "storage_backend.h" > +#include "logging.h" > >  #define VIR_FROM_THIS VIR_FROM_STORAGE > > -#define storageLog(msg...) fprintf(stderr, msg) > - >  static virStorageDriverStatePtr driverState; > >  static int storageDriverShutdown(void); > @@ -70,8 +69,7 @@ storageDriverAutostart(virStorageDriverStatePtr driver) { >              !virStoragePoolObjIsActive(pool)) { >              virStorageBackendPtr backend; >              if ((backend = virStorageBackendForType(pool->def->type)) == NULL) { > -                storageLog("Missing backend %d", > -                           pool->def->type); > +                VIR_ERROR("Missing backend %d", pool->def->type); >                  virStoragePoolObjUnlock(pool); >                  continue; >              } > @@ -79,9 +77,9 @@ storageDriverAutostart(virStorageDriverStatePtr driver) { >              if (backend->startPool && >                  backend->startPool(NULL, pool) < 0) { >                  virErrorPtr err = virGetLastError(); > -                storageLog("Failed to autostart storage pool '%s': %s", > -                           pool->def->name, err ? err->message : > -                           "no error message found"); > +                VIR_ERROR("Failed to autostart storage pool '%s': %s", > +                          pool->def->name, err ? err->message : > +                          "no error message found"); >                  virStoragePoolObjUnlock(pool); >                  continue; >              } > @@ -90,9 +88,9 @@ storageDriverAutostart(virStorageDriverStatePtr driver) { >                  virErrorPtr err = virGetLastError(); >                  if (backend->stopPool) >                      backend->stopPool(NULL, pool); > -                storageLog("Failed to autostart storage pool '%s': %s", > -                           pool->def->name, err ? err->message : > -                           "no error message found"); > +                VIR_ERROR("Failed to autostart storage pool '%s': %s", > +                          pool->def->name, err ? err->message : > +                          "no error message found"); >                  virStoragePoolObjUnlock(pool); >                  continue; >              } > @@ -132,7 +130,6 @@ storageDriverStartup(int privileged) { >              goto error; > >          if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) { > -            storageLog("out of memory in virAsprintf"); >              VIR_FREE(userdir); >              goto out_of_memory; >          } > @@ -175,7 +172,7 @@ storageDriverStartup(int privileged) { >      return 0; > >  out_of_memory: > -    storageLog("virStorageStartup: out of memory"); > +    virReportOOMError(NULL); >  error: >      VIR_FREE(base); >      storageDriverUnlock(driverState); > @@ -635,7 +632,7 @@ storagePoolUndefine(virStoragePoolPtr obj) { > >      if (unlink(pool->autostartLink) < 0 && errno != ENOENT && errno != ENOTDIR) { >          char ebuf[1024]; > -        storageLog("Failed to delete autostart link '%s': %s", > +        VIR_ERROR("Failed to delete autostart link '%s': %s", >                     pool->autostartLink, virStrerror(errno, ebuf, sizeof ebuf)); >      } >  Hum shouldn't we also provide localization for all those errors ?  VIR_ERROR[0](_(....)) instead ? actually while we are cleaning up those, under src/*/*.c we have both non-localized and localized error messages being used, I would suggest to make sure everthing gets localized ! But that can be done as a separate change, ACK to the patch as is ! Daniel