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

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



On Thu, Feb 04, 2010 at 05:28:33PM +0000, Daniel P. Berrange wrote:
> On Thu, Feb 04, 2010 at 06:09:07PM +0100, Daniel Veillard wrote:
> > 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 !
> 
> None of the logging messages are intended for the end users, but rather
> for sending bug reports back to distro package maintainers and/or us
> here upstream.  In addition the messages in the logging calls changes
> quite alot and is often fairly technical text, so translators will
> have a hard time giving useful translations.  As such I believe logging
> messages should not be translated at all.

  okay but we get both kind right now :-) , not a big deal

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]