Re: [libvirt] [PATCH 1/2] python: cleanup vcpu related binding APIs
On 04/17/2012 11:45 AM, Guannan Ren wrote:
*libvirt_virDomainGetVcpus: add error handling, return -1 instead
of None
*libvirt_virDomainPinVcpu and libvirt_virDomainPinVcpuFlags:
make use of libvirt_boolUnwrap
set vcpus which are list in tuple pycpumap, trun off the rest
in cpumap. The original way ignored the error info from PyTuple_GetItem
if pos is out of bounds. "IndexError: tuple index out of range"
The error message will only be raised on next command in interactive mode.
---
python/libvirt-override.c | 173 +++++++++++++++++++++++++++++---------------
1 files changed, 114 insertions(+), 59 deletions(-)
@@ -1388,54 +1394,80 @@ libvirt_virDomainGetVcpus(PyObject *self
ATTRIBUTE_UNUSED,
PyObject *info = PyTuple_New(4);
if (info == NULL)
goto cleanup;
- PyTuple_SetItem(info, 0, PyInt_FromLong((long)cpuinfo[i].number));
- PyTuple_SetItem(info, 1, PyInt_FromLong((long)cpuinfo[i].state));
- PyTuple_SetItem(info, 2, PyLong_FromLongLong((long long)cpuinfo[i].cpuTime));
- PyTuple_SetItem(info, 3, PyInt_FromLong((long)cpuinfo[i].cpu));
- PyList_SetItem(pycpuinfo, i, info);
+ if (PyTuple_SetItem(info, 0, PyInt_FromLong((long)cpuinfo[i].number)) < 0)
Ouch. PyInt_FromLong can return NULL (on OOM situations), but
PyTuple_SetItem() doesn't take too kindly to NULL arguments. You need
to use temporary variables through here.
for (i = 0 ; i < dominfo.nrVirtCpu ; i++) {
PyObject *info = PyTuple_New(VIR_NODEINFO_MAXCPUS(nodeinfo));
- int j;
if (info == NULL)
goto cleanup;
+
+ int j;
Why did you sink the declaration of j? We require C99, so it works, but
it's not our usual style.
for (j = 0 ; j < VIR_NODEINFO_MAXCPUS(nodeinfo) ; j++) {
- PyTuple_SetItem(info, j, PyBool_FromLong(VIR_CPU_USABLE(cpumap, cpumaplen,
i, j)));
+ if (PyTuple_SetItem(info, j, PyBool_FromLong(VIR_CPU_USABLE(cpumap,
cpumaplen, i, j))) < 0)
Again, this needs a temporary to avoid calling PyTuple_SetItem(NULL).
+
+ if ((!flag) || (libvirt_boolUnwrap(flag, &b) < 0))
Redundant parenthesis. This could be written:
if (!flag || libvirt_boolUnwrap(flag, &b) < 0)
Not quite ready, but I like the direction that your headed.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)