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

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&quot;&lt;gren(a)redhat.com&gt; > To: libvir-list(a)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'/>

> +<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'/>

> +<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.