[libvirt] [PATCH] storage: Replace storageLog with VIR_ERROR
--- 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)); } -- 1.6.3.3
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/
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 :|
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/
2010/2/4 Daniel Veillard <veillard@redhat.com>:
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
As Dan said in his response, logging messages shouldn't be localized, IMHO. Thanks, pushed. Matthias
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Matthias Bolte