[libvirt] [PATCH] util: use string libvirt to prefix error message instead of libvir
BZ:https://bugzilla.redhat.com/show_bug.cgi?id=912021 Without error handler set, virDefaultErrorFunc will be called, the error message is prefixed with "libvir:". It become a little better by using prefix "libvirt:" when working with upper application. For example: 1, stop libvirtd daemon 2, run virt-top. libvir: XML-RPC error : Failed to connect \ socket to '/var/run/libvirt/libvirt-sock-ro': \ No such file or directory libvirt: VIR_ERR_SYSTEM_ERROR: VIR_FROM_RPC: \ Failed to connect socket to '/var/run/libvirt/libvirt-sock-ro': \ No such file or directory --- src/util/virerror.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/virerror.c b/src/util/virerror.c index 301a1ac..40c3b25 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -525,13 +525,13 @@ virDefaultErrorFunc(virErrorPtr err) len = strlen(err->message); if ((err->domain == VIR_FROM_XML) && (err->code == VIR_ERR_XML_DETAIL) && (err->int1 != 0)) - fprintf(stderr, "libvir: %s %s %s%s: line %d: %s", + fprintf(stderr, "libvirt: %s %s %s%s: line %d: %s", dom, lvl, domain, network, err->int1, err->message); else if ((len == 0) || (err->message[len - 1] != '\n')) - fprintf(stderr, "libvir: %s %s %s%s: %s\n", + fprintf(stderr, "libvirt: %s %s %s%s: %s\n", dom, lvl, domain, network, err->message); else - fprintf(stderr, "libvir: %s %s %s%s: %s", + fprintf(stderr, "libvirt: %s %s %s%s: %s", dom, lvl, domain, network, err->message); } -- 1.7.11.2
On Mon, Mar 04, 2013 at 08:54:10PM +0800, Guannan Ren wrote:
BZ:https://bugzilla.redhat.com/show_bug.cgi?id=912021 Without error handler set, virDefaultErrorFunc will be called, the error message is prefixed with "libvir:". It become a little better by using prefix "libvirt:" when working with upper application.
My concern is that making this change could break scripts that depend on the old spelling. Guannan mentioned to me that this error doesn't arise when using virsh. I don't have a strong opinion either way, but I think if there's no danger of breaking existing code that depends on the spelling then we should fix this, otherwise leave it as is. Dave
For example: 1, stop libvirtd daemon 2, run virt-top. libvir: XML-RPC error : Failed to connect \ socket to '/var/run/libvirt/libvirt-sock-ro': \ No such file or directory libvirt: VIR_ERR_SYSTEM_ERROR: VIR_FROM_RPC: \ Failed to connect socket to '/var/run/libvirt/libvirt-sock-ro': \ No such file or directory --- src/util/virerror.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/util/virerror.c b/src/util/virerror.c index 301a1ac..40c3b25 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -525,13 +525,13 @@ virDefaultErrorFunc(virErrorPtr err) len = strlen(err->message); if ((err->domain == VIR_FROM_XML) && (err->code == VIR_ERR_XML_DETAIL) && (err->int1 != 0)) - fprintf(stderr, "libvir: %s %s %s%s: line %d: %s", + fprintf(stderr, "libvirt: %s %s %s%s: line %d: %s", dom, lvl, domain, network, err->int1, err->message); else if ((len == 0) || (err->message[len - 1] != '\n')) - fprintf(stderr, "libvir: %s %s %s%s: %s\n", + fprintf(stderr, "libvirt: %s %s %s%s: %s\n", dom, lvl, domain, network, err->message); else - fprintf(stderr, "libvir: %s %s %s%s: %s", + fprintf(stderr, "libvirt: %s %s %s%s: %s", dom, lvl, domain, network, err->message); }
-- 1.7.11.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Mar 04, 2013 at 09:50:23AM -0500, Dave Allan wrote:
On Mon, Mar 04, 2013 at 08:54:10PM +0800, Guannan Ren wrote:
BZ:https://bugzilla.redhat.com/show_bug.cgi?id=912021 Without error handler set, virDefaultErrorFunc will be called, the error message is prefixed with "libvir:". It become a little better by using prefix "libvirt:" when working with upper application.
My concern is that making this change could break scripts that depend on the old spelling. Guannan mentioned to me that this error doesn't arise when using virsh. I don't have a strong opinion either way, but I think if there's no danger of breaking existing code that depends on the spelling then we should fix this, otherwise leave it as is.
The reason it doesn't hurt virsh is that it disabled the default error reporting function. It reports errors directly itself. In fact most apps end up disabling the default error reporting function & doing their own error reporting to stderr, because the default is not flexible enough. /me wishes we didn't ever have it in the first place - libraries have no business printing anything to stderr, without explicit app opt-in. I think it would be ok to rename this though. Apps relying on screen-scraping this text are doomed - we've never considered error message text to be part of the stable interface, nor even error codes - we'll often change codes to better ones over time. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 03/04/2013 10:53 PM, Daniel P. Berrange wrote:
BZ:https://bugzilla.redhat.com/show_bug.cgi?id=912021 Without error handler set, virDefaultErrorFunc will be called, the error message is prefixed with "libvir:". It become a little better by using prefix "libvirt:" when working with upper application. My concern is that making this change could break scripts that depend on the old spelling. Guannan mentioned to me that this error doesn't arise when using virsh. I don't have a strong opinion either way, but I think if there's no danger of breaking existing code that depends on
On Mon, Mar 04, 2013 at 08:54:10PM +0800, Guannan Ren wrote: the spelling then we should fix this, otherwise leave it as is. The reason it doesn't hurt virsh is that it disabled the default error reporting function. It reports errors directly itself. In fact most apps end up disabling the default error reporting function & doing their own error reporting to stderr, because
On Mon, Mar 04, 2013 at 09:50:23AM -0500, Dave Allan wrote: the default is not flexible enough.
/me wishes we didn't ever have it in the first place - libraries have no business printing anything to stderr, without explicit app opt-in.
I think it would be ok to rename this though. Apps relying on screen-scraping this text are doomed - we've never considered error message text to be part of the stable interface, nor even error codes - we'll often change codes to better ones over time.
Daniel
Thanks, pushed Guannan
participants (3)
-
Daniel P. Berrange -
Dave Allan -
Guannan Ren