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

Re: [libvirt] [PATCH] Add two NUMA tuning python bindings APIs



On 01/19/2012 04:54 PM, Guan Nan Ren wrote:

----- Original Message -----
From: "Alex Jia"<ajia redhat com>
To: "Guan Nan Ren"<gren redhat com>
Cc: libvir-list redhat com
Sent: Thursday, January 19, 2012 1:39:58 PM
Subject: Re: [libvirt] [PATCH] Add two NUMA tuning python bindings APIs

On 01/19/2012 11:29 AM, Alex Jia wrote:
On 01/19/2012 09:38 AM, Guan Nan Ren wrote:
    Hi

       Anybody could give a hand on reviewing this patch,
       I appreciate it.
       Chinese New Year is coming, best wishes to this community :)

    Guannan Ren
Happy New Year!
----- Original Message -----
From: "Guannan Ren"<gren redhat com>
To: libvir-list redhat com
Sent: Monday, January 16, 2012 6:58:06 PM
Subject: [libvirt] [PATCH] Add two NUMA tuning python bindings APIs

          *virDomainSetNumaParameters
          *virDomainGetNumaParameters
---
   python/libvirt-override-api.xml |   13 +++
   python/libvirt-override.c       |  186
+++++++++++++++++++++++++++++++++++++++
   2 files changed, 199 insertions(+), 0 deletions(-)

diff --git a/python/libvirt-override-api.xml
b/python/libvirt-override-api.xml
index 704fee9..e09290c 100644
--- a/python/libvirt-override-api.xml
+++ b/python/libvirt-override-api.xml
@@ -248,6 +248,19 @@
<arg name='domain' type='virDomainPtr' info='pointer to domain object'/>
<arg name='flags' type='int' info='an OR&apos;ed set of
virDomainModificationImpact'/>
</function>
+<function name='virDomainSetNumaParameters' file='python'>
+<info>Change the NUMA tunables</info>
+<return type='int' info='-1 in case of error, 0 in case of success.'/>
+<arg name='domain' type='virDomainPtr' info='pointer to domain
object'/>
+<arg name='params' type='virTypedParameterPtr' info='pointer to
memory tunable objects'/>
A copy-paste error, s/memory/numa/.
+<arg name='flags'  type='int' info='an OR&apos;ed set of
virDomainModificationImpact'/>
+</function>
+<function name='virDomainGetNumaParameters' file='python'>
+<info>Get the NUMA parameters</info>
+<return type='virTypedParameterPtr' info='the I/O tunables value or
None in case of error'/>
Save as above, s/I/O/numa/.
+<arg name='domain' type='virDomainPtr' info='pointer to domain
object'/>
+<arg name='flags' type='int' info='an OR&apos;ed set of
virDomainModificationImpact'/>
+</function>
<function name='virConnectListStoragePools' file='python'>
<info>list the storage pools, stores the pointers to the names in
@names</info>
<arg name='conn' type='virConnectPtr' info='pointer to the hypervisor
connection'/>
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index d2aad0f..27ad1d8 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -1000,6 +1000,190 @@ libvirt_virDomainGetMemoryParameters(PyObject
*self ATTRIBUTE_UNUSED,
   }

   static PyObject *
+libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED,
+                                     PyObject *args) {
+    virDomainPtr domain;
+    PyObject *pyobj_domain, *info;
+    int i_retval;
+    int nparams = 0, i;
+    unsigned int flags;
+    virTypedParameterPtr params;
+
+    if (!PyArg_ParseTuple(args,
+                          (char *)"OOi:virDomainSetNumaParameters",
+&pyobj_domain,&info,&flags))
+        return(NULL);
+    domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
+
+    LIBVIRT_BEGIN_ALLOW_THREADS;
+    i_retval = virDomainGetNumaParameters(domain, NULL,&nparams,
flags);
+    LIBVIRT_END_ALLOW_THREADS;
+
+    if (i_retval<   0)
+        return VIR_PY_INT_FAIL;
+
+    if ((params = malloc(sizeof(*params)*nparams)) == NULL)
+        return VIR_PY_INT_FAIL;
+
+    LIBVIRT_BEGIN_ALLOW_THREADS;
+    i_retval = virDomainGetNumaParameters(domain, params,&nparams,
flags);
+    LIBVIRT_END_ALLOW_THREADS;
+
+    if (i_retval<   0) {
+        free(params);
+        return VIR_PY_INT_FAIL;
+    }
+
+    /* convert to a Python tuple of long objects */
+    for (i = 0; i<   nparams; i++) {
+        PyObject *key, *val;
+        key = libvirt_constcharPtrWrap(params[i].field);
+        val = PyDict_GetItem(info, key);
+        Py_DECREF(key);
+
+        if (val == NULL)
+            continue;
+
+        switch (params[i].type) {
+        case VIR_TYPED_PARAM_INT:
+            params[i].value.i = (int)PyInt_AS_LONG(val);
+            break;
+
+        case VIR_TYPED_PARAM_UINT:
+            params[i].value.ui = (unsigned int)PyInt_AS_LONG(val);
+            break;
+
+        case VIR_TYPED_PARAM_LLONG:
+            params[i].value.l = (long long)PyLong_AsLongLong(val);
+            break;
+
+        case VIR_TYPED_PARAM_ULLONG:
+            params[i].value.ul = (unsigned long
long)PyLong_AsLongLong(val);
+            break;
+
+        case VIR_TYPED_PARAM_DOUBLE:
+            params[i].value.d = (double)PyFloat_AsDouble(val);
+            break;
+
+        case VIR_TYPED_PARAM_BOOLEAN:
+            {
+                /* Hack - Python's definition of Py_True breaks strict
+                 * aliasing rules, so can't directly compare
+                 */
+                PyObject *hacktrue = PyBool_FromLong(1);
+                params[i].value.b = hacktrue == val ? 1: 0;
+                Py_DECREF(hacktrue);
+            }
+            break;
+
+        case VIR_TYPED_PARAM_STRING:
+            params[i].value.s = PyString_AsString(val);
+            break;
+
+        default:
+            free(params);
+            return VIR_PY_INT_FAIL;
+        }
+    }
+
+    LIBVIRT_BEGIN_ALLOW_THREADS;
+    i_retval = virDomainSetNumaParameters(domain, params, nparams,
flags);
+    LIBVIRT_END_ALLOW_THREADS;
+    if (i_retval<   0) {
+        free(params);
+        return VIR_PY_INT_FAIL;
+    }
+
+    free(params);
+    return VIR_PY_INT_SUCCESS;
+}
+
+static PyObject *
+libvirt_virDomainGetNumaParameters(PyObject *self ATTRIBUTE_UNUSED,
+                                     PyObject *args) {
+    virDomainPtr domain;
+    PyObject *pyobj_domain, *info;
+    int i_retval;
+    int nparams = 0, i;
+    unsigned int flags;
+    virTypedParameterPtr params;
+
+    if (!PyArg_ParseTuple(args, (char
*)"Oi:virDomainGetNumaParameters",
+&pyobj_domain,&flags))
+        return(NULL);
+    domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
+
+    LIBVIRT_BEGIN_ALLOW_THREADS;
+    i_retval = virDomainGetNumaParameters(domain, NULL,&nparams,
flags);
+    LIBVIRT_END_ALLOW_THREADS;
+
+    if (i_retval<   0)
+        return VIR_PY_NONE;
+
+    if ((params = malloc(sizeof(*params)*nparams)) == NULL)
+        return VIR_PY_NONE;
+
+    LIBVIRT_BEGIN_ALLOW_THREADS;
+    i_retval = virDomainGetNumaParameters(domain, params,&nparams,
flags);
+    LIBVIRT_END_ALLOW_THREADS;
+
+    if (i_retval<   0) {
+        free(params);
+        return VIR_PY_NONE;
+    }
+
+    /* convert to a Python tuple of long objects */
+    if ((info = PyDict_New()) == NULL) {
+        free(params);
+        return VIR_PY_NONE;
+    }
+
+    for (i = 0 ; i<   nparams ; i++) {
+        PyObject *key, *val;
+
+        switch (params[i].type) {
+        case VIR_TYPED_PARAM_INT:
+            val = PyInt_FromLong((long)params[i].value.i);
+            break;
+
+        case VIR_TYPED_PARAM_UINT:
+            val = PyInt_FromLong((long)params[i].value.ui);
+            break;
+
+        case VIR_TYPED_PARAM_LLONG:
+            val = PyLong_FromLongLong((long long)params[i].value.l);
+            break;
+
+        case VIR_TYPED_PARAM_ULLONG:
+            val = PyLong_FromLongLong((long long)params[i].value.ul);
+            break;
+
+        case VIR_TYPED_PARAM_DOUBLE:
+            val = PyFloat_FromDouble((double)params[i].value.d);
+            break;
+
+        case VIR_TYPED_PARAM_BOOLEAN:
+            val = PyBool_FromLong((long)params[i].value.b);
+            break;
+
+        case VIR_TYPED_PARAM_STRING:
+            val = libvirt_constcharPtrWrap(params[i].value.s);
The above style isn't consistent with previous codes, the
libvirt_constcharPtrWrap is
a wrapper function, it will be better to uniform them.

In addition, the function libvirt_constcharPtrWrap will call
PyString_FromString() to
return a new reference of a string, but the original string memory
hasn't been released,
so original function/caller need to free it, moreover, the
libvirt_constcharPtrWrap is
called in a loop, so you also need to free it in a loop and 'default'
branch.
    Hi Alex

      The call to PyString_FromString() will cause Python memory manager
    to allocate a new string object in Python's private heap rather than
    incrementing a reference to the string in system heap.
      Meanwhile, the function returns the ownership of a new reference(probably
    the first one) to the string object to "val".
      According to the Memory Management doc,"PyDict_SetItem() and friends
    don’t take over ownership" that means It doesn't increment the reference
    to the string object.
I haven't found a certain document in python.org about whether PyDict_SetItem() borrows/steals a references, please paste a link in here if you find it, it will be helpful
for others, thanks.

In addition, you can find a exact answer in dictobject.c, the function PyDict_SetItem()
indeed increases reference count for key and value:

http://svn.python.org/projects/python/trunk/Objects/dictobject.c

Moreover, I saw the wrapper function libvirt_constcharPtrWrap() also increase reference
count.
      So, The object reference returned from the C function that is called from Python
    is returned to the Python program with only one reference. as for the string
    in the system heap, it is release by "free(params)" above the last return.

You should note it's a loop! so you should free memory in loop like this:
<snip>
for(i=0; i < nparams; i++)
    if (params[i].type == VIR_TYPED_PARAM_STRING)
        free(params[i].value.s);

free(params);
</snip>

Meanwhile, also do it in 'default' branch.

The following are valgrind detect report:

==13741== 43 bytes in 1 blocks are definitely lost in loss record 500 of 2,087
==13741==    at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==13741==    by 0x39E1A85EC3: PyObject_Malloc (obmalloc.c:935)
==13741==    by 0x39E1A9053C: PyString_FromString (stringobject.c:138)
==13741== by 0xB8F8F70: libvirt_virDomainGetNumaParameters (libvirt-override.c:1170)
==13741==    by 0x39E1ADE7F3: PyEval_EvalFrameEx (ceval.c:3794)
==13741==    by 0x39E1ADF99E: PyEval_EvalFrameEx (ceval.c:3880)
==13741==    by 0x39E1AE0466: PyEval_EvalCodeEx (ceval.c:3044)
==13741==    by 0x39E1AE0541: PyEval_EvalCode (ceval.c:545)
==13741==    by 0x39E1AFB88B: run_mod (pythonrun.c:1351)
==13741==    by 0x39E1AFB95F: PyRun_FileExFlags (pythonrun.c:1337)
==13741==    by 0x39E1AFCE4B: PyRun_SimpleFileExFlags (pythonrun.c:941)
==13741==    by 0x39E1B094CE: Py_Main (main.c:577)
==13741==
==13741== 101 bytes in 2 blocks are definitely lost in loss record 1,448 of 2,087
==13741==    at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==13741==    by 0x39E1A85EC3: PyObject_Malloc (obmalloc.c:935)
==13741==    by 0x39E1A9053C: PyString_FromString (stringobject.c:138)
==13741== by 0xB8F8EEC: libvirt_virDomainGetNumaParameters (libvirt-override.c:1179)
==13741==    by 0x39E1ADE7F3: PyEval_EvalFrameEx (ceval.c:3794)
==13741==    by 0x39E1ADF99E: PyEval_EvalFrameEx (ceval.c:3880)
==13741==    by 0x39E1AE0466: PyEval_EvalCodeEx (ceval.c:3044)
==13741==    by 0x39E1AE0541: PyEval_EvalCode (ceval.c:545)
==13741==    by 0x39E1AFB88B: run_mod (pythonrun.c:1351)
==13741==    by 0x39E1AFB95F: PyRun_FileExFlags (pythonrun.c:1337)
==13741==    by 0x39E1AFCE4B: PyRun_SimpleFileExFlags (pythonrun.c:941)
==13741==    by 0x39E1B094CE: Py_Main (main.c:577)

    Guannan Ren






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