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

-- 
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]