[libvirt] [PATCH 1/2 v3] Python: Refactoring virTypedParameter conversion for NUMA tuning APIs

*virDomainSetNumaParameters *virDomainGetNumaParameters --- python/Makefile.am | 4 +- python/libvirt-override-api.xml | 13 ++ python/libvirt-override.c | 314 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 330 insertions(+), 1 deletions(-) diff --git a/python/Makefile.am b/python/Makefile.am index 3068eee..4302fa5 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -8,6 +8,8 @@ SUBDIRS= . tests INCLUDES = \ $(PYTHON_INCLUDES) \ -I$(top_srcdir)/include \ + -I$(top_srcdir)/src \ + -I$(top_srcdir)/gnulib/lib \ -I$(top_builddir)/include \ -I$(top_builddir)/$(subdir) \ $(GETTEXT_CPPFLAGS) @@ -42,7 +44,7 @@ all-local: libvirt.py libvirt_qemu.py pyexec_LTLIBRARIES = libvirtmod.la libvirtmod_qemu.la -libvirtmod_la_SOURCES = libvirt-override.c typewrappers.c +libvirtmod_la_SOURCES = libvirt-override.c typewrappers.c ../src/util/virtypedparam.c nodist_libvirtmod_la_SOURCES = libvirt.c libvirt.h # Python <= 2.4 header files contain a redundant decl, hence we # need extra flags here diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 704fee9..748aa17 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 numa tunable objects'/> + <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='int' info='returns a dictionary of params in case of success, -1 in case of error'/> + <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..5f9d83e 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -21,6 +21,7 @@ #include "libvirt/virterror.h" #include "typewrappers.h" #include "libvirt.h" +#include "util/virtypedparam.h" #ifndef __CYGWIN__ extern void initlibvirtmod(void); @@ -61,6 +62,208 @@ static char *py_str(PyObject *obj) return PyString_AsString(str); } +/* Two helper functions to help the conversions between C to Python + * for the virTypedParameter used in the following APIs. */ +static PyObject * +getPyVirTypedParameter(virTypedParameterPtr params, int nparams) +{ + PyObject *info; + PyObject *key, *val; + PyObject *ret = NULL; + int i; + + if (!params) + return ret; + + /* convert to a Python tuple of long objects */ + if ((info = PyDict_New()) == NULL) { + return ret; + } + + for (i = 0 ; i < nparams ; i++) { + 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((unsigned 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_FromUnsignedLongLong((unsigned 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); + break; + + default: + Py_DECREF(info); + return ret; + } + + key = libvirt_constcharPtrWrap(params[i].field); + if (!key || !val) + goto fail; + + if (PyDict_SetItem(info, key, val) < 0) + goto fail; + + Py_DECREF(key); + Py_DECREF(val); + } + return info; +fail: + Py_XDECREF(info); + Py_XDECREF(key); + Py_XDECREF(val); + return ret; +} + +static PyObject * +setPyVirTypedParameter(PyObject *info, virTypedParameterPtr params, int nparams) +{ + PyObject *key, *val; + PyObject *ret = NULL; + int i; + + if (!info || !params) + return ret; + + /* convert to a Python tuple of long objects */ + for (i = 0; i < nparams; i++) { + 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: + { + long long_val; + if (PyInt_Check(val)) { + long_val = PyInt_AsLong(val); + if ((long_val == -1) && PyErr_Occurred()) + return ret; + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be int"); + return ret; + } + params[i].value.i = (int)long_val; + } + break; + case VIR_TYPED_PARAM_UINT: + { + long long_val; + if (PyInt_Check(val)) { + long_val = PyInt_AsLong(val); + if ((long_val == -1) && PyErr_Occurred()) + return ret; + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be int"); + return ret; + } + params[i].value.ui = (unsigned int)long_val; + } + break; + case VIR_TYPED_PARAM_LLONG: + { + long long llong_val; + if (PyLong_Check(val)) { + llong_val = PyLong_AsLongLong(val); + if ((llong_val == -1) && PyErr_Occurred()) + return ret; + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be long"); + return ret; + } + params[i].value.l = llong_val; + } + break; + case VIR_TYPED_PARAM_ULLONG: + { + unsigned long long ullong_val; + if (PyLong_Check(val)) { + ullong_val = PyLong_AsUnsignedLongLong(val); + if ((ullong_val == -1) && PyErr_Occurred()) + return ret; + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be long"); + return ret; + + } + params[i].value.ul = ullong_val; + } + break; + case VIR_TYPED_PARAM_DOUBLE: + { + double double_val; + if (PyFloat_Check(val)) { + double_val = PyFloat_AsDouble(val); + if ((double_val == -1) && PyErr_Occurred()) + return ret; + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be float"); + return ret; + } + params[i].value.d = double_val; + } + break; + case VIR_TYPED_PARAM_BOOLEAN: + { + /* Hack - Python's definition of Py_True breaks strict + * aliasing rules, so can't directly compare + */ + if (PyBool_Check(val)) { + PyObject *hacktrue = PyBool_FromLong(1); + params[i].value.b = hacktrue == val ? 1: 0; + Py_DECREF(hacktrue); + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be bool"); + return ret; + } + } + break; + case VIR_TYPED_PARAM_STRING: + { + if (PyString_Check(val)) { + free(params[i].value.s); + params[i].value.s = PyString_AsString(val); + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be string"); + return ret; + } + } + break; + default: + return ret; + } + } + return VIR_PY_NONE; +} + /************************************************************************ * * * Statistics * @@ -1000,6 +1203,115 @@ libvirt_virDomainGetMemoryParameters(PyObject *self ATTRIBUTE_UNUSED, } static PyObject * +libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + virDomainPtr domain; + PyObject *pyobj_domain, *info; + PyObject *ret = NULL; + int i_retval; + int nparams = 0; + 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 PyErr_NoMemory(); + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainGetNumaParameters(domain, params, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) { + ret = VIR_PY_INT_FAIL; + goto fail; + } + + if (!setPyVirTypedParameter(info, params, nparams)) + goto fail; + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainSetNumaParameters(domain, params, nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) { + ret = VIR_PY_INT_FAIL; + goto fail; + } + + /* The string generated by PyString_AsString + * must not be deallocated */ + free(params); + return VIR_PY_INT_SUCCESS; +fail: + /*same as above*/ + free(params); + return ret; +} + +static PyObject * +libvirt_virDomainGetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + virDomainPtr domain; + PyObject *pyobj_domain, *info; + PyObject *ret = NULL; + int i_retval; + int nparams = 0; + 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_INT_FAIL; + + if ((params = malloc(sizeof(*params)*nparams)) == NULL) + return PyErr_NoMemory(); + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainGetNumaParameters(domain, params, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) { + ret = VIR_PY_INT_FAIL; + goto fail; + } + + info = getPyVirTypedParameter(params, nparams); + if (!info) + goto fail; + + virTypedParameterArrayClear(params, nparams); + free(params); + return info; + +fail: + virTypedParameterArrayClear(params, nparams); + free(params); + return ret; +} + +static PyObject * libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { virDomainPtr domain; @@ -5162,6 +5474,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 *) "virDomainSetNumaParameters", libvirt_virDomainSetNumaParameters, METH_VARARGS, NULL}, + {(char *) "virDomainGetNumaParameters", libvirt_virDomainGetNumaParameters, METH_VARARGS, NULL}, {(char *) "virDomainGetVcpus", libvirt_virDomainGetVcpus, METH_VARARGS, NULL}, {(char *) "virDomainPinVcpu", libvirt_virDomainPinVcpu, METH_VARARGS, NULL}, {(char *) "virDomainPinVcpuFlags", libvirt_virDomainPinVcpuFlags, METH_VARARGS, NULL}, -- 1.7.7.5

*libvirt_virDomainBlockStatsFlags *libvirt_virDomainGetSchedulerParameters *libvirt_virDomainGetSchedulerParametersFlags *libvirt_virDomainSetSchedulerParameters *libvirt_virDomainSetSchedulerParametersFlags *libvirt_virDomainSetBlkioParameters *libvirt_virDomainGetBlkioParameters *libvirt_virDomainSetMemoryParameters *libvirt_virDomainGetMemoryParameters *libvirt_virDomainSetBlockIoTune *libvirt_virDomainGetBlockIoTune --- python/libvirt-override-api.xml | 10 +- python/libvirt-override.c | 789 ++++++++++---------------------------- 2 files changed, 213 insertions(+), 586 deletions(-) diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 748aa17..934a832 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -144,7 +144,7 @@ </function> <function name='virDomainBlockStatsFlags' file='python'> <info>Extracts block device statistics parameters of a running domain</info> - <return type='virTypedParameterPtr' info='None in case of error, returns a dictionary of params'/> + <return type='int' info='returns a dictionary of params in case of success, -1 in case of error'/> <arg name='domain' type='virDomainPtr' info='pointer to domain object'/> <arg name='path' type='char *' info='the path for the block device'/> <arg name='flags' type='int' info='flags (unused; pass 0)'/> @@ -174,7 +174,7 @@ </function> <function name='virDomainGetSchedulerParametersFlags' file='python'> <info>Get the scheduler parameters</info> - <return type='virSchedParameterPtr' info='None in case of error, returns a dictionary of params'/> + <return type='int' info='returns a dictionary of params in case of success, -1 in case of error'/> <arg name='domain' type='virDomainPtr' info='pointer to domain object'/> <arg name='flags' type='int' info='an OR'ed set of virDomainModificationImpact'/> </function> @@ -231,7 +231,7 @@ </function> <function name='virDomainGetBlkioParameters' file='python'> <info>Get the blkio parameters</info> - <return type='virSchedParameterPtr' info='None in case of error, returns a dictionary of params'/> + <return type='int' info='returns a dictionary of params in case of success, -1 in case of error'/> <arg name='domain' type='virDomainPtr' info='pointer to domain object'/> <arg name='flags' type='int' info='an OR'ed set of virDomainModificationImpact'/> </function> @@ -244,7 +244,7 @@ </function> <function name='virDomainGetMemoryParameters' file='python'> <info>Get the memory parameters</info> - <return type='virSchedParameterPtr' info='None in case of error, returns a dictionary of params'/> + <return type='int' info='returns a dictionary of params in case of success, -1 in case of error'/> <arg name='domain' type='virDomainPtr' info='pointer to domain object'/> <arg name='flags' type='int' info='an OR'ed set of virDomainModificationImpact'/> </function> @@ -415,7 +415,7 @@ <arg name='dom' type='virDomainPtr' info='pointer to the domain'/> <arg name='disk' type='const char *' info='disk name'/> <arg name='flags' type='unsigned int' info='an OR'ed set of virDomainModificationImpact'/> - <return type='virTypedParameterPtr' info='the I/O tunables value or None in case of error'/> + <return type='int' info='returns a dictionary of params in case of success, -1 in case of error'/> </function> <function name='virDomainBlockPeek' file='python'> <info>Read the contents of domain's disk device</info> diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 5f9d83e..5ef2d4a 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -304,11 +304,13 @@ libvirt_virDomainBlockStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { static PyObject * libvirt_virDomainBlockStatsFlags(PyObject *self ATTRIBUTE_UNUSED, - PyObject *args) { + PyObject *args) +{ virDomainPtr domain; PyObject *pyobj_domain, *info; + PyObject *ret = NULL; int i_retval; - int nparams = 0, i; + int nparams = 0; unsigned int flags; virTypedParameterPtr params; const char *path; @@ -323,69 +325,33 @@ libvirt_virDomainBlockStatsFlags(PyObject *self ATTRIBUTE_UNUSED, LIBVIRT_END_ALLOW_THREADS; if (i_retval < 0) - return VIR_PY_NONE; + return VIR_PY_INT_FAIL; if ((params = malloc(sizeof(*params)*nparams)) == NULL) - return VIR_PY_NONE; + return PyErr_NoMemory(); LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainBlockStatsFlags(domain, path, 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; + ret = VIR_PY_INT_FAIL; + goto fail; } - 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; - - default: - free(params); - Py_DECREF(info); - return VIR_PY_NONE; - } - - key = libvirt_constcharPtrWrap(params[i].field); - PyDict_SetItem(info, key, val); - } + info = getPyVirTypedParameter(params, nparams); + if (!info) + goto fail; + virTypedParameterArrayClear(params, nparams); free(params); - return(info); + return info; +fail: + virTypedParameterArrayClear(params, nparams); + free(params); + return ret; } - static PyObject * libvirt_virDomainInterfaceStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { virDomainPtr domain; @@ -498,12 +464,14 @@ libvirt_virDomainGetSchedulerType(PyObject *self ATTRIBUTE_UNUSED, static PyObject * libvirt_virDomainGetSchedulerParameters(PyObject *self ATTRIBUTE_UNUSED, - PyObject *args) { + PyObject *args) +{ virDomainPtr domain; PyObject *pyobj_domain, *info; + PyObject *ret = NULL; char *c_retval; int i_retval; - int nparams, i; + int nparams = 0; virTypedParameterPtr params; if (!PyArg_ParseTuple(args, (char *)"O:virDomainGetScedulerParameters", @@ -516,75 +484,44 @@ libvirt_virDomainGetSchedulerParameters(PyObject *self ATTRIBUTE_UNUSED, LIBVIRT_END_ALLOW_THREADS; if (c_retval == NULL) - return VIR_PY_NONE; + return VIR_PY_INT_FAIL; free(c_retval); if ((params = malloc(sizeof(*params)*nparams)) == NULL) - return VIR_PY_NONE; + return PyErr_NoMemory(); LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetSchedulerParameters(domain, params, &nparams); 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; + ret = VIR_PY_INT_FAIL; + goto fail; } - 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; - - default: - free(params); - Py_DECREF(info); - return VIR_PY_NONE; - } + info = getPyVirTypedParameter(params, nparams); + if (!info) + goto fail; - key = libvirt_constcharPtrWrap(params[i].field); - PyDict_SetItem(info, key, val); - } + virTypedParameterArrayClear(params, nparams); free(params); - return(info); + return info; +fail: + virTypedParameterArrayClear(params, nparams); + free(params); + return ret; } static PyObject * libvirt_virDomainGetSchedulerParametersFlags(PyObject *self ATTRIBUTE_UNUSED, - PyObject *args) { + PyObject *args) +{ virDomainPtr domain; PyObject *pyobj_domain, *info; + PyObject *ret = NULL; char *c_retval; int i_retval; - int nparams, i; + int nparams = 0; unsigned int flags; virTypedParameterPtr params; @@ -598,75 +535,44 @@ libvirt_virDomainGetSchedulerParametersFlags(PyObject *self ATTRIBUTE_UNUSED, LIBVIRT_END_ALLOW_THREADS; if (c_retval == NULL) - return VIR_PY_NONE; + return VIR_PY_INT_FAIL; free(c_retval); if ((params = malloc(sizeof(*params)*nparams)) == NULL) - return VIR_PY_NONE; + return PyErr_NoMemory(); LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetSchedulerParametersFlags(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; + ret = VIR_PY_INT_FAIL; + goto fail; } - 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; - - default: - free(params); - Py_DECREF(info); - return VIR_PY_NONE; - } + info = getPyVirTypedParameter(params, nparams); + if (!info) + goto fail; - key = libvirt_constcharPtrWrap(params[i].field); - PyDict_SetItem(info, key, val); - } + virTypedParameterArrayClear(params, nparams); free(params); - return(info); + return info; +fail: + virTypedParameterArrayClear(params, nparams); + free(params); + return ret; } static PyObject * libvirt_virDomainSetSchedulerParameters(PyObject *self ATTRIBUTE_UNUSED, - PyObject *args) { + PyObject *args) +{ virDomainPtr domain; PyObject *pyobj_domain, *info; + PyObject *ret = NULL; char *c_retval; int i_retval; - int nparams, i; + int nparams = 0; virTypedParameterPtr params; if (!PyArg_ParseTuple(args, (char *)"OO:virDomainSetScedulerParameters", @@ -683,85 +589,48 @@ libvirt_virDomainSetSchedulerParameters(PyObject *self ATTRIBUTE_UNUSED, free(c_retval); if ((params = malloc(sizeof(*params)*nparams)) == NULL) - return VIR_PY_INT_FAIL; + return PyErr_NoMemory(); LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetSchedulerParameters(domain, params, &nparams); LIBVIRT_END_ALLOW_THREADS; if (i_retval < 0) { - free(params); - return VIR_PY_INT_FAIL; + ret = VIR_PY_INT_FAIL; + goto 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; - - default: - free(params); - return VIR_PY_INT_FAIL; - } - } + if (!setPyVirTypedParameter(info, params, nparams)) + goto fail; LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainSetSchedulerParameters(domain, params, nparams); LIBVIRT_END_ALLOW_THREADS; + if (i_retval < 0) { - free(params); - return VIR_PY_INT_FAIL; + ret = VIR_PY_INT_FAIL; + goto fail; } + virTypedParameterArrayClear(params, nparams); free(params); return VIR_PY_INT_SUCCESS; +fail: + virTypedParameterArrayClear(params, nparams); + free(params); + return ret; } static PyObject * libvirt_virDomainSetSchedulerParametersFlags(PyObject *self ATTRIBUTE_UNUSED, - PyObject *args) { + PyObject *args) +{ virDomainPtr domain; PyObject *pyobj_domain, *info; + PyObject *ret = NULL; char *c_retval; int i_retval; - int nparams, i; + int nparams = 0; unsigned int flags; virTypedParameterPtr params; @@ -780,85 +649,47 @@ libvirt_virDomainSetSchedulerParametersFlags(PyObject *self ATTRIBUTE_UNUSED, free(c_retval); if ((params = malloc(sizeof(*params)*nparams)) == NULL) - return VIR_PY_INT_FAIL; + return PyErr_NoMemory(); LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetSchedulerParametersFlags(domain, params, &nparams, flags); LIBVIRT_END_ALLOW_THREADS; if (i_retval < 0) { - free(params); - return VIR_PY_INT_FAIL; + ret = VIR_PY_INT_FAIL; + goto 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; - - default: - free(params); - return VIR_PY_INT_FAIL; - } - } + if (!setPyVirTypedParameter(info, params, nparams)) + goto fail; LIBVIRT_BEGIN_ALLOW_THREADS; - i_retval = virDomainSetSchedulerParametersFlags(domain, params, nparams, flags); + i_retval = virDomainSetSchedulerParametersFlags(domain, params, nparams, 0); LIBVIRT_END_ALLOW_THREADS; + if (i_retval < 0) { - free(params); - return VIR_PY_INT_FAIL; + ret = VIR_PY_INT_FAIL; + goto fail; } + virTypedParameterArrayClear(params, nparams); free(params); return VIR_PY_INT_SUCCESS; +fail: + virTypedParameterArrayClear(params, nparams); + free(params); + return ret; } - static PyObject * libvirt_virDomainSetBlkioParameters(PyObject *self ATTRIBUTE_UNUSED, - PyObject *args) { + PyObject *args) +{ virDomainPtr domain; PyObject *pyobj_domain, *info; + PyObject *ret = NULL; int i_retval; - int nparams = 0, i; + int nparams = 0; unsigned int flags; virTypedParameterPtr params; @@ -876,84 +707,48 @@ libvirt_virDomainSetBlkioParameters(PyObject *self ATTRIBUTE_UNUSED, return VIR_PY_INT_FAIL; if ((params = malloc(sizeof(*params)*nparams)) == NULL) - return VIR_PY_INT_FAIL; + return PyErr_NoMemory(); LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetBlkioParameters(domain, params, &nparams, flags); LIBVIRT_END_ALLOW_THREADS; if (i_retval < 0) { - free(params); - return VIR_PY_INT_FAIL; + ret = VIR_PY_INT_FAIL; + goto 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; - - default: - free(params); - return VIR_PY_INT_FAIL; - } - } + if (!setPyVirTypedParameter(info, params, nparams)) + goto fail; LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainSetBlkioParameters(domain, params, nparams, flags); LIBVIRT_END_ALLOW_THREADS; + if (i_retval < 0) { - free(params); - return VIR_PY_INT_FAIL; + ret = VIR_PY_INT_FAIL; + goto fail; } + /* The string generated by PyString_AsString + * must not be deallocated */ free(params); return VIR_PY_INT_SUCCESS; +fail: + /*same as above*/ + free(params); + return ret; } static PyObject * libvirt_virDomainGetBlkioParameters(PyObject *self ATTRIBUTE_UNUSED, - PyObject *args) { + PyObject *args) +{ virDomainPtr domain; PyObject *pyobj_domain, *info; + PyObject *ret = NULL; int i_retval; - int nparams = 0, i; + int nparams = 0; unsigned int flags; virTypedParameterPtr params; @@ -966,74 +761,43 @@ libvirt_virDomainGetBlkioParameters(PyObject *self ATTRIBUTE_UNUSED, i_retval = virDomainGetBlkioParameters(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 = virDomainGetBlkioParameters(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; + if (i_retval < 0) + return VIR_PY_INT_FAIL; - default: - free(params); - Py_DECREF(info); - return VIR_PY_NONE; - } + if ((params = malloc(sizeof(*params)*nparams)) == NULL) + return PyErr_NoMemory(); - key = libvirt_constcharPtrWrap(params[i].field); - PyDict_SetItem(info, key, val); + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainGetBlkioParameters(domain, params, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) { + ret = VIR_PY_INT_FAIL; + goto fail; } + + info = getPyVirTypedParameter(params, nparams); + if (!info) + goto fail; + + virTypedParameterArrayClear(params, nparams); free(params); - return(info); + return info; +fail: + virTypedParameterArrayClear(params, nparams); + free(params); + return ret; } static PyObject * libvirt_virDomainSetMemoryParameters(PyObject *self ATTRIBUTE_UNUSED, - PyObject *args) { + PyObject *args) +{ virDomainPtr domain; PyObject *pyobj_domain, *info; + PyObject *ret = NULL; int i_retval; - int nparams = 0, i; + int nparams = 0; unsigned int flags; virTypedParameterPtr params; @@ -1051,84 +815,47 @@ libvirt_virDomainSetMemoryParameters(PyObject *self ATTRIBUTE_UNUSED, return VIR_PY_INT_FAIL; if ((params = malloc(sizeof(*params)*nparams)) == NULL) - return VIR_PY_INT_FAIL; + return PyErr_NoMemory(); LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetMemoryParameters(domain, params, &nparams, flags); LIBVIRT_END_ALLOW_THREADS; if (i_retval < 0) { - free(params); - return VIR_PY_INT_FAIL; + ret = VIR_PY_INT_FAIL; + goto 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; - - default: - free(params); - return VIR_PY_INT_FAIL; - } - } + if (!setPyVirTypedParameter(info, params, nparams)) + goto fail; LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainSetMemoryParameters(domain, params, nparams, flags); LIBVIRT_END_ALLOW_THREADS; + if (i_retval < 0) { - free(params); - return VIR_PY_INT_FAIL; + ret = VIR_PY_INT_FAIL; + goto fail; } + virTypedParameterArrayClear(params, nparams); free(params); return VIR_PY_INT_SUCCESS; +fail: + virTypedParameterArrayClear(params, nparams); + free(params); + return ret; } static PyObject * libvirt_virDomainGetMemoryParameters(PyObject *self ATTRIBUTE_UNUSED, - PyObject *args) { + PyObject *args) +{ virDomainPtr domain; PyObject *pyobj_domain, *info; + PyObject *ret = NULL; int i_retval; - int nparams = 0, i; + int nparams = 0; unsigned int flags; virTypedParameterPtr params; @@ -1142,64 +869,32 @@ libvirt_virDomainGetMemoryParameters(PyObject *self ATTRIBUTE_UNUSED, LIBVIRT_END_ALLOW_THREADS; if (i_retval < 0) - return VIR_PY_NONE; + return VIR_PY_INT_FAIL; if ((params = malloc(sizeof(*params)*nparams)) == NULL) - return VIR_PY_NONE; + return PyErr_NoMemory(); LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetMemoryParameters(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; + ret = VIR_PY_INT_FAIL; + goto fail; } - 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; + info = getPyVirTypedParameter(params, nparams); + if (!info) + goto fail; - default: - free(params); - Py_DECREF(info); - return VIR_PY_NONE; - } + virTypedParameterArrayClear(params, nparams); + free(params); + return info; - key = libvirt_constcharPtrWrap(params[i].field); - PyDict_SetItem(info, key, val); - } +fail: + virTypedParameterArrayClear(params, nparams); free(params); - return(info); + return ret; } static PyObject * @@ -3625,93 +3320,56 @@ libvirt_virDomainSetBlockIoTune(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { virDomainPtr domain; - PyObject *pyobj_domain, *pyinfo; + PyObject *pyobj_domain, *info; + PyObject *ret = NULL; const char *disk; + int i_retval; + int nparams = 0; unsigned int flags; virTypedParameterPtr params; - int nparams = 0, i; - int c_ret; if (!PyArg_ParseTuple(args, (char *)"OzOi:virDomainSetBlockIoTune", - &pyobj_domain, &disk, &pyinfo, &flags)) + &pyobj_domain, &disk, &info, &flags)) return(NULL); domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); LIBVIRT_BEGIN_ALLOW_THREADS; - c_ret = virDomainGetBlockIoTune(domain, disk, NULL, &nparams, flags); + i_retval = virDomainGetBlockIoTune(domain, disk, NULL, &nparams, flags); LIBVIRT_END_ALLOW_THREADS; - if (c_ret < 0) + if (i_retval < 0) return VIR_PY_INT_FAIL; if ((params = malloc(sizeof(*params)*nparams)) == NULL) - return VIR_PY_INT_FAIL; + return PyErr_NoMemory(); LIBVIRT_BEGIN_ALLOW_THREADS; - c_ret = virDomainGetBlockIoTune(domain, disk, params, &nparams, flags); + i_retval = virDomainGetBlockIoTune(domain, disk, params, &nparams, flags); LIBVIRT_END_ALLOW_THREADS; - if (c_ret < 0) { - free(params); - return VIR_PY_INT_FAIL; + if (i_retval < 0) { + ret = VIR_PY_INT_FAIL; + goto 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(pyinfo, 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: - { - PyObject *hacktrue = PyBool_FromLong(1); - params[i].value.b = hacktrue == val ? 1: 0; - Py_DECREF(hacktrue); - } - break; - - default: - free(params); - return VIR_PY_INT_FAIL; - } - } + if (!setPyVirTypedParameter(info, params, nparams)) + goto fail; LIBVIRT_BEGIN_ALLOW_THREADS; - c_ret = virDomainSetBlockIoTune(domain, disk, params, nparams, flags); + i_retval = virDomainSetBlockIoTune(domain, disk, params, nparams, flags); LIBVIRT_END_ALLOW_THREADS; - if (c_ret < 0) { - free(params); - return VIR_PY_INT_FAIL; + if (i_retval < 0) { + ret = VIR_PY_INT_FAIL; + goto fail; } - + virTypedParameterArrayClear(params, nparams); free(params); return VIR_PY_INT_SUCCESS; +fail: + virTypedParameterArrayClear(params, nparams); + free(params); + return ret; } static PyObject * @@ -3719,12 +3377,13 @@ libvirt_virDomainGetBlockIoTune(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { virDomainPtr domain; - PyObject *pyobj_domain, *pyreply; + PyObject *pyobj_domain, *info; + PyObject *ret = NULL; const char *disk; - int nparams = 0, i; + int i_retval; + int nparams = 0; unsigned int flags; virTypedParameterPtr params; - int c_ret; if (!PyArg_ParseTuple(args, (char *)"Ozi:virDomainGetBlockIoTune", &pyobj_domain, &disk, &flags)) @@ -3732,68 +3391,36 @@ libvirt_virDomainGetBlockIoTune(PyObject *self ATTRIBUTE_UNUSED, domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); LIBVIRT_BEGIN_ALLOW_THREADS; - c_ret = virDomainGetBlockIoTune(domain, disk, NULL, &nparams, flags); + i_retval = virDomainGetBlockIoTune(domain, disk, NULL, &nparams, flags); LIBVIRT_END_ALLOW_THREADS; - if (c_ret < 0) - return VIR_PY_NONE; + if (i_retval < 0) + return VIR_PY_INT_FAIL; if ((params = malloc(sizeof(*params)*nparams)) == NULL) - return VIR_PY_NONE; + return PyErr_NoMemory(); LIBVIRT_BEGIN_ALLOW_THREADS; - c_ret = virDomainGetBlockIoTune(domain, disk, params, &nparams, flags); + i_retval = virDomainGetBlockIoTune(domain, disk, params, &nparams, flags); LIBVIRT_END_ALLOW_THREADS; - if (c_ret < 0) { - free(params); - return VIR_PY_NONE; - } - - /* convert to a Python tuple of long objects */ - if ((pyreply = PyDict_New()) == NULL) { - free(params); - return VIR_PY_NONE; + if (i_retval < 0) { + ret = VIR_PY_INT_FAIL; + goto fail; } - 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((unsigned 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; + info = getPyVirTypedParameter(params, nparams); + if (!info) + goto fail; - default: - free(params); - Py_DECREF(pyreply); - return VIR_PY_NONE; - } + virTypedParameterArrayClear(params, nparams); + free(params); + return info; - key = libvirt_constcharPtrWrap(params[i].field); - PyDict_SetItem(pyreply, key, val); - } +fail: + virTypedParameterArrayClear(params, nparams); free(params); - return(pyreply); + return ret; } /******************************************* -- 1.7.7.5

On 01/28/2012 07:53 AM, Guannan Ren wrote:
*libvirt_virDomainBlockStatsFlags *libvirt_virDomainGetSchedulerParameters *libvirt_virDomainGetSchedulerParametersFlags *libvirt_virDomainSetSchedulerParameters *libvirt_virDomainSetSchedulerParametersFlags *libvirt_virDomainSetBlkioParameters *libvirt_virDomainGetBlkioParameters *libvirt_virDomainSetMemoryParameters *libvirt_virDomainGetMemoryParameters *libvirt_virDomainSetBlockIoTune *libvirt_virDomainGetBlockIoTune --- python/libvirt-override-api.xml | 10 +- python/libvirt-override.c | 789 ++++++++++---------------------------- 2 files changed, 213 insertions(+), 586 deletions(-)
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 748aa17..934a832 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -144,7 +144,7 @@ </function> <function name='virDomainBlockStatsFlags' file='python'> <info>Extracts block device statistics parameters of a running domain</info> - <return type='virTypedParameterPtr' info='None in case of error, returns a dictionary of params'/> + <return type='int' info='returns a dictionary of params in case of success, -1 in case of error'/>
type='int' is wrong; we're returning a dictionary on success, so I would argue that we return None on error (and NULL if we want to raise a python exception, but that doesn't have to be part of the python self-documentation). Same for all the getter functions.
+++ b/python/libvirt-override.c @@ -304,11 +304,13 @@ libvirt_virDomainBlockStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) {
static PyObject * libvirt_virDomainBlockStatsFlags(PyObject *self ATTRIBUTE_UNUSED, - PyObject *args) { + PyObject *args) +{ virDomainPtr domain; PyObject *pyobj_domain, *info; + PyObject *ret = NULL; int i_retval; - int nparams = 0, i; + int nparams = 0; unsigned int flags; virTypedParameterPtr params; const char *path; @@ -323,69 +325,33 @@ libvirt_virDomainBlockStatsFlags(PyObject *self ATTRIBUTE_UNUSED, LIBVIRT_END_ALLOW_THREADS;
if (i_retval < 0) - return VIR_PY_NONE; + return VIR_PY_INT_FAIL;
This change doesn't make sense.
if ((params = malloc(sizeof(*params)*nparams)) == NULL) - return VIR_PY_NONE; + return PyErr_NoMemory();
This one looks good. But same comment as in 1/2 about skipping the malloc() and just using 'return PyDict_New();' for an empty dictionary (or python exception) if i_retval is 0.
LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainBlockStatsFlags(domain, path, 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; + ret = VIR_PY_INT_FAIL; + goto fail;
Should still return VIR_PY_NONE if i_retval is < 0.
+ info = getPyVirTypedParameter(params, nparams);
Yay - the benefits of factoring common code into a helper method.
+ if (!info) + goto fail;
+ virTypedParameterArrayClear(params, nparams); free(params); - return(info); + return info; +fail: + virTypedParameterArrayClear(params, nparams); + free(params); + return ret;
You could share these, by using the label cleanup, and: ret = info; cleanup; virTypedParameterArrayClear(params, nparams); VIR_FREE(params); return ret; Similar comments on all the getter functions.
@@ -683,85 +589,48 @@ libvirt_virDomainSetSchedulerParameters(PyObject *self ATTRIBUTE_UNUSED, free(c_retval);
if ((params = malloc(sizeof(*params)*nparams)) == NULL) - return VIR_PY_INT_FAIL; + return PyErr_NoMemory();
LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetSchedulerParameters(domain, params, &nparams); LIBVIRT_END_ALLOW_THREADS;
if (i_retval < 0) { - free(params); - return VIR_PY_INT_FAIL; + ret = VIR_PY_INT_FAIL; + goto fail; }
Looks good.
+ if (!setPyVirTypedParameter(info, params, nparams)) + goto fail;
LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainSetSchedulerParameters(domain, params, nparams);
Same comments as on 1/2 - all the setter functions should be remembering and using the new params array created by setPyVirtTypedParameter, and not the original params from the getter function. Probably not worth rewriting this patch until we are happy with how the factoring for 1/2 works out, since it will be copying the same design pattern to multiple functions. I'll see about posting a patch that refactors the Makefile to let the python glue code use libvirt_util, and we should get that in before rebasing this series on top of that. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 01/28/2012 10:53 PM, Guannan Ren wrote:
*virDomainSetNumaParameters *virDomainGetNumaParameters --- python/Makefile.am | 4 +- python/libvirt-override-api.xml | 13 ++ python/libvirt-override.c | 314 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 330 insertions(+), 1 deletions(-)
diff --git a/python/Makefile.am b/python/Makefile.am index 3068eee..4302fa5 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -8,6 +8,8 @@ SUBDIRS= . tests INCLUDES = \ $(PYTHON_INCLUDES) \ -I$(top_srcdir)/include \ + -I$(top_srcdir)/src \ + -I$(top_srcdir)/gnulib/lib \ -I$(top_builddir)/include \ -I$(top_builddir)/$(subdir) \ $(GETTEXT_CPPFLAGS) @@ -42,7 +44,7 @@ all-local: libvirt.py libvirt_qemu.py
pyexec_LTLIBRARIES = libvirtmod.la libvirtmod_qemu.la
-libvirtmod_la_SOURCES = libvirt-override.c typewrappers.c +libvirtmod_la_SOURCES = libvirt-override.c typewrappers.c ../src/util/virtypedparam.c
It exists a compilation error for me: make[3]: Entering directory `/home/ajia/Workspace/libvirt/python' CC libvirtmod_la-libvirt-override.lo In file included from ../src/util/virtypedparam.h:26, from libvirt-override.c:24: ../src/internal.h:253:23: error: probes.h: No such file or directory make[3]: *** [libvirtmod_la-libvirt-override.lo] Error 1
nodist_libvirtmod_la_SOURCES = libvirt.c libvirt.h # Python<= 2.4 header files contain a redundant decl, hence we # need extra flags here diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 704fee9..748aa17 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 numa tunable objects'/> +<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='int' info='returns a dictionary of params in case of success, -1 in case of error'/> +<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..5f9d83e 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -21,6 +21,7 @@ #include "libvirt/virterror.h" #include "typewrappers.h" #include "libvirt.h" +#include "util/virtypedparam.h"
#ifndef __CYGWIN__ extern void initlibvirtmod(void); @@ -61,6 +62,208 @@ static char *py_str(PyObject *obj) return PyString_AsString(str); }
+/* Two helper functions to help the conversions between C to Python + * for the virTypedParameter used in the following APIs. */ +static PyObject * +getPyVirTypedParameter(virTypedParameterPtr params, int nparams) +{ + PyObject *info; + PyObject *key, *val; + PyObject *ret = NULL; + int i; + + if (!params) + return ret; + + /* convert to a Python tuple of long objects */ + if ((info = PyDict_New()) == NULL) { + return ret; + } + + for (i = 0 ; i< nparams ; i++) { + 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((unsigned 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_FromUnsignedLongLong((unsigned 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); + break; + + default: + Py_DECREF(info); + return ret; + } + + key = libvirt_constcharPtrWrap(params[i].field); + if (!key || !val) + goto fail; + + if (PyDict_SetItem(info, key, val)< 0) + goto fail; + + Py_DECREF(key); + Py_DECREF(val); + } + return info; +fail: + Py_XDECREF(info); + Py_XDECREF(key); + Py_XDECREF(val); + return ret; +} + +static PyObject * +setPyVirTypedParameter(PyObject *info, virTypedParameterPtr params, int nparams) +{ + PyObject *key, *val; + PyObject *ret = NULL; + int i; + + if (!info || !params) + return ret; + + /* convert to a Python tuple of long objects */ + for (i = 0; i< nparams; i++) { + 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: + { + long long_val; + if (PyInt_Check(val)) { + long_val = PyInt_AsLong(val); + if ((long_val == -1)&& PyErr_Occurred()) + return ret; + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be int"); + return ret; + } + params[i].value.i = (int)long_val; + } + break; + case VIR_TYPED_PARAM_UINT: + { + long long_val; + if (PyInt_Check(val)) { + long_val = PyInt_AsLong(val); + if ((long_val == -1)&& PyErr_Occurred()) + return ret; + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be int"); + return ret; + } + params[i].value.ui = (unsigned int)long_val; + } + break; + case VIR_TYPED_PARAM_LLONG: + { + long long llong_val; + if (PyLong_Check(val)) { + llong_val = PyLong_AsLongLong(val); + if ((llong_val == -1)&& PyErr_Occurred()) + return ret; + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be long"); + return ret; + } + params[i].value.l = llong_val; + } + break; + case VIR_TYPED_PARAM_ULLONG: + { + unsigned long long ullong_val; + if (PyLong_Check(val)) { + ullong_val = PyLong_AsUnsignedLongLong(val); + if ((ullong_val == -1)&& PyErr_Occurred()) + return ret; + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be long"); + return ret; + + } + params[i].value.ul = ullong_val; + } + break; + case VIR_TYPED_PARAM_DOUBLE: + { + double double_val; + if (PyFloat_Check(val)) { + double_val = PyFloat_AsDouble(val); + if ((double_val == -1)&& PyErr_Occurred()) + return ret; + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be float"); + return ret; + } + params[i].value.d = double_val; + } + break; + case VIR_TYPED_PARAM_BOOLEAN: + { + /* Hack - Python's definition of Py_True breaks strict + * aliasing rules, so can't directly compare + */ + if (PyBool_Check(val)) { + PyObject *hacktrue = PyBool_FromLong(1); + params[i].value.b = hacktrue == val ? 1: 0; + Py_DECREF(hacktrue); + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be bool"); + return ret; + } + } + break; + case VIR_TYPED_PARAM_STRING: + { + if (PyString_Check(val)) { + free(params[i].value.s); + params[i].value.s = PyString_AsString(val); + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be string"); + return ret; + } + } + break; + default: + return ret; + } + } + return VIR_PY_NONE; +} + /************************************************************************ * * * Statistics * @@ -1000,6 +1203,115 @@ libvirt_virDomainGetMemoryParameters(PyObject *self ATTRIBUTE_UNUSED, }
static PyObject * +libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + virDomainPtr domain; + PyObject *pyobj_domain, *info; + PyObject *ret = NULL; + int i_retval; + int nparams = 0; + 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 PyErr_NoMemory(); + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainGetNumaParameters(domain, params,&nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval< 0) { + ret = VIR_PY_INT_FAIL; + goto fail; + } + + if (!setPyVirTypedParameter(info, params, nparams)) + goto fail; + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainSetNumaParameters(domain, params, nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval< 0) { + ret = VIR_PY_INT_FAIL; + goto fail; + } + + /* The string generated by PyString_AsString + * must not be deallocated */ + free(params); + return VIR_PY_INT_SUCCESS; +fail: + /*same as above*/ + free(params); + return ret; +} + +static PyObject * +libvirt_virDomainGetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + virDomainPtr domain; + PyObject *pyobj_domain, *info; + PyObject *ret = NULL; + int i_retval; + int nparams = 0; + 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_INT_FAIL; + + if ((params = malloc(sizeof(*params)*nparams)) == NULL) + return PyErr_NoMemory(); + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainGetNumaParameters(domain, params,&nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval< 0) { + ret = VIR_PY_INT_FAIL; + goto fail; + } + + info = getPyVirTypedParameter(params, nparams); + if (!info) + goto fail; + + virTypedParameterArrayClear(params, nparams); + free(params); + return info; + +fail: + virTypedParameterArrayClear(params, nparams); + free(params); + return ret; +} + +static PyObject * libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { virDomainPtr domain; @@ -5162,6 +5474,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 *) "virDomainSetNumaParameters", libvirt_virDomainSetNumaParameters, METH_VARARGS, NULL}, + {(char *) "virDomainGetNumaParameters", libvirt_virDomainGetNumaParameters, METH_VARARGS, NULL}, {(char *) "virDomainGetVcpus", libvirt_virDomainGetVcpus, METH_VARARGS, NULL}, {(char *) "virDomainPinVcpu", libvirt_virDomainPinVcpu, METH_VARARGS, NULL}, {(char *) "virDomainPinVcpuFlags", libvirt_virDomainPinVcpuFlags, METH_VARARGS, NULL},

On 01/29/2012 01:39 PM, Alex Jia wrote:
On 01/28/2012 10:53 PM, Guannan Ren wrote:
*virDomainSetNumaParameters *virDomainGetNumaParameters --- python/Makefile.am | 4 +- python/libvirt-override-api.xml | 13 ++ python/libvirt-override.c | 314 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 330 insertions(+), 1 deletions(-)
diff --git a/python/Makefile.am b/python/Makefile.am index 3068eee..4302fa5 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -8,6 +8,8 @@ SUBDIRS= . tests INCLUDES = \ $(PYTHON_INCLUDES) \ -I$(top_srcdir)/include \ + -I$(top_srcdir)/src \ + -I$(top_srcdir)/gnulib/lib \ -I$(top_builddir)/include \ -I$(top_builddir)/$(subdir) \ $(GETTEXT_CPPFLAGS) @@ -42,7 +44,7 @@ all-local: libvirt.py libvirt_qemu.py
pyexec_LTLIBRARIES = libvirtmod.la libvirtmod_qemu.la
-libvirtmod_la_SOURCES = libvirt-override.c typewrappers.c +libvirtmod_la_SOURCES = libvirt-override.c typewrappers.c ../src/util/virtypedparam.c
It exists a compilation error for me:
make[3]: Entering directory `/home/ajia/Workspace/libvirt/python' CC libvirtmod_la-libvirt-override.lo In file included from ../src/util/virtypedparam.h:26, from libvirt-override.c:24: ../src/internal.h:253:23: error: probes.h: No such file or directory make[3]: *** [libvirtmod_la-libvirt-override.lo] Error 1
did you add "-I$(top_srcdir)/src" in python/Makefile.am it tell where to get the ./src/probes.h

On 01/29/2012 03:29 PM, Guannan Ren wrote:
On 01/29/2012 01:39 PM, Alex Jia wrote:
On 01/28/2012 10:53 PM, Guannan Ren wrote:
*virDomainSetNumaParameters *virDomainGetNumaParameters --- python/Makefile.am | 4 +- python/libvirt-override-api.xml | 13 ++ python/libvirt-override.c | 314 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 330 insertions(+), 1 deletions(-)
diff --git a/python/Makefile.am b/python/Makefile.am index 3068eee..4302fa5 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -8,6 +8,8 @@ SUBDIRS= . tests INCLUDES = \ $(PYTHON_INCLUDES) \ -I$(top_srcdir)/include \ + -I$(top_srcdir)/src \ + -I$(top_srcdir)/gnulib/lib \ -I$(top_builddir)/include \ -I$(top_builddir)/$(subdir) \ $(GETTEXT_CPPFLAGS) @@ -42,7 +44,7 @@ all-local: libvirt.py libvirt_qemu.py
pyexec_LTLIBRARIES = libvirtmod.la libvirtmod_qemu.la
-libvirtmod_la_SOURCES = libvirt-override.c typewrappers.c +libvirtmod_la_SOURCES = libvirt-override.c typewrappers.c ../src/util/virtypedparam.c
It exists a compilation error for me:
make[3]: Entering directory `/home/ajia/Workspace/libvirt/python' CC libvirtmod_la-libvirt-override.lo In file included from ../src/util/virtypedparam.h:26, from libvirt-override.c:24: ../src/internal.h:253:23: error: probes.h: No such file or directory make[3]: *** [libvirtmod_la-libvirt-override.lo] Error 1
did you add "-I$(top_srcdir)/src" in python/Makefile.am it tell where to get the ./src/probes.h
It seems I must to install systemtap-sdt-devel on rhel, then libvirt will check whether 'dtrace' enable, the issue is whether it should complication fails if lack of 'dtrace', if so, we should tell user to install systemtap-sdt-devel package as a pre-requirement, however, lack of probes.h doesn't affect compilation for people not using dtrace, for details, please see following link: http://www.redhat.com/archives/libvir-list/2010-October/msg00886.html Regards, Alex

Could anyone please help to review these patchset? Thanks :) On 01/28/2012 10:53 PM, Guannan Ren wrote:
*virDomainSetNumaParameters *virDomainGetNumaParameters --- python/Makefile.am | 4 +- python/libvirt-override-api.xml | 13 ++ python/libvirt-override.c | 314 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 330 insertions(+), 1 deletions(-)
diff --git a/python/Makefile.am b/python/Makefile.am index 3068eee..4302fa5 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -8,6 +8,8 @@ SUBDIRS= . tests INCLUDES = \ $(PYTHON_INCLUDES) \ -I$(top_srcdir)/include \ + -I$(top_srcdir)/src \ + -I$(top_srcdir)/gnulib/lib \ -I$(top_builddir)/include \ -I$(top_builddir)/$(subdir) \ $(GETTEXT_CPPFLAGS) @@ -42,7 +44,7 @@ all-local: libvirt.py libvirt_qemu.py
pyexec_LTLIBRARIES = libvirtmod.la libvirtmod_qemu.la
-libvirtmod_la_SOURCES = libvirt-override.c typewrappers.c +libvirtmod_la_SOURCES = libvirt-override.c typewrappers.c ../src/util/virtypedparam.c nodist_libvirtmod_la_SOURCES = libvirt.c libvirt.h # Python<= 2.4 header files contain a redundant decl, hence we # need extra flags here diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 704fee9..748aa17 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 numa tunable objects'/> +<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='int' info='returns a dictionary of params in case of success, -1 in case of error'/> +<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..5f9d83e 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -21,6 +21,7 @@ #include "libvirt/virterror.h" #include "typewrappers.h" #include "libvirt.h" +#include "util/virtypedparam.h"
#ifndef __CYGWIN__ extern void initlibvirtmod(void); @@ -61,6 +62,208 @@ static char *py_str(PyObject *obj) return PyString_AsString(str); }
+/* Two helper functions to help the conversions between C to Python + * for the virTypedParameter used in the following APIs. */ +static PyObject * +getPyVirTypedParameter(virTypedParameterPtr params, int nparams) +{ + PyObject *info; + PyObject *key, *val; + PyObject *ret = NULL; + int i; + + if (!params) + return ret; + + /* convert to a Python tuple of long objects */ + if ((info = PyDict_New()) == NULL) { + return ret; + } + + for (i = 0 ; i< nparams ; i++) { + 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((unsigned 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_FromUnsignedLongLong((unsigned 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); + break; + + default: + Py_DECREF(info); + return ret; + } + + key = libvirt_constcharPtrWrap(params[i].field); + if (!key || !val) + goto fail; + + if (PyDict_SetItem(info, key, val)< 0) + goto fail; + + Py_DECREF(key); + Py_DECREF(val); + } + return info; +fail: + Py_XDECREF(info); + Py_XDECREF(key); + Py_XDECREF(val); + return ret; +} + +static PyObject * +setPyVirTypedParameter(PyObject *info, virTypedParameterPtr params, int nparams) +{ + PyObject *key, *val; + PyObject *ret = NULL; + int i; + + if (!info || !params) + return ret; + + /* convert to a Python tuple of long objects */ + for (i = 0; i< nparams; i++) { + 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: + { + long long_val; + if (PyInt_Check(val)) { + long_val = PyInt_AsLong(val); + if ((long_val == -1)&& PyErr_Occurred()) + return ret; + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be int"); + return ret; + } + params[i].value.i = (int)long_val; + } + break; + case VIR_TYPED_PARAM_UINT: + { + long long_val; + if (PyInt_Check(val)) { + long_val = PyInt_AsLong(val); + if ((long_val == -1)&& PyErr_Occurred()) + return ret; + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be int"); + return ret; + } + params[i].value.ui = (unsigned int)long_val; + } + break; + case VIR_TYPED_PARAM_LLONG: + { + long long llong_val; + if (PyLong_Check(val)) { + llong_val = PyLong_AsLongLong(val); + if ((llong_val == -1)&& PyErr_Occurred()) + return ret; + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be long"); + return ret; + } + params[i].value.l = llong_val; + } + break; + case VIR_TYPED_PARAM_ULLONG: + { + unsigned long long ullong_val; + if (PyLong_Check(val)) { + ullong_val = PyLong_AsUnsignedLongLong(val); + if ((ullong_val == -1)&& PyErr_Occurred()) + return ret; + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be long"); + return ret; + + } + params[i].value.ul = ullong_val; + } + break; + case VIR_TYPED_PARAM_DOUBLE: + { + double double_val; + if (PyFloat_Check(val)) { + double_val = PyFloat_AsDouble(val); + if ((double_val == -1)&& PyErr_Occurred()) + return ret; + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be float"); + return ret; + } + params[i].value.d = double_val; + } + break; + case VIR_TYPED_PARAM_BOOLEAN: + { + /* Hack - Python's definition of Py_True breaks strict + * aliasing rules, so can't directly compare + */ + if (PyBool_Check(val)) { + PyObject *hacktrue = PyBool_FromLong(1); + params[i].value.b = hacktrue == val ? 1: 0; + Py_DECREF(hacktrue); + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be bool"); + return ret; + } + } + break; + case VIR_TYPED_PARAM_STRING: + { + if (PyString_Check(val)) { + free(params[i].value.s); + params[i].value.s = PyString_AsString(val); + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be string"); + return ret; + } + } + break; + default: + return ret; + } + } + return VIR_PY_NONE; +} + /************************************************************************ * * * Statistics * @@ -1000,6 +1203,115 @@ libvirt_virDomainGetMemoryParameters(PyObject *self ATTRIBUTE_UNUSED, }
static PyObject * +libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + virDomainPtr domain; + PyObject *pyobj_domain, *info; + PyObject *ret = NULL; + int i_retval; + int nparams = 0; + 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 PyErr_NoMemory(); + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainGetNumaParameters(domain, params,&nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval< 0) { + ret = VIR_PY_INT_FAIL; + goto fail; + } + + if (!setPyVirTypedParameter(info, params, nparams)) + goto fail; + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainSetNumaParameters(domain, params, nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval< 0) { + ret = VIR_PY_INT_FAIL; + goto fail; + } + + /* The string generated by PyString_AsString + * must not be deallocated */ + free(params); + return VIR_PY_INT_SUCCESS; +fail: + /*same as above*/ + free(params); + return ret; +} + +static PyObject * +libvirt_virDomainGetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + virDomainPtr domain; + PyObject *pyobj_domain, *info; + PyObject *ret = NULL; + int i_retval; + int nparams = 0; + 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_INT_FAIL; + + if ((params = malloc(sizeof(*params)*nparams)) == NULL) + return PyErr_NoMemory(); + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainGetNumaParameters(domain, params,&nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval< 0) { + ret = VIR_PY_INT_FAIL; + goto fail; + } + + info = getPyVirTypedParameter(params, nparams); + if (!info) + goto fail; + + virTypedParameterArrayClear(params, nparams); + free(params); + return info; + +fail: + virTypedParameterArrayClear(params, nparams); + free(params); + return ret; +} + +static PyObject * libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { virDomainPtr domain; @@ -5162,6 +5474,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 *) "virDomainSetNumaParameters", libvirt_virDomainSetNumaParameters, METH_VARARGS, NULL}, + {(char *) "virDomainGetNumaParameters", libvirt_virDomainGetNumaParameters, METH_VARARGS, NULL}, {(char *) "virDomainGetVcpus", libvirt_virDomainGetVcpus, METH_VARARGS, NULL}, {(char *) "virDomainPinVcpu", libvirt_virDomainPinVcpu, METH_VARARGS, NULL}, {(char *) "virDomainPinVcpuFlags", libvirt_virDomainPinVcpuFlags, METH_VARARGS, NULL},

On 01/28/2012 07:53 AM, Guannan Ren wrote:
*virDomainSetNumaParameters *virDomainGetNumaParameters --- python/Makefile.am | 4 +- python/libvirt-override-api.xml | 13 ++ python/libvirt-override.c | 314 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 330 insertions(+), 1 deletions(-)
diff --git a/python/Makefile.am b/python/Makefile.am index 3068eee..4302fa5 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -8,6 +8,8 @@ SUBDIRS= . tests INCLUDES = \ $(PYTHON_INCLUDES) \ -I$(top_srcdir)/include \ + -I$(top_srcdir)/src \ + -I$(top_srcdir)/gnulib/lib \
Hmm, you converted TAB to space.
-I$(top_builddir)/include \
Also, since gnulib has some directly-supplied headers (srcdir) and some generated headers (builddir), we really should be using both locations so as not to break VPATH.
-I$(top_builddir)/$(subdir) \ $(GETTEXT_CPPFLAGS) @@ -42,7 +44,7 @@ all-local: libvirt.py libvirt_qemu.py
pyexec_LTLIBRARIES = libvirtmod.la libvirtmod_qemu.la
-libvirtmod_la_SOURCES = libvirt-override.c typewrappers.c +libvirtmod_la_SOURCES = libvirt-override.c typewrappers.c ../src/util/virtypedparam.c
I'm not sure I like this. Rather than pulling in just one or two source files, we should probably instead figure out how to directly link against the libvirt_util library and have all of the functions available. This would also make it possible to use VIR_FREE and friends (at which point, we should disable the syntax-check exceptions currently in effect on the python files). I think I will do a preliminary patch, which does _just_ the makefile work to pull in the use of libvirt_util, then we can rebase this patch on top of that one. I know Alex Jia was also complaining about the inability to use normal libvirt conventions, because the Makefile wasn't yet set up for it, so this will be a good move overall.
+ <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 numa tunable objects'/>
Is th is type correct, or can it be any python dictionary type that maps valid numa tunable parameter names to values?
+ <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='int' info='returns a dictionary of params in case of success, -1 in case of error'/>
The return type should be a python object - a dictionary on success, PyNone on failure where libvirt populated an error message, or NULL on a python exception.
+++ b/python/libvirt-override.c @@ -21,6 +21,7 @@ #include "libvirt/virterror.h" #include "typewrappers.h" #include "libvirt.h" +#include "util/virtypedparam.h"
Hmm, the rest of our code sets up INCLUDES so that we can use just "virtypedparam.h" instead of "util/virtypedparam.h"; another thing for me to do in pulling out the infrastructure into a preliminary patch.
#ifndef __CYGWIN__ extern void initlibvirtmod(void); @@ -61,6 +62,208 @@ static char *py_str(PyObject *obj) return PyString_AsString(str); }
+/* Two helper functions to help the conversions between C to Python + * for the virTypedParameter used in the following APIs. */ +static PyObject * +getPyVirTypedParameter(virTypedParameterPtr params, int nparams) +{ + PyObject *info; + PyObject *key, *val; + PyObject *ret = NULL; + int i; + + if (!params) + return ret;
If we return NULL, we should ensure that there is a valid python exception object ready for the caller to access. I'm thinking it might be better to mark this function with ATTRIBUTE_NONNULL(1) to avoid worrying about whether the caller has properly generated a python exception before passing us NULL.
+ + /* convert to a Python tuple of long objects */ + if ((info = PyDict_New()) == NULL) { + return ret; + }
This one's fine - PyDict_New guarantees a python exception is ready to go.
+ + for (i = 0 ; i < nparams ; i++) { + switch (params[i].type) { + case VIR_TYPED_PARAM_INT: + val = PyInt_FromLong((long)params[i].value.i); + break;
+ + case VIR_TYPED_PARAM_STRING: + val = libvirt_constcharPtrWrap(params[i].value.s); + break; + + default: + Py_DECREF(info); + return ret;
This returns NULL but without setting a python exception. Then again, this can only happen if the server sent us something which we don't understand (arguably a bug in the libvirt.so used by the server). Maybe it would be better to return Py_None instead of NULL in this situation, since I don't know what python exception we would raise. At any rate, if my understanding is correct, your use of a simple Py_DECREF of a non-empty python dictionary sets up garbage collection for all the contents of that dictionary (that is, we don't have to explicitly clean up the contents, just abandoning info was good enough).
+ } + + key = libvirt_constcharPtrWrap(params[i].field); + if (!key || !val) + goto fail; + + if (PyDict_SetItem(info, key, val) < 0) + goto fail; + + Py_DECREF(key); + Py_DECREF(val); + } + return info; +fail: + Py_XDECREF(info); + Py_XDECREF(key); + Py_XDECREF(val); + return ret; +}
The rest of this function looks right to me - any way that gets to the fail label will properly have set a python exception and return NULL.
+ +static PyObject * +setPyVirTypedParameter(PyObject *info, virTypedParameterPtr params, int nparams)
A comment would go a long way on describing what this function does. My understanding is that it takes an incoming python object 'info', which should behave like a dictionary containing just name/value references, as well as incoming params previously collected from the domain. Then for every key in params found in info, we update params in-place to contain the new value from info, so that we can then pass params back to libvirt to update values. I think this has a couple of design issues. Consider virDomainSetBlockIoTune, which currently has 6 defined parameters when getting values, but where the parameters are, to some degree, mutually exclusive. That is, if I set VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC to non-zero, then I must also omit VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC or else set it to 0 when setting values. But if read_bytes_sec was previously set, and the user passes in a python array containing just a mapping for a non-zero total_bytes_sec, then modifying params in place and passing back all 6 parameters will cause an error (we're passing in conflicting non-zero values), while passing in a params of just length 1 will properly let libvirt recognize that I am replacing read_bytes_sec with total_bytes_sec (well, it will once I fix the bugs I just noticed in qemu_driver.c in handling that situation). The python code should mirror the C calling conventions where I can pass in a smaller array than what I get back out on queries, so that libvirt is free to reuse or replace unspecified parameters as it sees fit. In other words, I think we really want a different signature - we want to pass in three items - a python dictionary, and a C array and length that tell us what types the code is expecting, and in return get one item - a new C array with length given by PyDict_Size of the incoming dictionary, or NULL if a python exception has been raised. Something like this signature: /* Allocate a new typed parameter array with the same contents and * length as info, and using the array params of length nparams as * hints on what types to use when creating the new array. Return * NULL on failure. */ static virTypedParamterPtr setPyVirTypedParameter(PyObject *info, const virTypedParameterPtr params, int nparams)
+{ + PyObject *key, *val; + PyObject *ret = NULL; + int i; + + if (!info || !params) + return ret;
Not so good. We are returning NULL without setting a python exception. Again an argument that these should be ATTRIBUTE_NONNULL, and that the caller should not be passing invalid data.
+ + /* convert to a Python tuple of long objects */
Comment is wrong. We are converting a python dictionary into a C struct virTypedParameter array.
+ for (i = 0; i < nparams; i++) {
I think we're iterating over the wrong struct. We want to convert ever object in info into a new C structure, and fail if we don't know how to do the conversion. That is, I think we need the overall structure of this function to look more like: pos = 0; allocate return value according to size of info while (PyDict_Next(info, &pos, &key, &value)) { for (i = 0; i < nparams; i++) { if params[i].field matches key, use params[i].type and break } if i == nparams, error out that we don't know how to convert key populate ret[pos] according to learned type }
+ key = libvirt_constcharPtrWrap(params[i].field);
Need to check for NULL return, and error out.
+ val = PyDict_GetItem(info, key); + Py_DECREF(key); + + if (val == NULL) + continue;
This says that a parameter in params but not in the dictionary is left unmodified, but it doesn't filter out entries in the dictionary but not in params. Again evidence that we are iterating over the wrong struct.
+ + switch (params[i].type) { + case VIR_TYPED_PARAM_INT: + { + long long_val; + if (PyInt_Check(val)) { + long_val = PyInt_AsLong(val); + if ((long_val == -1) && PyErr_Occurred()) + return ret; + } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be int"); + return ret; + } + params[i].value.i = (int)long_val;
This allows silent overflow. We should validate that (int)long_val == long_val, and reject it if it is out of range.
+ case VIR_TYPED_PARAM_STRING: + { + if (PyString_Check(val)) { + free(params[i].value.s); + params[i].value.s = PyString_AsString(val);
This can lead to some problematic allocation patterns, since the caller must not free this string, but you are replacing a string that the caller must otherwise free to avoid a leak. All the more reason you need to return a new virTypedParameterPtr array, and not modify the incoming one in place. You are missing a check for a NULL return; but then again, the previous call to PyString_Check guarantees that you should succeed. On the other hand, since PyString_AsString properly raises a python exception if val is not a string, we could skip the PyString_Check() call. I don't know which way is easier to write.
+ } else { + PyErr_SetString(PyExc_TypeError, + "Attribute value type must be string"); + return ret; + } + } + break; + default: + return ret; + } + } + return VIR_PY_NONE;
Here, I'd return the new array that we just created, or NULL; no need to return VIR_PY_NONE since the result of this function is not passed back to python, but only used in calling a libvirt C function.
static PyObject * +libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args)
Indentation.
+ if (!setPyVirTypedParameter(info, params, nparams)) + goto fail;
Given my above thoughts, you should be storing the result of this call,
+ + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainSetNumaParameters(domain, params, nparams, flags);
and passing that result, along with the size of info (and _not_ nparams).
+ LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) { + ret = VIR_PY_INT_FAIL; + goto fail; + } + + /* The string generated by PyString_AsString + * must not be deallocated */
Ah, but if you follow my advice about setPyVirTypedParameter returning a _new_ array, then what we really want to do here is: virTypedParameterArrayClear(params, nparams); VIR_FREE(params); VIR_FREE(new_params); That is, params contains strings returned by libvirt which the caller must free, while new_params contains strings which point into python objects and must not be freed.
+ free(params); + return VIR_PY_INT_SUCCESS; +fail: + /*same as above*/ + free(params); + return ret;
We should probably combine these cleanups into a single path, using our common idiom: ret = VIR_PY_INT_SUCCESS; cleanup: free params... return ret; and change goto fail into goto cleanup.
+} + +static PyObject * +libvirt_virDomainGetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{
+ + if (i_retval < 0) + return VIR_PY_INT_FAIL;
Wrong return value - here, you want to return None, since the successful return is a python dictionary and not the integer 0. You might want to take a shortcut here - if nparams is 0, you can return an empty dictionary, rather than continuing on with mallocing a 0-length array and wasting time with more libvirt calls.
+ + if ((params = malloc(sizeof(*params)*nparams)) == NULL) + return PyErr_NoMemory(); + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainGetNumaParameters(domain, params, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) { + ret = VIR_PY_INT_FAIL;
Again, this should return None, not -1. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 02/03/2012 01:48 AM, Eric Blake wrote:
On 01/28/2012 07:53 AM, Guannan Ren wrote:
*virDomainSetNumaParameters *virDomainGetNumaParameters --- python/Makefile.am | 4 +- python/libvirt-override-api.xml | 13 ++ python/libvirt-override.c | 314 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 330 insertions(+), 1 deletions(-)
diff --git a/python/Makefile.am b/python/Makefile.am index 3068eee..4302fa5 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -8,6 +8,8 @@ SUBDIRS= . tests INCLUDES = \ $(PYTHON_INCLUDES) \ -I$(top_srcdir)/include \ + -I$(top_srcdir)/src \ + -I$(top_srcdir)/gnulib/lib \
Hi Eric Thank you for your so patient comments as well as the time on them. Let me rethink about it. Regards, Guannan Ren

On 02/03/2012 01:48 AM, Eric Blake wrote:
On 01/28/2012 07:53 AM, Guannan Ren wrote:
*virDomainSetNumaParameters *virDomainGetNumaParameters --- python/Makefile.am | 4 +- python/libvirt-override-api.xml | 13 ++ python/libvirt-override.c | 314 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 330 insertions(+), 1 deletions(-)
diff --git a/python/Makefile.am b/python/Makefile.am index 3068eee..4302fa5 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -8,6 +8,8 @@ SUBDIRS= . tests INCLUDES = \ $(PYTHON_INCLUDES) \ -I$(top_srcdir)/include \ + -I$(top_srcdir)/src \ + -I$(top_srcdir)/gnulib/lib \
Hmm, you converted TAB to space.
-I$(top_builddir)/include \ Also, since gnulib has some directly-supplied headers (srcdir) and some generated headers (builddir), we really should be using both locations so as not to break VPATH.
-I$(top_builddir)/$(subdir) \ $(GETTEXT_CPPFLAGS) @@ -42,7 +44,7 @@ all-local: libvirt.py libvirt_qemu.py
pyexec_LTLIBRARIES = libvirtmod.la libvirtmod_qemu.la
-libvirtmod_la_SOURCES = libvirt-override.c typewrappers.c +libvirtmod_la_SOURCES = libvirt-override.c typewrappers.c ../src/util/virtypedparam.c I'm not sure I like this. Rather than pulling in just one or two source files, we should probably instead figure out how to directly link against the libvirt_util library and have all of the functions available. This would also make it possible to use VIR_FREE and friends (at which point, we should disable the syntax-check exceptions currently in effect on the python files).
I think I will do a preliminary patch, which does _just_ the makefile work to pull in the use of libvirt_util, then we can rebase this patch on top of that one. I know Alex Jia was also complaining about the inability to use normal libvirt conventions, because the Makefile wasn't yet set up for it, so this will be a good move overall.
+<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 numa tunable objects'/> Is th is type correct, or can it be any python dictionary type that maps valid numa tunable parameter names to values?
+<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='int' info='returns a dictionary of params in case of success, -1 in case of error'/> The return type should be a python object - a dictionary on success, PyNone on failure where libvirt populated an error message, or NULL on a python exception.
+++ b/python/libvirt-override.c @@ -21,6 +21,7 @@ #include "libvirt/virterror.h" #include "typewrappers.h" #include "libvirt.h" +#include "util/virtypedparam.h" Hmm, the rest of our code sets up INCLUDES so that we can use just "virtypedparam.h" instead of "util/virtypedparam.h"; another thing for me to do in pulling out the infrastructure into a preliminary patch.
#ifndef __CYGWIN__ extern void initlibvirtmod(void); @@ -61,6 +62,208 @@ static char *py_str(PyObject *obj) return PyString_AsString(str); }
+/* Two helper functions to help the conversions between C to Python + * for the virTypedParameter used in the following APIs. */ +static PyObject * +getPyVirTypedParameter(virTypedParameterPtr params, int nparams) +{ + PyObject *info; + PyObject *key, *val; + PyObject *ret = NULL; + int i; + + if (!params) + return ret;
If we return NULL, we should ensure that there is a valid python exception object ready for the caller to access. I'm thinking it might be better to mark this function with ATTRIBUTE_NONNULL(1) to avoid worrying about whether the caller has properly generated a python exception before passing us NULL.
Hi Eric I saw your comments about the nonnull attribute usage as follows http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308 I am not clear about it is still helpful to use it here? Guannan Ren
participants (3)
-
Alex Jia
-
Eric Blake
-
Guannan Ren