[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 9/9] virNodeGetCPUMapFlags: Add python binding



On 10/16/2012 08:05 AM, Viktor Mihajlovski wrote:
> Added a method getCPUMapFlags to virConnect.

s/Flags//g

> It can be used as follows:
> 
> import libvirt
> import sys
> import os
> 
> conn = libvirt.openReadOnly(None)
> if conn == None:
>     print 'Failed to open connection to the hypervisor'
>     sys.exit(1)
> 
> try:
>     (cpus, cpumap, online) = conn.getCPUMapFlags(0)
> except:
>     print 'Failed to extract the node cpu map information'
>     sys.exit(1)
> 
> print 'CPUs total %d, online %d' % (cpus, online)
> print 'CPU map %s' % str(cpumap)
> 
> del conn
> print "OK"
> 
> sys.exit(0)
> 
> Signed-off-by: Viktor Mihajlovski <mihajlov linux vnet ibm com>
> ---
>  python/libvirt-override-api.xml |    6 ++++
>  python/libvirt-override.c       |   56 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+), 0 deletions(-)
> 
> diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml
> index b76fb4e..59ac190 100644
> --- a/python/libvirt-override-api.xml
> +++ b/python/libvirt-override-api.xml
> @@ -542,5 +542,11 @@
>        <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/>
>        <arg name='flags' type='int' info='unused, always pass 0'/>
>      </function>
> +    <function name='virNodeGetCPUMapFlags' file='python'>
> +      <info>Get node CPU information</info>
> +      <return type='str *' info='(cpunum, online, cpumap) on success, None on error'/>
> +      <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/>
> +      <arg name='flags' type='int' info='unused, pass 0'/>
> +    </function>
>    </symbols>
>  </api>
> diff --git a/python/libvirt-override.c b/python/libvirt-override.c
> index 81099b1..9d86964 100644
> --- a/python/libvirt-override.c
> +++ b/python/libvirt-override.c
> @@ -6347,6 +6347,61 @@ cleanup:
>      return ret;
>  }
>  
> +static PyObject *
> +libvirt_virNodeGetCPUMapFlags(PyObject *self ATTRIBUTE_UNUSED,
> +                              PyObject *args)
> +{
> +    virConnectPtr conn;
> +    PyObject *pyobj_conn;
> +    PyObject *ret = NULL;
> +    PyObject *pycpumap = NULL;
> +    PyObject *pyused = NULL;
> +    int i_retval;
> +    unsigned char *cpumap = NULL;
> +    unsigned int online = 0;
> +    unsigned int flags;
> +    int i;
> +
> +    if (!PyArg_ParseTuple(args, (char *)"Oi:virNodeGetCPUMapFlags",
> +                          &pyobj_conn, &flags))
> +        return NULL;
> +    conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);
> +
> +    LIBVIRT_BEGIN_ALLOW_THREADS;
> +    i_retval = virNodeGetCPUMapFlags(conn, &cpumap, &online, flags);
> +    LIBVIRT_END_ALLOW_THREADS;
> +
> +    if (i_retval < 0)
> +        goto error;

The error label returns Py_None, which is the clue for the next layer up
to parse the libvirt error - good in this case...

> +
> +    if ((ret = PyTuple_New(3)) == NULL)
> +        goto error;

...but bad here - if PyTuple_New failed, then there is a python
exception, and we should be returning NULL instead of Py_None so that
the exception is properly propagated (and since there is no libvirt
error to parse).

> +
> +    if ((pycpumap = PyList_New(i_retval)) == NULL)
> +        goto error;
> +
> +    for (i = 0; i < i_retval; i++) {
> +        if ((pyused = PyBool_FromLong(VIR_CPU_USED(cpumap, i))) == NULL)
> +            goto error;
> +        if (PyList_SetItem(pycpumap, i, pyused) < 0)
> +            goto error;
> +    }

Again, all three of these goto error points should cause the function to
return NULL, not Py_None.  Fix that by returning early on libvirt
failure (when there is nothing else to clean up), then setting ret to
NULL in the error label.

> +
> +    PyTuple_SetItem(ret, 0, PyLong_FromLong(i_retval));
> +    PyTuple_SetItem(ret, 1, pycpumap);
> +    PyTuple_SetItem(ret, 2, PyLong_FromLong(online));

Missing error checking.  PyLong_FromLong() can return NULL on OOM
errors, but PyTuple_SetItem() must not be called with a NULL argument;
furthermore, PyTuple_SetItem() must be checked for failures.

There's still a lot of work that needs to be done here to properly check
for errors, so I'll hand this one back to you to clean up.  But squash
this in to start your cleanup:

diff --git i/python/libvirt-override-api.xml
w/python/libvirt-override-api.xml
index 59ac190..e54701c 100644
--- i/python/libvirt-override-api.xml
+++ w/python/libvirt-override-api.xml
@@ -542,7 +542,7 @@
       <arg name='conn' type='virConnectPtr' info='pointer to the
hypervisor connection'/>
       <arg name='flags' type='int' info='unused, always pass 0'/>
     </function>
-    <function name='virNodeGetCPUMapFlags' file='python'>
+    <function name='virNodeGetCPUMap' file='python'>
       <info>Get node CPU information</info>
       <return type='str *' info='(cpunum, online, cpumap) on success,
None on error'/>
       <arg name='conn' type='virConnectPtr' info='pointer to the
hypervisor connection'/>
diff --git i/python/libvirt-override.c w/python/libvirt-override.c
index 7f90abf..79afc91 100644
--- i/python/libvirt-override.c
+++ w/python/libvirt-override.c
@@ -6398,8 +6398,8 @@ cleanup:
 }

 static PyObject *
-libvirt_virNodeGetCPUMapFlags(PyObject *self ATTRIBUTE_UNUSED,
-                              PyObject *args)
+libvirt_virNodeGetCPUMap(PyObject *self ATTRIBUTE_UNUSED,
+                         PyObject *args)
 {
     virConnectPtr conn;
     PyObject *pyobj_conn;
@@ -6412,17 +6412,17 @@ libvirt_virNodeGetCPUMapFlags(PyObject *self
ATTRIBUTE_UNUSED,
     unsigned int flags;
     int i;

-    if (!PyArg_ParseTuple(args, (char *)"Oi:virNodeGetCPUMapFlags",
+    if (!PyArg_ParseTuple(args, (char *)"Oi:virNodeGetCPUMap",
                           &pyobj_conn, &flags))
         return NULL;
     conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);

     LIBVIRT_BEGIN_ALLOW_THREADS;
-    i_retval = virNodeGetCPUMapFlags(conn, &cpumap, &online, flags);
+    i_retval = virNodeGetCPUMap(conn, &cpumap, &online, flags);
     LIBVIRT_END_ALLOW_THREADS;

     if (i_retval < 0)
-        goto error;
+        return VIR_PY_NONE;

     if ((ret = PyTuple_New(3)) == NULL)
         goto error;
@@ -6448,7 +6448,7 @@ error:
     Py_XDECREF(ret);
     Py_XDECREF(pycpumap);
     Py_XDECREF(pyused);
-    ret = VIR_PY_NONE;
+    ret = NULL;
     goto cleanup;
 }

@@ -6569,7 +6569,7 @@ static PyMethodDef libvirtMethods[] = {
     {(char *) "virDomainGetDiskErrors", libvirt_virDomainGetDiskErrors,
METH_VARARGS, NULL},
     {(char *) "virNodeGetMemoryParameters",
libvirt_virNodeGetMemoryParameters, METH_VARARGS, NULL},
     {(char *) "virNodeSetMemoryParameters",
libvirt_virNodeSetMemoryParameters, METH_VARARGS, NULL},
-    {(char *) "virNodeGetCPUMapFlags", libvirt_virNodeGetCPUMapFlags,
METH_VARARGS, NULL},
+    {(char *) "virNodeGetCPUMap", libvirt_virNodeGetCPUMap,
METH_VARARGS, NULL},
     {NULL, NULL, 0, NULL}
 };

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org



Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]