[libvirt] [PATCH] Add two NUMA tuning python bindings APIs
Alex Jia
ajia at redhat.com
Thu Jan 19 09:46:36 UTC 2012
On 01/19/2012 04:54 PM, Guan Nan Ren wrote:
>
> ----- Original Message -----
> From: "Alex Jia"<ajia at redhat.com>
> To: "Guan Nan Ren"<gren at redhat.com>
> Cc: libvir-list at 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 at redhat.com>
>>> To: libvir-list at 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'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'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'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
>
>
>
>
More information about the libvir-list
mailing list