2:18 p.m.
Hi all,
Looking at python/libvir.c, all the custom functions return VIR_PY_NONE
on error. This unfortunately doesn't map well to some of the
generated python bindings which expect an error val of -1. So if these
commands fail, no exception will be thrown at the python level.
(Ex. virDomainGetAutostart, virDomainGetVcpus and their python equiv.)
I'm wondering where the fix should be: in python/libvir.c, changing
these error return values, or in the generator somewhere?
Thanks,
Cole
2:24 p.m.
On Wed, Feb 06, 2008 at 03:18:13PM -0500, Cole Robinson wrote:
Hi all,
Looking at python/libvir.c, all the custom functions return VIR_PY_NONE
on error. This unfortunately doesn't map well to some of the
generated python bindings which expect an error val of -1. So if these
commands fail, no exception will be thrown at the python level.
(Ex. virDomainGetAutostart, virDomainGetVcpus and their python equiv.)
I'm wondering where the fix should be: in python/libvir.c, changing
these error return values, or in the generator somewhere?
The fix should be in the libvir.c file - I didn't write this custom
bindings correctly. Simply remove the line
if (c_retval < 0)
return VIR_PY_NONE;
So 'c_retval' will get propagated back to the pure python layer which
will do the -1 check
Regards,
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 -=|
4:35 p.m.
Daniel P. Berrange wrote:
On Wed, Feb 06, 2008 at 03:18:13PM -0500, Cole Robinson wrote:
> Hi all,
>
> Looking at python/libvir.c, all the custom functions return VIR_PY_NONE
> on error. This unfortunately doesn't map well to some of the
> generated python bindings which expect an error val of -1. So if these
> commands fail, no exception will be thrown at the python level.
> (Ex. virDomainGetAutostart, virDomainGetVcpus and their python equiv.)
>
> I'm wondering where the fix should be: in python/libvir.c, changing
> these error return values, or in the generator somewhere?
The fix should be in the libvir.c file - I didn't write this custom
bindings correctly. Simply remove the line
if (c_retval < 0)
return VIR_PY_NONE;
So 'c_retval' will get propagated back to the pure python layer which
will do the -1 check
Regards,
Dan.
Hmm, I don't think any of the cases where there is an error actually
end up returning c_retval, its always python wrapped contents of
something passed by reference, so I don't know how that would work.
The patch below seems to do the job: I tested it with the autostart
function on a xen domain (which errors as its not supported) and it
now correctly throws an exception from the python bindings.
Thanks,
Cole
diff --git a/python/libvir.c b/python/libvir.c
index 96555b3..67d6252 100644
--- a/python/libvir.c
+++ b/python/libvir.c
@@ -27,6 +27,8 @@ extern void initcygvirtmod(void);
is so common that we encapsulate it here. Now, each use is simply
return VIR_PY_NONE; */
#define VIR_PY_NONE (Py_INCREF (Py_None), Py_None)
+#define VIR_PY_INT_FAIL (libvirt_intWrap(-1))
+#define VIR_PY_INT_SUCCESS (libvirt_intWrap(0))
/************************************************************************
* *
@@ -214,15 +216,15 @@ libvirt_virDomainSetSchedulerParameters(PyObject *self
ATTRIBUTE_UNUSED,
c_retval = virDomainGetSchedulerType(domain, &nparams);
if (c_retval == NULL)
- return VIR_PY_NONE;
+ return VIR_PY_INT_FAIL;
free(c_retval);
if ((params = malloc(sizeof(*params)*nparams)) == NULL)
- return VIR_PY_NONE;
+ return VIR_PY_INT_FAIL;
if (virDomainGetSchedulerParameters(domain, params, &nparams) < 0) {
free(params);
- return VIR_PY_NONE;
+ return VIR_PY_INT_FAIL;
}
/* convert to a Python tupple of long objects */
@@ -269,17 +271,17 @@ libvirt_virDomainSetSchedulerParameters(PyObject *self
ATTRIBUTE_UNUSED,
default:
free(params);
- return VIR_PY_NONE;
+ return VIR_PY_INT_FAIL;
}
}
if (virDomainSetSchedulerParameters(domain, params, nparams) < 0) {
free(params);
- return VIR_PY_NONE;
+ return VIR_PY_INT_FAIL;
}
free(params);
- return VIR_PY_NONE;
+ return VIR_PY_INT_SUCCESS;
}
static PyObject *
@@ -383,11 +385,11 @@ libvirt_virDomainPinVcpu(PyObject *self ATTRIBUTE_UNUSED,
domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
if (virNodeGetInfo(virDomainGetConnect(domain), &nodeinfo) != 0)
- return VIR_PY_NONE;
+ return VIR_PY_INT_FAIL;
cpumaplen = VIR_CPU_MAPLEN(VIR_NODEINFO_MAXCPUS(nodeinfo));
if ((cpumap = malloc(cpumaplen)) == NULL)
- return VIR_PY_NONE;
+ return VIR_PY_INT_FAIL;
memset(cpumap, 0, cpumaplen);
truth = PyBool_FromLong(1);
@@ -403,7 +405,7 @@ libvirt_virDomainPinVcpu(PyObject *self ATTRIBUTE_UNUSED,
Py_DECREF(truth);
free(cpumap);
- return VIR_PY_NONE;
+ return VIR_PY_INT_SUCCESS;
}
@@ -1030,7 +1032,7 @@ libvirt_virDomainGetAutostart(PyObject *self ATTRIBUTE_UNUSED,
PyObject *args) {
LIBVIRT_END_ALLOW_THREADS;
if (c_retval < 0)
- return VIR_PY_NONE;
+ return VIR_PY_INT_FAIL;
py_retval = libvirt_intWrap(autostart);
return(py_retval);
}
@@ -1053,7 +1055,7 @@ libvirt_virNetworkGetAutostart(PyObject *self ATTRIBUTE_UNUSED,
PyObject *args)
LIBVIRT_END_ALLOW_THREADS;
if (c_retval < 0)
- return VIR_PY_NONE;
+ return VIR_PY_INT_FAIL;
py_retval = libvirt_intWrap(autostart);
return(py_retval);
}
3:49 a.m.
On Wed, Feb 06, 2008 at 05:35:09PM -0500, Cole Robinson wrote:
Daniel P. Berrange wrote:
> On Wed, Feb 06, 2008 at 03:18:13PM -0500, Cole Robinson wrote:
>> Hi all,
>>
>> Looking at python/libvir.c, all the custom functions return VIR_PY_NONE
>> on error. This unfortunately doesn't map well to some of the
>> generated python bindings which expect an error val of -1. So if these
>> commands fail, no exception will be thrown at the python level.
>> (Ex. virDomainGetAutostart, virDomainGetVcpus and their python equiv.)
>>
>> I'm wondering where the fix should be: in python/libvir.c, changing
>> these error return values, or in the generator somewhere?
>
> The fix should be in the libvir.c file - I didn't write this custom
> bindings correctly. Simply remove the line
>
> if (c_retval < 0)
> return VIR_PY_NONE;
>
> So 'c_retval' will get propagated back to the pure python layer which
> will do the -1 check
>
> Regards,
> Dan.
Hmm, I don't think any of the cases where there is an error actually
end up returning c_retval, its always python wrapped contents of
something passed by reference, so I don't know how that would work.
The patch below seems to do the job: I tested it with the autostart
function on a xen domain (which errors as its not supported) and it
now correctly throws an exception from the python bindings.
Okay, this is similar to changing a void public API to an int returning
one, it makes sense, and should be no problem, patch is very clean, so
great ! I applied it and commited it to CVS,
thanks !
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
6133
days inactive
6134
days old
3 comments
3 participants
participants (3)
-
Cole Robinson -
Daniel P. Berrange -
Daniel Veillard