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

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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