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