[libvirt] PATCH: Improve python exception messages

Most of the libvirt python API bindings use code snippet like this when raising an exception: if ret is None:raise libvirtError('virConnectOpen() failed') THis sets the message associated with the exception to "virConnectOpen() failed" This contains essentially zero useful information - you can see that it was virConnectOpen which failed from the stack trace. Now the libvirt error object has a real message, such as "authentication failed" Or "unable to connect to '/var/run/libvirt/libvirt-sock': Connection refused" This patch makes sure we extract this real error message and use it to set the message associated with the exception object. This is one step in getting better error reporting for virt-install/virt-manager, which is particularly needed for remote connections Daniel diff -u -r1.9 libvir.py --- python/libvir.py 11 Jun 2008 07:49:01 -0000 1.9 +++ python/libvir.py 18 Aug 2008 12:07:03 -0000 @@ -13,10 +13,22 @@ import types +def _getErrorMessage(conn, defmsg): + err = None + if conn is None: + err = virGetLastError() + else: + err = conn.virConnGetLastError() + if err is None: + return defmsg + else: + return err[2] + + # The root of all libvirt errors. class libvirtError(Exception): def __init__(self, msg, conn=None, dom=None, net=None, pool=None, vol=None): - Exception.__init__(self, msg) + Exception.__init__(self, _getErrorMessage(conn, msg)) if dom is not None: conn = dom._conn @@ -77,12 +89,6 @@ return None return self.err[8] - def __str__(self): - if self.get_error_message() is None: - return Exception.__str__(self) - else: - return Exception.__str__(self) + " " + self.get_error_message() - # # register the libvirt global error handler # -- |: 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 Tue, Aug 19, 2008 at 10:31:34AM +0100, Daniel P. Berrange wrote:
Most of the libvirt python API bindings use code snippet like this when raising an exception:
if ret is None:raise libvirtError('virConnectOpen() failed')
THis sets the message associated with the exception to
"virConnectOpen() failed"
This contains essentially zero useful information - you can see that it was virConnectOpen which failed from the stack trace.
Now the libvirt error object has a real message, such as
"authentication failed"
Or
"unable to connect to '/var/run/libvirt/libvirt-sock': Connection refused"
This patch makes sure we extract this real error message and use it to set the message associated with the exception object. This is one step in getting better error reporting for virt-install/virt-manager, which is particularly needed for remote connections
Good idea, +1, 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 Tue, Aug 19, 2008 at 10:31:34AM +0100, Daniel P. Berrange wrote:
Most of the libvirt python API bindings use code snippet like this when raising an exception:
if ret is None:raise libvirtError('virConnectOpen() failed')
THis sets the message associated with the exception to
"virConnectOpen() failed"
This contains essentially zero useful information - you can see that it was virConnectOpen which failed from the stack trace.
Now the libvirt error object has a real message, such as
"authentication failed"
Or
"unable to connect to '/var/run/libvirt/libvirt-sock': Connection refused"
This patch makes sure we extract this real error message and use it to set the message associated with the exception object. This is one step in getting better error reporting for virt-install/virt-manager, which is particularly needed for remote connections
This patch was flawed - it missed out the error message info when passed in a domain/network/volume/pool instead of a connection object. Here is an updated patch which addresses that Daniel Index: python/libvir.py =================================================================== RCS file: /data/cvs/libvirt/python/libvir.py,v retrieving revision 1.9 diff -u -r1.9 libvir.py --- python/libvir.py 11 Jun 2008 07:49:01 -0000 1.9 +++ python/libvir.py 21 Aug 2008 10:58:53 -0000 @@ -15,8 +15,7 @@ # The root of all libvirt errors. class libvirtError(Exception): - def __init__(self, msg, conn=None, dom=None, net=None, pool=None, vol=None): - Exception.__init__(self, msg) + def __init__(self, defmsg, conn=None, dom=None, net=None, pool=None, vol=None): if dom is not None: conn = dom._conn @@ -28,9 +27,17 @@ conn = vol._conn if conn is None: - self.err = virGetLastError() + err = virGetLastError() + else: + err = conn.virConnGetLastError() + if err is None: + msg = defmsg else: - self.err = conn.virConnGetLastError() + msg = err[2] + + Exception.__init__(self, msg) + + self.err = err def get_error_code(self): if self.err is None: @@ -77,12 +84,6 @@ return None return self.err[8] - def __str__(self): - if self.get_error_message() is None: - return Exception.__str__(self) - else: - return Exception.__str__(self) + " " + self.get_error_message() - # # register the libvirt global error handler # -- |: 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, Aug 21, 2008 at 12:00:46PM +0100, Daniel P. Berrange wrote:
On Tue, Aug 19, 2008 at 10:31:34AM +0100, Daniel P. Berrange wrote:
This patch makes sure we extract this real error message and use it to set the message associated with the exception object. This is one step in getting better error reporting for virt-install/virt-manager, which is particularly needed for remote connections
This patch was flawed - it missed out the error message info when passed in a domain/network/volume/pool instead of a connection object. Here is an updated patch which addresses that
okay, +1, hopefully that time my mail will make up to the list, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

On Thu, Aug 21, 2008 at 12:00:46PM +0100, Daniel P. Berrange wrote:
This patch was flawed - it missed out the error message info when passed in a domain/network/volume/pool instead of a connection object. Here is an updated patch which addresses that
+1. I added the original '.. failed' messages. Before that there was no exception on failure at all :-) Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v

On Fri, Aug 22, 2008 at 09:16:57AM +0100, Richard W.M. Jones wrote:
On Thu, Aug 21, 2008 at 12:00:46PM +0100, Daniel P. Berrange wrote:
This patch was flawed - it missed out the error message info when passed in a domain/network/volume/pool instead of a connection object. Here is an updated patch which addresses that
+1.
I added the original '.. failed' messages. Before that there was no exception on failure at all :-)
I think already sent a +1 on the updated patch, but can't find it anymore, I agree 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/
participants (3)
-
Daniel P. Berrange
-
Daniel Veillard
-
Richard W.M. Jones