[Libvir] Python bindings, errors & exceptions
I'm really confused about how the Python bindings are supposed to handle errors. Can someone explain how this is supposed to work? Case in point: currently virt-manager fails because conn.listNetworks () returns None, whereas virt-manager is expecting it to return a list of network objects. The code returns None because the underlying calls (either virConnectNumOfNetworks or virConnectListNetworks) is failing. The functions in question are: class virConnect: # libvirtclass.py # ... def listNetworks(self): """list the networks, stores the pointers to the names in @names """ ret = libvirtmod.virConnectListNetworks(self._o) return ret (the above code is automatically generated by generator.py), and: static PyObject * // libvir.c libvirt_virConnectListNetworks(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *py_retval; char **names = NULL; int c_retval, i; virConnectPtr conn; PyObject *pyobj_conn; // ... c_retval = virConnectNumOfNetworks(conn); if (c_retval < 0) { Py_INCREF(Py_None); return (Py_None); } // ... py_retval = PyList_New(c_retval); if (names) { for (i = 0;i < c_retval;i++) { PyList_SetItem(py_retval, i, libvirt_constcharPtrWrap(names[i])); free(names[i]); } free(names); } return(py_retval); } (I've omitted some code to make the general idea clearer). The upshot is that if the underlying functions fail, at no point is an exception thrown. This is not always the case. For other functions which return C structure pointers (eg. libvirt.open which wraps virConnectOpen), the bindings automatically catch the invalid return and throw an exception. For example: def open(name): # libvirtclass.py """This function should be called first to get a connection to the Hypervisor and xen store """ ret = libvirtmod.virConnectOpen(name) if ret is None:raise libvirtError('virConnectOpen() failed') return virConnect(_obj=ret) It is my view that all errors in C code should turn into Python exceptions. One way to do that would be to have a Python virterror handler which just directly throws the exception. I don't know if this is safe because the exception would unwind through C code, and in some languages that is safe, in others it is not. Another way would be to have a Python virterror handler which remembers that an exception happened, and after each auto-generated function call we check this and raise the exception. Since I'm just starting out in Python, I don't know if there are thread or other issues with this. Comments would be welcome. Rich. -- Emerging Technologies, Red Hat http://et.redhat.com/~rjones/ 64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421 "[Negative numbers] darken the very whole doctrines of the equations and make dark of the things which are in their nature excessively obvious and simple" (Francis Maseres FRS, mathematician, 1759)
On Fri, Mar 23, 2007 at 12:12:56PM +0000, Richard W.M. Jones wrote:
I'm really confused about how the Python bindings are supposed to handle errors. Can someone explain how this is supposed to work?
Case in point: currently virt-manager fails because conn.listNetworks () returns None, whereas virt-manager is expecting it to return a list of network objects. The code returns None because the underlying calls (either virConnectNumOfNetworks or virConnectListNetworks) is failing.
The functions in question are:
class virConnect: # libvirtclass.py # ... def listNetworks(self): """list the networks, stores the pointers to the names in @names """ ret = libvirtmod.virConnectListNetworks(self._o) return ret
(the above code is automatically generated by generator.py), and:
static PyObject * // libvir.c libvirt_virConnectListNetworks(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *py_retval; char **names = NULL; int c_retval, i; virConnectPtr conn; PyObject *pyobj_conn;
// ...
c_retval = virConnectNumOfNetworks(conn); if (c_retval < 0) { Py_INCREF(Py_None); return (Py_None); }
// ...
py_retval = PyList_New(c_retval);
if (names) { for (i = 0;i < c_retval;i++) { PyList_SetItem(py_retval, i, libvirt_constcharPtrWrap(names[i])); free(names[i]); } free(names); }
return(py_retval); }
(I've omitted some code to make the general idea clearer).
The upshot is that if the underlying functions fail, at no point is an exception thrown.
Yes, returning None here is totally bogus - it should be raising a libvirtError object.
This is not always the case. For other functions which return C structure pointers (eg. libvirt.open which wraps virConnectOpen), the bindings automatically catch the invalid return and throw an exception. For example:
def open(name): # libvirtclass.py """This function should be called first to get a connection to the Hypervisor and xen store """ ret = libvirtmod.virConnectOpen(name) if ret is None:raise libvirtError('virConnectOpen() failed') return virConnect(_obj=ret)
It is my view that all errors in C code should turn into Python exceptions.
Indeed they should - all the generated C code bindings do - its just a few of these hand written bindings that are wrong.
One way to do that would be to have a Python virterror handler which just directly throws the exception. I don't know if this is safe because the exception would unwind through C code, and in some languages that is safe, in others it is not.
I can't see that being remotely safe to do in python.
Another way would be to have a Python virterror handler which remembers that an exception happened, and after each auto-generated function call we check this and raise the exception. Since I'm just starting out in Python, I don't know if there are thread or other issues with this.
That shouldn't be neccessary - the libvirtError() constructor calls the virGetLastError or virConnectGetLastError functions to retrieve the full error report details. So simply raise libvirtError("blah", conn) should do the trick. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
Daniel P. Berrange wrote:
class virConnect: # libvirtclass.py # ... def listNetworks(self): [...]
static PyObject * // libvir.c libvirt_virConnectListNetworks(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { [...] Yes, returning None here is totally bogus - it should be raising a
On Fri, Mar 23, 2007 at 12:12:56PM +0000, Richard W.M. Jones wrote: [...] libvirtError object.
Which code is wrong here? From looking at this it was my impression that listNetworks is autogenerated by generator.py, and all the code in libvir.c is hand-written, but you say:
Indeed they should - all the generated C code bindings do - its just a few of these hand written bindings that are wrong.
Rich. -- Emerging Technologies, Red Hat http://et.redhat.com/~rjones/ 64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421 "[Negative numbers] darken the very whole doctrines of the equations and make dark of the things which are in their nature excessively obvious and simple" (Francis Maseres FRS, mathematician, 1759)
On Fri, Mar 23, 2007 at 01:02:37PM +0000, Richard W.M. Jones wrote:
Daniel P. Berrange wrote:
class virConnect: # libvirtclass.py # ... def listNetworks(self): [...]
static PyObject * // libvir.c libvirt_virConnectListNetworks(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { [...] Yes, returning None here is totally bogus - it should be raising a
On Fri, Mar 23, 2007 at 12:12:56PM +0000, Richard W.M. Jones wrote: [...] libvirtError object.
Which code is wrong here? From looking at this it was my impression that listNetworks is autogenerated by generator.py, and all the code in libvir.c is hand-written, but you say:
Indeed they should - all the generated C code bindings do - its just a few of these hand written bindings that are wrong.
Actually looks like it is the generated code actually. The generator.py appears to only add in the 'raise libvirtError' stuff if the return value is a virConnectPtr/virDomainPtr/virNeworkPtr object - any other function with a non-object return value gives back None upon error. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
Daniel P. Berrange wrote:
Actually looks like it is the generated code actually. The generator.py appears to only add in the 'raise libvirtError' stuff if the return value is a virConnectPtr/virDomainPtr/virNeworkPtr object - any other function with a non-object return value gives back None upon error.
The generator.py code as it stands doesn't deal with the case where a function is prototyped (in C) as returning 'int'. If I were to modify generator.py so that it adds the check for functions returning int, then it doesn't work because there is not enough information from just the C return type to tell what the Python return type will be. As an example: save C returns int Python returns int Current bindings fail to check for ret < 0 and raise an exception. listNetworks C returns int Python returns list of networks, or None Current bindings fail to check for ret == None and raise an exception. We could do some run-time type magic in the generated wrapper, although that seems evil. I think the above are examples of the only two possible cases. Rich. -- Emerging Technologies, Red Hat http://et.redhat.com/~rjones/ 64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421 "[Negative numbers] darken the very whole doctrines of the equations and make dark of the things which are in their nature excessively obvious and simple" (Francis Maseres FRS, mathematician, 1759)
On Fri, Mar 23, 2007 at 01:28:36PM +0000, Richard W.M. Jones wrote:
Daniel P. Berrange wrote:
Actually looks like it is the generated code actually. The generator.py appears to only add in the 'raise libvirtError' stuff if the return value is a virConnectPtr/virDomainPtr/virNeworkPtr object - any other function with a non-object return value gives back None upon error.
The generator.py code as it stands doesn't deal with the case where a function is prototyped (in C) as returning 'int'.
If I were to modify generator.py so that it adds the check for functions returning int, then it doesn't work because there is not enough information from just the C return type to tell what the Python return type will be.
Right and that's why the automatic generator does not raise exception automatically. For example getMaxMem could return -1 to mean unlimited (e.g. on Domain 0) which is a correct return code in my opinion and should not be considered and error, while getMem returning -1 should really mean an error occured. No way automatic code could catch the subtle difference.
We could do some run-time type magic in the generated wrapper, although that seems evil.
At best keep a list of functions where one would not want to generate exception automatically. We could possibly drop the generator but it still has quite some value to me as long as the interfaces evolve, or even the documentation of function since the python binding modules gets the comments directly from the comments in the C code. It's easy to dismiss the generators because of it's limitations but it really lowers the maintaince and preserve accuracy over long term (at least that's my experience). 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/
Daniel Veillard wrote:
On Fri, Mar 23, 2007 at 01:28:36PM +0000, Richard W.M. Jones wrote:
Daniel P. Berrange wrote:
Actually looks like it is the generated code actually. The generator.py appears to only add in the 'raise libvirtError' stuff if the return value is a virConnectPtr/virDomainPtr/virNeworkPtr object - any other function with a non-object return value gives back None upon error. The generator.py code as it stands doesn't deal with the case where a function is prototyped (in C) as returning 'int'.
If I were to modify generator.py so that it adds the check for functions returning int, then it doesn't work because there is not enough information from just the C return type to tell what the Python return type will be.
Right and that's why the automatic generator does not raise exception automatically. For example getMaxMem could return -1 to mean unlimited (e.g. on Domain 0) which is a correct return code in my opinion and should not be considered and error, while getMem returning -1 should really mean an error occured. No way automatic code could catch the subtle difference.
getMaxMem definitely sounds like a problem. I wouldn't have spotted that on my own.
We could do some run-time type magic in the generated wrapper, although that seems evil.
At best keep a list of functions where one would not want to generate exception automatically.
Yes, I'm working on this. Thanks, Rich. -- Emerging Technologies, Red Hat http://et.redhat.com/~rjones/ 64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421 "[Negative numbers] darken the very whole doctrines of the equations and make dark of the things which are in their nature excessively obvious and simple" (Francis Maseres FRS, mathematician, 1759)
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Richard W.M. Jones