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

[Alex Jia <ajia@xxxxxxxxxx>]

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@xxxxxxxxxx>
> > To: libvir-list@xxxxxxxxxx
> > Sent: Monday, January 16, 2012 6:58:06 PM
> > Subject:  [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.
> > +            break;
> > +
> > +        default:
> > +            free(params);
> > +            Py_DECREF(info);
> > +            return VIR_PY_NONE;
> > +        }
> > +
> > +        key = libvirt_constcharPtrWrap(params[i].field);
> Memory leaks again.
> > +        PyDict_SetItem(info, key, val);
> > +    }
> > +    free(params);
> > +    return(info);
> > +}
> > +
> > +static PyObject *
> >   libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED,
> >                             PyObject *args) {
> >       virDomainPtr domain;
> > @@ -5162,6 +5346,8 @@ static PyMethodDef libvirtMethods[] = {
> >       {(char *) "virDomainGetBlkioParameters", 
> > libvirt_virDomainGetBlkioParameters, METH_VARARGS, NULL},
> >       {(char *) "virDomainSetMemoryParameters", 
> > libvirt_virDomainSetMemoryParameters, METH_VARARGS, NULL},
> >       {(char *) "virDomainGetMemoryParameters", 
> > libvirt_virDomainGetMemoryParameters, METH_VARARGS, NULL},
> > +    {(char *) "virDomainGetNumaParameters", 
> > libvirt_virDomainGetNumaParameters, METH_VARARGS, NULL},
> > +    {(char *) "virDomainSetNumaParameters", 
> > libvirt_virDomainSetNumaParameters, METH_VARARGS, NULL},
> >       {(char *) "virDomainGetVcpus", libvirt_virDomainGetVcpus, 
> > METH_VARARGS, NULL},
> >       {(char *) "virDomainPinVcpu", libvirt_virDomainPinVcpu, 
> > METH_VARARGS, NULL},
> >       {(char *) "virDomainPinVcpuFlags", 
> > libvirt_virDomainPinVcpuFlags, METH_VARARGS, NULL},
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list