[libvirt] [PATCH v4] python: refactoring virTypedParameter conversion for NUMA tuning APIs
*getPyVirTypedParameter *setPyVirTypedParameter *virDomainSetNumaParameters *virDomainGetNumaParameters --- python/libvirt-override-api.xml | 13 ++ python/libvirt-override.c | 385 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 398 insertions(+), 0 deletions(-) diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index b2b8152..8eed0bb 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='str *' info='returns a dictionary of params in case of success, None 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 33a841d..3c6020b 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -22,6 +22,7 @@ #include "typewrappers.h" #include "libvirt.h" #include "memory.h" +#include "virtypedparam.h" #ifndef __CYGWIN__ extern void initlibvirtmod(void); @@ -62,6 +63,263 @@ 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 int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) +getPyVirTypedParameter(PyObject **info, + const virTypedParameterPtr params, int nparams) +{ + PyObject *key, *val; + int i; + + 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: + return 0; + } + + key = libvirt_constcharPtrWrap(params[i].field); + if (!key || !val) + goto cleanup; + + if (PyDict_SetItem(*info, key, val) < 0) + goto cleanup; + + Py_DECREF(key); + Py_DECREF(val); + } + return 1; +cleanup: + Py_XDECREF(key); + Py_XDECREF(val); + return -1; +} + +/* 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 virTypedParameterPtr ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) +setPyVirTypedParameter(PyObject *info, + const virTypedParameterPtr params, int nparams) +{ + PyObject *key, *value; + Py_ssize_t pos = 0; + virTypedParameterPtr temp = NULL, ret = NULL; + int size, i; + + if ((size = PyDict_Size(info)) < 0) + return NULL; + + if (VIR_ALLOC_N(ret, size) < 0) { + PyErr_NoMemory(); + return NULL; + } + + temp = &ret[0]; + while (PyDict_Next(info, &pos, &key, &value)) { + char *keystr = NULL; + bool found = false; + + if (PyString_Check(key)) { + if ((keystr = PyString_AsString(key)) == NULL) + goto cleanup; + } else { + PyErr_Format(PyExc_TypeError, + "Attribut name must be string"); + goto cleanup; + } + + for (i = 0; i < nparams; i++) { + if (STREQ(params[i].field, keystr)) { + found = true; + strncpy(temp->field, params[i].field, + sizeof(temp->field)); + temp->type = params[i].type; + + switch(params[i].type) { + case VIR_TYPED_PARAM_INT: + { + long long_val; + if (PyInt_Check(value)) { + long_val = PyInt_AsLong(value); + if ((long_val == -1) && PyErr_Occurred()) + goto cleanup; + } else { + PyErr_Format(PyExc_TypeError, + "The value type of " + "attribute \"%s\" must be int", keystr); + goto cleanup; + } + + if ((int)long_val == long_val) { + temp->value.i = (int)long_val; + } else { + PyErr_Format(PyExc_ValueError, + "The value of " + "attribute \"%s\" is out of int range", keystr); + goto cleanup; + } + } + break; + case VIR_TYPED_PARAM_UINT: + { + long long_val; + if (PyInt_Check(value)) { + long_val = PyInt_AsLong(value); + if ((long_val == -1) && PyErr_Occurred()) + goto cleanup; + } else { + PyErr_Format(PyExc_TypeError, + "The value type of " + "attribute \"%s\" must be int", keystr); + goto cleanup; + } + + if ((unsigned int)long_val == long_val) { + temp->value.ui = (unsigned int)long_val; + } else { + PyErr_Format(PyExc_ValueError, + "The value of " + "attribute \"%s\" is out of int range", keystr); + goto cleanup; + } + } + break; + case VIR_TYPED_PARAM_LLONG: + { + long long llong_val; + if (PyLong_Check(value)) { + llong_val = PyLong_AsLongLong(value); + if ((llong_val == -1) && PyErr_Occurred()) + goto cleanup; + } else { + PyErr_Format(PyExc_TypeError, + "The value type of " + "attribute \"%s\" must be long", keystr); + goto cleanup; + } + temp->value.l = llong_val; + } + break; + case VIR_TYPED_PARAM_ULLONG: + { + unsigned long long ullong_val; + if (PyLong_Check(value)) { + ullong_val = PyLong_AsUnsignedLongLong(value); + if ((ullong_val == -1) && PyErr_Occurred()) + goto cleanup; + } else { + PyErr_Format(PyExc_TypeError, + "The value type of " + "attribute \"%s\" must be long", keystr); + goto cleanup; + + } + temp->value.ul = ullong_val; + } + break; + case VIR_TYPED_PARAM_DOUBLE: + { + double double_val; + if (PyFloat_Check(value)) { + double_val = PyFloat_AsDouble(value); + if ((double_val == -1) && PyErr_Occurred()) + goto cleanup; + } else { + PyErr_Format(PyExc_TypeError, + "The value type of " + "attribute \"%s\" must be float", keystr); + goto cleanup; + } + temp->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(value)) { + PyObject *hacktrue = PyBool_FromLong(1); + temp->value.b = hacktrue == value ? 1: 0; + Py_DECREF(hacktrue); + } else { + PyErr_Format(PyExc_TypeError, + "The value type of " + "attribute \"%s\" must be bool", keystr); + goto cleanup; + } + } + break; + case VIR_TYPED_PARAM_STRING: + { + char *string_val; + if (PyString_Check(value)) { + if ((string_val = PyString_AsString(value)) == NULL) + goto cleanup; + if ((temp->value.s = strdup(string_val)) == NULL) { + PyErr_NoMemory(); + goto cleanup; + } + } else { + PyErr_Format(PyExc_TypeError, + "The value type of " + "attribute \"%s\" must be string", keystr); + goto cleanup; + } + } + break; + default: + goto cleanup; + } + } + } + + if (i == nparams && !found) { + PyErr_Format(PyExc_LookupError, + "Attribute name \"%s\" could not be recognized", keystr); + goto cleanup; + } + + temp++; + } + return ret; + +cleanup: + virTypedParameterArrayClear(ret, size); + VIR_FREE(ret); + return NULL; +} + /************************************************************************ * * * Statistics * @@ -1007,6 +1265,131 @@ 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, size = 0; + unsigned int flags; + virTypedParameterPtr params, new_params; + + if (!PyArg_ParseTuple(args, + (char *)"OOi:virDomainSetNumaParameters", + &pyobj_domain, &info, &flags)) + return NULL; + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + if ((size = PyDict_Size(info)) < 0) + return NULL; + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainGetNumaParameters(domain, NULL, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0 || !nparams) + return VIR_PY_INT_FAIL; + + if (VIR_ALLOC_N(params, nparams) < 0) + 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 cleanup; + } + + new_params = setPyVirTypedParameter(info, params, nparams); + if (!new_params) + goto cleanup; + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainSetNumaParameters(domain, new_params, size, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) { + ret = VIR_PY_INT_FAIL; + goto cleanup; + } + + ret = VIR_PY_INT_SUCCESS; + +cleanup: + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); + virTypedParameterArrayClear(new_params, size); + VIR_FREE(new_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); + + if ((info = PyDict_New()) == NULL) { + return ret; + } + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainGetNumaParameters(domain, NULL, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) + return VIR_PY_NONE; + + if (!nparams) + return info; + + if (VIR_ALLOC_N(params, nparams) < 0) + 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_NONE; + goto cleanup; + } + + i_retval = getPyVirTypedParameter(&info, params, nparams); + + if (i_retval < 0) { + Py_XDECREF(info); + goto cleanup; + } else if (i_retval == 0) { + Py_XDECREF(info); + ret = VIR_PY_NONE; + goto cleanup; + } else { + ret = info; + } + +cleanup: + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); + return ret; +} + +static PyObject * libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { virDomainPtr domain; @@ -5203,6 +5586,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
On 02/08/2012 05:20 AM, Guannan Ren wrote:
*getPyVirTypedParameter *setPyVirTypedParameter *virDomainSetNumaParameters *virDomainGetNumaParameters --- python/libvirt-override-api.xml | 13 ++ python/libvirt-override.c | 385 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 398 insertions(+), 0 deletions(-)
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index b2b8152..8eed0bb 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>
<info> is probably the most important element here; any <function> not in this file copies its in-python documentation verbatim from src/libvirt.c; but where we have a different python signature (such as here, because while the C code needs a user-allocated array and length, the python binding just takes a single python dictionary object), having only a 4-word documentation when doing 'help(libvirt.virDomain.setNumaParameters)' pales in comparison to what the C users will see. But I'm not too worried about this; you aren't the first person to do awfully short override text.
+ <function name='virDomainGetNumaParameters' file='python'> + <info>Get the NUMA parameters</info> + <return type='str *' info='returns a dictionary of params in case of success, None in case of error'/>
In fact, the more I've been playing with the python stuff in trying to review this patch, the more I've come to the conclusion that the <return> element of this XML is more or less ignored, and only the <info> element makes it anywhere that it might be useful. But let's focus on getting the functionality right before we worry about improving the documentation.
+/* Two helper functions to help the conversions between C to Python + * for the virTypedParameter used in the following APIs */
I'd document this function in isolation, since the next function is documented in isolation.
+static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) +getPyVirTypedParameter(PyObject **info,
I'd pass this as PyObject *info (no need for double-indirection, since you aren't updating the pointer, just the object that it points to). Either that, or offload the creation of info out of the caller into this function, and actually update the pointer. We'll see how the caller uses it, before I decide...
+ const virTypedParameterPtr params, int nparams) +{ + PyObject *key, *val; + int i; + + for (i = 0 ; i < nparams ; i++) { + switch (params[i].type) { + case VIR_TYPED_PARAM_INT: + val = PyInt_FromLong((long)params[i].value.i);
The cast is not necessary. (But it doesn't hurt, either).
+ 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);
Same for all these casts.
+ break; + + case VIR_TYPED_PARAM_STRING: + val = libvirt_constcharPtrWrap(params[i].value.s); + break; + + default: + return 0;
If we get here, we have a coding error. In fact, if we code things correctly, we can let the compiler tell us if a new VIR_TYPED_PARAM is ever added, where we need to cover it in our switch statement.
+ } + + key = libvirt_constcharPtrWrap(params[i].field); + if (!key || !val) + goto cleanup; + + if (PyDict_SetItem(*info, key, val) < 0) + goto cleanup; + + Py_DECREF(key); + Py_DECREF(val); + } + return 1;
I'd return 0 here, for consistency with the rest of libvirt conventions.
+cleanup: + Py_XDECREF(key); + Py_XDECREF(val); + return -1; +} + +/* 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 virTypedParameterPtr ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) +setPyVirTypedParameter(PyObject *info, + const virTypedParameterPtr params, int nparams) +{ + PyObject *key, *value; + Py_ssize_t pos = 0; + virTypedParameterPtr temp = NULL, ret = NULL; + int size, i; + + if ((size = PyDict_Size(info)) < 0) + return NULL;
Here, we should also reject a size of 0, with a python exception, since the setter APIs should not be called with a non-NULL array but a 0 size. We could also take a shortcut an fail if size > nparams, since that means at least one entry in the python dictionary is invalid; but erroring out now would not give a very nice message.
+ + if (VIR_ALLOC_N(ret, size) < 0) { + PyErr_NoMemory(); + return NULL; + } + + temp = &ret[0]; + while (PyDict_Next(info, &pos, &key, &value)) { + char *keystr = NULL; + bool found = false; + + if (PyString_Check(key)) { + if ((keystr = PyString_AsString(key)) == NULL) + goto cleanup; + } else { + PyErr_Format(PyExc_TypeError, + "Attribut name must be string");
s/Attribut/Attribute/
+ goto cleanup; + }
We can let python raise the exception on our behalf.
+ + for (i = 0; i < nparams; i++) { + if (STREQ(params[i].field, keystr)) {
Indentation got hard here. A little refactoring to reject unfound keys first makes life a bit easier.
+ found = true; + strncpy(temp->field, params[i].field, + sizeof(temp->field));
'make syntax-check' didn't like you: prohibit_strncpy python/libvirt-override.c:164: strncpy(temp->field, params[i].field, maint.mk: use virStrncpy, not strncpy
+ temp->type = params[i].type; + + switch(params[i].type) { + case VIR_TYPED_PARAM_INT: + { + long long_val; + if (PyInt_Check(value)) { + long_val = PyInt_AsLong(value); + if ((long_val == -1) && PyErr_Occurred()) + goto cleanup; + } else { + PyErr_Format(PyExc_TypeError, + "The value type of " + "attribute \"%s\" must be int", keystr); + goto cleanup; + }
I still think we can let python throw the exception for us.
+ + if ((int)long_val == long_val) { + temp->value.i = (int)long_val;
Useless cast on the second line. The first one is necessary, though.
+ case VIR_TYPED_PARAM_BOOLEAN: + { + /* Hack - Python's definition of Py_True breaks strict + * aliasing rules, so can't directly compare + */ + if (PyBool_Check(value)) { + PyObject *hacktrue = PyBool_FromLong(1); + temp->value.b = hacktrue == value ? 1: 0;
Formatting: space between ?: characters in the ternary operator.
+ Py_DECREF(hacktrue); + } else { + PyErr_Format(PyExc_TypeError, + "The value type of " + "attribute \"%s\" must be bool", keystr); + goto cleanup; + } + } + break; + case VIR_TYPED_PARAM_STRING: + { + char *string_val; + if (PyString_Check(value)) { + if ((string_val = PyString_AsString(value)) == NULL) + goto cleanup; + if ((temp->value.s = strdup(string_val)) == NULL) {
If we don't strdup() here, then we have one less point of allocation failure, and callers merely need remember to _not_ clear the returned array on success.
+ PyErr_NoMemory(); + goto cleanup; + } + } else { + PyErr_Format(PyExc_TypeError, + "The value type of " + "attribute \"%s\" must be string", keystr); + goto cleanup; + } + } + break; + default: + goto cleanup; + } + } + } + + if (i == nparams && !found) {
This is redundant - found will always be true iff we broke out of the for loop before setting i to nparams. I'm hoisting this condition earlier.
+ PyErr_Format(PyExc_LookupError, + "Attribute name \"%s\" could not be recognized", keystr); + goto cleanup; + } + + temp++; + } + return ret; + +cleanup: + virTypedParameterArrayClear(ret, size); + VIR_FREE(ret); + return NULL; +} + /************************************************************************ * * * Statistics * @@ -1007,6 +1265,131 @@ 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, size = 0; + unsigned int flags; + virTypedParameterPtr params, new_params; + + if (!PyArg_ParseTuple(args, + (char *)"OOi:virDomainSetNumaParameters", + &pyobj_domain, &info, &flags)) + return NULL; + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + if ((size = PyDict_Size(info)) < 0) + return NULL; + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainGetNumaParameters(domain, NULL, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0 || !nparams) + return VIR_PY_INT_FAIL;
If i_retval < 0, then libvirt has set an error message. But if nparams is 0, then we need to raise a message.
+ + if (VIR_ALLOC_N(params, nparams) < 0) + 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 cleanup; + } + + new_params = setPyVirTypedParameter(info, params, nparams); + if (!new_params) + goto cleanup; + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainSetNumaParameters(domain, new_params, size, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) { + ret = VIR_PY_INT_FAIL; + goto cleanup; + } + + ret = VIR_PY_INT_SUCCESS;
Looks good.
+ +cleanup: + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); + virTypedParameterArrayClear(new_params, size); + VIR_FREE(new_params);
See above comments about reducing malloc pressure; omitting strdup above means we can omit the array clear here.
+ 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); + + if ((info = PyDict_New()) == NULL) { + return ret; + }
Allocation...
+ + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainGetNumaParameters(domain, NULL, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) + return VIR_PY_NONE;
...and memory leak. I've made up my mind - it's better to have the helper function allocate info.
+ + if (!nparams) + return info;
Here, we can do a one-shot allocation, since we aren't going to reach the helper.
+ + if (VIR_ALLOC_N(params, nparams) < 0) + 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_NONE; + goto cleanup; + } + + i_retval = getPyVirTypedParameter(&info, params, nparams); + + if (i_retval < 0) { + Py_XDECREF(info); + goto cleanup;
If the helper allocates info on success, then the helper can also free it before returning failure, making our life easier. In fact, changing the helper's signature will make it even easier to use.
+ } else if (i_retval == 0) { + Py_XDECREF(info); + ret = VIR_PY_NONE; + goto cleanup;
Returning a 0-entry info is okay here.
+ } else { + ret = info; + } + +cleanup: + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); + return ret; +} + +static PyObject * libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { virDomainPtr domain; @@ -5203,6 +5586,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},
Here's the incremental diff I'm testing: diff --git i/python/libvirt-override.c w/python/libvirt-override.c index 3c6020b..d424f19 100644 --- i/python/libvirt-override.c +++ w/python/libvirt-override.c @@ -16,6 +16,9 @@ which has over 180 autoconf-style HAVE_* definitions. Shame on them. */ #undef HAVE_PTHREAD_H +/* We want to see *_LAST enums. */ +#define VIR_ENUM_SENTINELS + #include <Python.h> #include "libvirt/libvirt.h" #include "libvirt/virterror.h" @@ -23,6 +26,8 @@ #include "libvirt.h" #include "memory.h" #include "virtypedparam.h" +#include "ignore-value.h" +#include "util.h" #ifndef __CYGWIN__ extern void initlibvirtmod(void); @@ -63,46 +68,50 @@ 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 int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) -getPyVirTypedParameter(PyObject **info, - const virTypedParameterPtr params, int nparams) +/* Helper function to convert a virTypedParameter output array into a + * Python dictionary for return to the user. Return NULL on failure, + * after raising a python exception. */ +static PyObject * ATTRIBUTE_NONNULL(1) +getPyVirTypedParameter(const virTypedParameterPtr params, int nparams) { - PyObject *key, *val; + PyObject *key, *val, *info; int i; + if ((info = PyDict_New()) == NULL) + return NULL; + for (i = 0 ; i < nparams ; i++) { - switch (params[i].type) { + switch ((virTypedParameterType) params[i].type) { case VIR_TYPED_PARAM_INT: - val = PyInt_FromLong((long)params[i].value.i); + val = PyInt_FromLong(params[i].value.i); break; case VIR_TYPED_PARAM_UINT: - val = PyInt_FromLong((unsigned long)params[i].value.ui); + val = PyInt_FromLong(params[i].value.ui); break; case VIR_TYPED_PARAM_LLONG: - val = PyLong_FromLongLong((long long)params[i].value.l); + val = PyLong_FromLongLong(params[i].value.l); break; case VIR_TYPED_PARAM_ULLONG: - val = PyLong_FromUnsignedLongLong((unsigned long long)params[i].value.ul); + val = PyLong_FromUnsignedLongLong(params[i].value.ul); break; case VIR_TYPED_PARAM_DOUBLE: - val = PyFloat_FromDouble((double)params[i].value.d); + val = PyFloat_FromDouble(params[i].value.d); break; case VIR_TYPED_PARAM_BOOLEAN: - val = PyBool_FromLong((long)params[i].value.b); + val = PyBool_FromLong(params[i].value.b); break; case VIR_TYPED_PARAM_STRING: val = libvirt_constcharPtrWrap(params[i].value.s); break; - default: + case VIR_TYPED_PARAM_LAST: + /* Shouldn't get here */ return 0; } @@ -110,23 +119,28 @@ getPyVirTypedParameter(PyObject **info, if (!key || !val) goto cleanup; - if (PyDict_SetItem(*info, key, val) < 0) + if (PyDict_SetItem(info, key, val) < 0) { + Py_DECREF(info); goto cleanup; + } Py_DECREF(key); Py_DECREF(val); } - return 1; + return info; + cleanup: Py_XDECREF(key); Py_XDECREF(val); - return -1; + return NULL; } /* 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. */ + * hints on what types to use when creating the new array. The caller + * must NOT clear the array before freeing it, as it points into info + * rather than allocated strings. Return NULL on failure, after + * raising a python exception. */ static virTypedParameterPtr ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) setPyVirTypedParameter(PyObject *info, const virTypedParameterPtr params, int nparams) @@ -139,6 +153,13 @@ setPyVirTypedParameter(PyObject *info, if ((size = PyDict_Size(info)) < 0) return NULL; + /* Libvirt APIs use NULL array and 0 size as a special case; + * setting should have at least one parameter. */ + if (size == 0) { + PyErr_Format(PyExc_LookupError, "Dictionary must not be empty"); + return NULL; + } + if (VIR_ALLOC_N(ret, size) < 0) { PyErr_NoMemory(); return NULL; @@ -147,167 +168,108 @@ setPyVirTypedParameter(PyObject *info, temp = &ret[0]; while (PyDict_Next(info, &pos, &key, &value)) { char *keystr = NULL; - bool found = false; - if (PyString_Check(key)) { - if ((keystr = PyString_AsString(key)) == NULL) - goto cleanup; - } else { - PyErr_Format(PyExc_TypeError, - "Attribut name must be string"); + if ((keystr = PyString_AsString(key)) == NULL) goto cleanup; - } for (i = 0; i < nparams; i++) { - if (STREQ(params[i].field, keystr)) { - found = true; - strncpy(temp->field, params[i].field, - sizeof(temp->field)); - temp->type = params[i].type; - - switch(params[i].type) { - case VIR_TYPED_PARAM_INT: - { - long long_val; - if (PyInt_Check(value)) { - long_val = PyInt_AsLong(value); - if ((long_val == -1) && PyErr_Occurred()) - goto cleanup; - } else { - PyErr_Format(PyExc_TypeError, - "The value type of " - "attribute \"%s\" must be int", keystr); - goto cleanup; - } - - if ((int)long_val == long_val) { - temp->value.i = (int)long_val; - } else { - PyErr_Format(PyExc_ValueError, - "The value of " - "attribute \"%s\" is out of int range", keystr); - goto cleanup; - } - } - break; - case VIR_TYPED_PARAM_UINT: - { - long long_val; - if (PyInt_Check(value)) { - long_val = PyInt_AsLong(value); - if ((long_val == -1) && PyErr_Occurred()) - goto cleanup; - } else { - PyErr_Format(PyExc_TypeError, - "The value type of " - "attribute \"%s\" must be int", keystr); - goto cleanup; - } - - if ((unsigned int)long_val == long_val) { - temp->value.ui = (unsigned int)long_val; - } else { - PyErr_Format(PyExc_ValueError, - "The value of " - "attribute \"%s\" is out of int range", keystr); - goto cleanup; - } - } - break; - case VIR_TYPED_PARAM_LLONG: - { - long long llong_val; - if (PyLong_Check(value)) { - llong_val = PyLong_AsLongLong(value); - if ((llong_val == -1) && PyErr_Occurred()) - goto cleanup; - } else { - PyErr_Format(PyExc_TypeError, - "The value type of " - "attribute \"%s\" must be long", keystr); - goto cleanup; - } - temp->value.l = llong_val; - } - break; - case VIR_TYPED_PARAM_ULLONG: - { - unsigned long long ullong_val; - if (PyLong_Check(value)) { - ullong_val = PyLong_AsUnsignedLongLong(value); - if ((ullong_val == -1) && PyErr_Occurred()) - goto cleanup; - } else { - PyErr_Format(PyExc_TypeError, - "The value type of " - "attribute \"%s\" must be long", keystr); - goto cleanup; - - } - temp->value.ul = ullong_val; - } - break; - case VIR_TYPED_PARAM_DOUBLE: - { - double double_val; - if (PyFloat_Check(value)) { - double_val = PyFloat_AsDouble(value); - if ((double_val == -1) && PyErr_Occurred()) - goto cleanup; - } else { - PyErr_Format(PyExc_TypeError, - "The value type of " - "attribute \"%s\" must be float", keystr); - goto cleanup; - } - temp->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(value)) { - PyObject *hacktrue = PyBool_FromLong(1); - temp->value.b = hacktrue == value ? 1: 0; - Py_DECREF(hacktrue); - } else { - PyErr_Format(PyExc_TypeError, - "The value type of " - "attribute \"%s\" must be bool", keystr); - goto cleanup; - } - } - break; - case VIR_TYPED_PARAM_STRING: - { - char *string_val; - if (PyString_Check(value)) { - if ((string_val = PyString_AsString(value)) == NULL) - goto cleanup; - if ((temp->value.s = strdup(string_val)) == NULL) { - PyErr_NoMemory(); - goto cleanup; - } - } else { - PyErr_Format(PyExc_TypeError, - "The value type of " - "attribute \"%s\" must be string", keystr); - goto cleanup; - } - } - break; - default: - goto cleanup; - } - } + if (STREQ(params[i].field, keystr)) + break; } + if (i == nparams) { + PyErr_Format(PyExc_LookupError, + "Attribute name \"%s\" could not be recognized", + keystr); + goto cleanup; + } + + ignore_value(virStrcpyStatic(temp->field, keystr)); + temp->type = params[i].type; - if (i == nparams && !found) { - PyErr_Format(PyExc_LookupError, - "Attribute name \"%s\" could not be recognized", keystr); + switch((virTypedParameterType) params[i].type) { + case VIR_TYPED_PARAM_INT: + { + long long_val = PyInt_AsLong(value); + if ((long_val == -1) && PyErr_Occurred()) + goto cleanup; + if ((int)long_val == long_val) { + temp->value.i = long_val; + } else { + PyErr_Format(PyExc_ValueError, + "The value of " + "attribute \"%s\" is out of int range", keystr); + goto cleanup; + } + } + break; + case VIR_TYPED_PARAM_UINT: + { + long long_val = PyInt_AsLong(value); + if ((long_val == -1) && PyErr_Occurred()) + goto cleanup; + if ((unsigned int)long_val == long_val) { + temp->value.ui = long_val; + } else { + PyErr_Format(PyExc_ValueError, + "The value of " + "attribute \"%s\" is out of int range", keystr); + goto cleanup; + } + } + break; + case VIR_TYPED_PARAM_LLONG: + { + long long llong_val = PyLong_AsLongLong(value); + if ((llong_val == -1) && PyErr_Occurred()) goto cleanup; + temp->value.l = llong_val; + } + break; + case VIR_TYPED_PARAM_ULLONG: + { + unsigned long long ullong_val = PyLong_AsUnsignedLongLong(value); + if ((ullong_val == -1) && PyErr_Occurred()) + goto cleanup; + temp->value.ul = ullong_val; + } + break; + case VIR_TYPED_PARAM_DOUBLE: + { + double double_val = PyFloat_AsDouble(value); + if ((double_val == -1) && PyErr_Occurred()) + goto cleanup; + temp->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(value)) { + PyObject *hacktrue = PyBool_FromLong(1); + temp->value.b = hacktrue == value ? 1 : 0; + Py_DECREF(hacktrue); + } else { + PyErr_Format(PyExc_TypeError, + "The value type of " + "attribute \"%s\" must be bool", keystr); + goto cleanup; + } + } + break; + case VIR_TYPED_PARAM_STRING: + { + char *string_val = PyString_AsString(value); + if (!string_val) + goto cleanup; + temp->value.s = string_val; + break; + } + + case VIR_TYPED_PARAM_LAST: + /* Shouldn't get here */ + goto cleanup; } temp++; @@ -315,7 +277,6 @@ setPyVirTypedParameter(PyObject *info, return ret; cleanup: - virTypedParameterArrayClear(ret, size); VIR_FREE(ret); return NULL; } @@ -1289,9 +1250,15 @@ libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, i_retval = virDomainGetNumaParameters(domain, NULL, &nparams, flags); LIBVIRT_END_ALLOW_THREADS; - if (i_retval < 0 || !nparams) + if (i_retval < 0) return VIR_PY_INT_FAIL; + if (size == 0) { + PyErr_Format(PyExc_LookupError, + "Domain has no settable attributes"); + return NULL; + } + if (VIR_ALLOC_N(params, nparams) < 0) return PyErr_NoMemory(); @@ -1322,7 +1289,6 @@ libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, cleanup: virTypedParameterArrayClear(params, nparams); VIR_FREE(params); - virTypedParameterArrayClear(new_params, size); VIR_FREE(new_params); return ret; } @@ -1332,7 +1298,7 @@ libvirt_virDomainGetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { virDomainPtr domain; - PyObject *pyobj_domain, *info; + PyObject *pyobj_domain; PyObject *ret = NULL; int i_retval; int nparams = 0; @@ -1344,10 +1310,6 @@ libvirt_virDomainGetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, return NULL; domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); - if ((info = PyDict_New()) == NULL) { - return ret; - } - LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetNumaParameters(domain, NULL, &nparams, flags); LIBVIRT_END_ALLOW_THREADS; @@ -1356,7 +1318,7 @@ libvirt_virDomainGetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, return VIR_PY_NONE; if (!nparams) - return info; + return PyDict_New(); if (VIR_ALLOC_N(params, nparams) < 0) return PyErr_NoMemory(); @@ -1370,18 +1332,7 @@ libvirt_virDomainGetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, goto cleanup; } - i_retval = getPyVirTypedParameter(&info, params, nparams); - - if (i_retval < 0) { - Py_XDECREF(info); - goto cleanup; - } else if (i_retval == 0) { - Py_XDECREF(info); - ret = VIR_PY_NONE; - goto cleanup; - } else { - ret = info; - } + ret = getPyVirTypedParameter(params, nparams); cleanup: virTypedParameterArrayClear(params, nparams); -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: Guannan Ren <gren@redhat.com> *getPyVirTypedParameter *setPyVirTypedParameter *virDomainSetNumaParameters *virDomainGetNumaParameters Signed-off-by: Eric Blake <eblake@redhat.com> --- v5: Incorporate my review comments on v4 python/libvirt-override-api.xml | 13 ++ python/libvirt-override.c | 336 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 349 insertions(+), 0 deletions(-) diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index b2b8152..8eed0bb 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='str *' info='returns a dictionary of params in case of success, None 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 33a841d..d424f19 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -16,12 +16,18 @@ which has over 180 autoconf-style HAVE_* definitions. Shame on them. */ #undef HAVE_PTHREAD_H +/* We want to see *_LAST enums. */ +#define VIR_ENUM_SENTINELS + #include <Python.h> #include "libvirt/libvirt.h" #include "libvirt/virterror.h" #include "typewrappers.h" #include "libvirt.h" #include "memory.h" +#include "virtypedparam.h" +#include "ignore-value.h" +#include "util.h" #ifndef __CYGWIN__ extern void initlibvirtmod(void); @@ -62,6 +68,219 @@ static char *py_str(PyObject *obj) return PyString_AsString(str); } +/* Helper function to convert a virTypedParameter output array into a + * Python dictionary for return to the user. Return NULL on failure, + * after raising a python exception. */ +static PyObject * ATTRIBUTE_NONNULL(1) +getPyVirTypedParameter(const virTypedParameterPtr params, int nparams) +{ + PyObject *key, *val, *info; + int i; + + if ((info = PyDict_New()) == NULL) + return NULL; + + for (i = 0 ; i < nparams ; i++) { + switch ((virTypedParameterType) params[i].type) { + case VIR_TYPED_PARAM_INT: + val = PyInt_FromLong(params[i].value.i); + break; + + case VIR_TYPED_PARAM_UINT: + val = PyInt_FromLong(params[i].value.ui); + break; + + case VIR_TYPED_PARAM_LLONG: + val = PyLong_FromLongLong(params[i].value.l); + break; + + case VIR_TYPED_PARAM_ULLONG: + val = PyLong_FromUnsignedLongLong(params[i].value.ul); + break; + + case VIR_TYPED_PARAM_DOUBLE: + val = PyFloat_FromDouble(params[i].value.d); + break; + + case VIR_TYPED_PARAM_BOOLEAN: + val = PyBool_FromLong(params[i].value.b); + break; + + case VIR_TYPED_PARAM_STRING: + val = libvirt_constcharPtrWrap(params[i].value.s); + break; + + case VIR_TYPED_PARAM_LAST: + /* Shouldn't get here */ + return 0; + } + + key = libvirt_constcharPtrWrap(params[i].field); + if (!key || !val) + goto cleanup; + + if (PyDict_SetItem(info, key, val) < 0) { + Py_DECREF(info); + goto cleanup; + } + + Py_DECREF(key); + Py_DECREF(val); + } + return info; + +cleanup: + Py_XDECREF(key); + Py_XDECREF(val); + return NULL; +} + +/* 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. The caller + * must NOT clear the array before freeing it, as it points into info + * rather than allocated strings. Return NULL on failure, after + * raising a python exception. */ +static virTypedParameterPtr ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) +setPyVirTypedParameter(PyObject *info, + const virTypedParameterPtr params, int nparams) +{ + PyObject *key, *value; + Py_ssize_t pos = 0; + virTypedParameterPtr temp = NULL, ret = NULL; + int size, i; + + if ((size = PyDict_Size(info)) < 0) + return NULL; + + /* Libvirt APIs use NULL array and 0 size as a special case; + * setting should have at least one parameter. */ + if (size == 0) { + PyErr_Format(PyExc_LookupError, "Dictionary must not be empty"); + return NULL; + } + + if (VIR_ALLOC_N(ret, size) < 0) { + PyErr_NoMemory(); + return NULL; + } + + temp = &ret[0]; + while (PyDict_Next(info, &pos, &key, &value)) { + char *keystr = NULL; + + if ((keystr = PyString_AsString(key)) == NULL) + goto cleanup; + + for (i = 0; i < nparams; i++) { + if (STREQ(params[i].field, keystr)) + break; + } + if (i == nparams) { + PyErr_Format(PyExc_LookupError, + "Attribute name \"%s\" could not be recognized", + keystr); + goto cleanup; + } + + ignore_value(virStrcpyStatic(temp->field, keystr)); + temp->type = params[i].type; + + switch((virTypedParameterType) params[i].type) { + case VIR_TYPED_PARAM_INT: + { + long long_val = PyInt_AsLong(value); + if ((long_val == -1) && PyErr_Occurred()) + goto cleanup; + if ((int)long_val == long_val) { + temp->value.i = long_val; + } else { + PyErr_Format(PyExc_ValueError, + "The value of " + "attribute \"%s\" is out of int range", keystr); + goto cleanup; + } + } + break; + case VIR_TYPED_PARAM_UINT: + { + long long_val = PyInt_AsLong(value); + if ((long_val == -1) && PyErr_Occurred()) + goto cleanup; + if ((unsigned int)long_val == long_val) { + temp->value.ui = long_val; + } else { + PyErr_Format(PyExc_ValueError, + "The value of " + "attribute \"%s\" is out of int range", keystr); + goto cleanup; + } + } + break; + case VIR_TYPED_PARAM_LLONG: + { + long long llong_val = PyLong_AsLongLong(value); + if ((llong_val == -1) && PyErr_Occurred()) + goto cleanup; + temp->value.l = llong_val; + } + break; + case VIR_TYPED_PARAM_ULLONG: + { + unsigned long long ullong_val = PyLong_AsUnsignedLongLong(value); + if ((ullong_val == -1) && PyErr_Occurred()) + goto cleanup; + temp->value.ul = ullong_val; + } + break; + case VIR_TYPED_PARAM_DOUBLE: + { + double double_val = PyFloat_AsDouble(value); + if ((double_val == -1) && PyErr_Occurred()) + goto cleanup; + temp->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(value)) { + PyObject *hacktrue = PyBool_FromLong(1); + temp->value.b = hacktrue == value ? 1 : 0; + Py_DECREF(hacktrue); + } else { + PyErr_Format(PyExc_TypeError, + "The value type of " + "attribute \"%s\" must be bool", keystr); + goto cleanup; + } + } + break; + case VIR_TYPED_PARAM_STRING: + { + char *string_val = PyString_AsString(value); + if (!string_val) + goto cleanup; + temp->value.s = string_val; + break; + } + + case VIR_TYPED_PARAM_LAST: + /* Shouldn't get here */ + goto cleanup; + } + + temp++; + } + return ret; + +cleanup: + VIR_FREE(ret); + return NULL; +} + /************************************************************************ * * * Statistics * @@ -1007,6 +1226,121 @@ 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, size = 0; + unsigned int flags; + virTypedParameterPtr params, new_params; + + if (!PyArg_ParseTuple(args, + (char *)"OOi:virDomainSetNumaParameters", + &pyobj_domain, &info, &flags)) + return NULL; + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + if ((size = PyDict_Size(info)) < 0) + return NULL; + + 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 (size == 0) { + PyErr_Format(PyExc_LookupError, + "Domain has no settable attributes"); + return NULL; + } + + if (VIR_ALLOC_N(params, nparams) < 0) + 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 cleanup; + } + + new_params = setPyVirTypedParameter(info, params, nparams); + if (!new_params) + goto cleanup; + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainSetNumaParameters(domain, new_params, size, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval < 0) { + ret = VIR_PY_INT_FAIL; + goto cleanup; + } + + ret = VIR_PY_INT_SUCCESS; + +cleanup: + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); + VIR_FREE(new_params); + return ret; +} + +static PyObject * +libvirt_virDomainGetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + virDomainPtr domain; + PyObject *pyobj_domain; + 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_NONE; + + if (!nparams) + return PyDict_New(); + + if (VIR_ALLOC_N(params, nparams) < 0) + 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_NONE; + goto cleanup; + } + + ret = getPyVirTypedParameter(params, nparams); + +cleanup: + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); + return ret; +} + +static PyObject * libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { virDomainPtr domain; @@ -5203,6 +5537,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.6
On 2012年02月09日 09:41, Eric Blake wrote:
From: Guannan Ren<gren@redhat.com>
*getPyVirTypedParameter *setPyVirTypedParameter *virDomainSetNumaParameters *virDomainGetNumaParameters
Signed-off-by: Eric Blake<eblake@redhat.com> ---
v5: Incorporate my review comments on v4
python/libvirt-override-api.xml | 13 ++ python/libvirt-override.c | 336 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 349 insertions(+), 0 deletions(-)
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index b2b8152..8eed0bb 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='str *' info='returns a dictionary of params in case of success, None 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 33a841d..d424f19 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -16,12 +16,18 @@ which has over 180 autoconf-style HAVE_* definitions. Shame on them. */ #undef HAVE_PTHREAD_H
+/* We want to see *_LAST enums. */ +#define VIR_ENUM_SENTINELS + #include<Python.h> #include "libvirt/libvirt.h" #include "libvirt/virterror.h" #include "typewrappers.h" #include "libvirt.h" #include "memory.h" +#include "virtypedparam.h" +#include "ignore-value.h" +#include "util.h"
#ifndef __CYGWIN__ extern void initlibvirtmod(void); @@ -62,6 +68,219 @@ static char *py_str(PyObject *obj) return PyString_AsString(str); }
+/* Helper function to convert a virTypedParameter output array into a + * Python dictionary for return to the user. Return NULL on failure, + * after raising a python exception. */ +static PyObject * ATTRIBUTE_NONNULL(1) +getPyVirTypedParameter(const virTypedParameterPtr params, int nparams) +{ + PyObject *key, *val, *info; + int i; + + if ((info = PyDict_New()) == NULL) + return NULL; + + for (i = 0 ; i< nparams ; i++) { + switch ((virTypedParameterType) params[i].type) { + case VIR_TYPED_PARAM_INT: + val = PyInt_FromLong(params[i].value.i); + break; + + case VIR_TYPED_PARAM_UINT: + val = PyInt_FromLong(params[i].value.ui); + break; + + case VIR_TYPED_PARAM_LLONG: + val = PyLong_FromLongLong(params[i].value.l); + break; + + case VIR_TYPED_PARAM_ULLONG: + val = PyLong_FromUnsignedLongLong(params[i].value.ul); + break; + + case VIR_TYPED_PARAM_DOUBLE: + val = PyFloat_FromDouble(params[i].value.d); + break; + + case VIR_TYPED_PARAM_BOOLEAN: + val = PyBool_FromLong(params[i].value.b); + break; + + case VIR_TYPED_PARAM_STRING: + val = libvirt_constcharPtrWrap(params[i].value.s); + break; + + case VIR_TYPED_PARAM_LAST: + /* Shouldn't get here */ + return 0; + } + + key = libvirt_constcharPtrWrap(params[i].field); + if (!key || !val) + goto cleanup; + + if (PyDict_SetItem(info, key, val)< 0) { + Py_DECREF(info); + goto cleanup; + } + + Py_DECREF(key); + Py_DECREF(val); + } + return info; + +cleanup: + Py_XDECREF(key); + Py_XDECREF(val); + return NULL; +} + +/* 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. The caller + * must NOT clear the array before freeing it, as it points into info + * rather than allocated strings. Return NULL on failure, after + * raising a python exception. */ +static virTypedParameterPtr ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) +setPyVirTypedParameter(PyObject *info, + const virTypedParameterPtr params, int nparams) +{ + PyObject *key, *value; + Py_ssize_t pos = 0; + virTypedParameterPtr temp = NULL, ret = NULL; + int size, i; + + if ((size = PyDict_Size(info))< 0) + return NULL; + + /* Libvirt APIs use NULL array and 0 size as a special case; + * setting should have at least one parameter. */ + if (size == 0) { + PyErr_Format(PyExc_LookupError, "Dictionary must not be empty"); + return NULL; + } + + if (VIR_ALLOC_N(ret, size)< 0) { + PyErr_NoMemory(); + return NULL; + } + + temp =&ret[0]; + while (PyDict_Next(info,&pos,&key,&value)) { + char *keystr = NULL; + + if ((keystr = PyString_AsString(key)) == NULL) + goto cleanup; + + for (i = 0; i< nparams; i++) { + if (STREQ(params[i].field, keystr)) + break; + } + if (i == nparams) { + PyErr_Format(PyExc_LookupError, + "Attribute name \"%s\" could not be recognized", + keystr); + goto cleanup; + } + + ignore_value(virStrcpyStatic(temp->field, keystr)); + temp->type = params[i].type; + + switch((virTypedParameterType) params[i].type) { + case VIR_TYPED_PARAM_INT: + { + long long_val = PyInt_AsLong(value); + if ((long_val == -1)&& PyErr_Occurred()) + goto cleanup; + if ((int)long_val == long_val) { + temp->value.i = long_val; + } else { + PyErr_Format(PyExc_ValueError, + "The value of " + "attribute \"%s\" is out of int range", keystr); + goto cleanup; + } + } + break; + case VIR_TYPED_PARAM_UINT: + { + long long_val = PyInt_AsLong(value); + if ((long_val == -1)&& PyErr_Occurred()) + goto cleanup; + if ((unsigned int)long_val == long_val) { + temp->value.ui = long_val; + } else { + PyErr_Format(PyExc_ValueError, + "The value of " + "attribute \"%s\" is out of int range", keystr); + goto cleanup; + } + } + break; + case VIR_TYPED_PARAM_LLONG: + { + long long llong_val = PyLong_AsLongLong(value); + if ((llong_val == -1)&& PyErr_Occurred()) + goto cleanup; + temp->value.l = llong_val; + } + break; + case VIR_TYPED_PARAM_ULLONG: + { + unsigned long long ullong_val = PyLong_AsUnsignedLongLong(value); + if ((ullong_val == -1)&& PyErr_Occurred()) + goto cleanup; + temp->value.ul = ullong_val; + } + break; + case VIR_TYPED_PARAM_DOUBLE: + { + double double_val = PyFloat_AsDouble(value); + if ((double_val == -1)&& PyErr_Occurred()) + goto cleanup; + temp->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(value)) { + PyObject *hacktrue = PyBool_FromLong(1); + temp->value.b = hacktrue == value ? 1 : 0; + Py_DECREF(hacktrue); + } else { + PyErr_Format(PyExc_TypeError, + "The value type of " + "attribute \"%s\" must be bool", keystr); + goto cleanup; + } + } + break; + case VIR_TYPED_PARAM_STRING: + { + char *string_val = PyString_AsString(value); + if (!string_val) + goto cleanup; + temp->value.s = string_val; + break; + } + + case VIR_TYPED_PARAM_LAST: + /* Shouldn't get here */ + goto cleanup; + } + + temp++; + } + return ret; + +cleanup: + VIR_FREE(ret); + return NULL; +} + /************************************************************************ * * * Statistics * @@ -1007,6 +1226,121 @@ 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, size = 0; + unsigned int flags; + virTypedParameterPtr params, new_params; + + if (!PyArg_ParseTuple(args, + (char *)"OOi:virDomainSetNumaParameters", +&pyobj_domain,&info,&flags)) + return NULL; + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + if ((size = PyDict_Size(info))< 0) + return NULL; + + 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 (size == 0) { + PyErr_Format(PyExc_LookupError, + "Domain has no settable attributes"); + return NULL; + } + + if (VIR_ALLOC_N(params, nparams)< 0) + 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 cleanup; + } + + new_params = setPyVirTypedParameter(info, params, nparams); + if (!new_params) + goto cleanup; + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainSetNumaParameters(domain, new_params, size, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval< 0) { + ret = VIR_PY_INT_FAIL; + goto cleanup; + } + + ret = VIR_PY_INT_SUCCESS; + +cleanup: + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); + VIR_FREE(new_params); + return ret; +} + +static PyObject * +libvirt_virDomainGetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + virDomainPtr domain; + PyObject *pyobj_domain; + 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_NONE; + + if (!nparams) + return PyDict_New(); + + if (VIR_ALLOC_N(params, nparams)< 0) + 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_NONE; + goto cleanup; + } + + ret = getPyVirTypedParameter(params, nparams); + +cleanup: + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); + return ret; +} + +static PyObject * libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { virDomainPtr domain; @@ -5203,6 +5537,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},
ACK Osier
On 02/09/2012 09:41 AM, Eric Blake wrote:
From: Guannan Ren<gren@redhat.com>
*getPyVirTypedParameter *setPyVirTypedParameter *virDomainSetNumaParameters *virDomainGetNumaParameters
Signed-off-by: Eric Blake<eblake@redhat.com> ---
v5: Incorporate my review comments on v4
python/libvirt-override-api.xml | 13 ++ python/libvirt-override.c | 336 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 349 insertions(+), 0 deletions(-)
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index b2b8152..8eed0bb 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='str *' info='returns a dictionary of params in case of success, None 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 33a841d..d424f19 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -16,12 +16,18 @@ which has over 180 autoconf-style HAVE_* definitions. Shame on them. */ #undef HAVE_PTHREAD_H
+/* We want to see *_LAST enums. */ +#define VIR_ENUM_SENTINELS + #include<Python.h> #include "libvirt/libvirt.h" #include "libvirt/virterror.h" #include "typewrappers.h" #include "libvirt.h" #include "memory.h" +#include "virtypedparam.h" +#include "ignore-value.h" +#include "util.h"
#ifndef __CYGWIN__ extern void initlibvirtmod(void); @@ -62,6 +68,219 @@ static char *py_str(PyObject *obj) return PyString_AsString(str); }
+/* Helper function to convert a virTypedParameter output array into a + * Python dictionary for return to the user. Return NULL on failure, + * after raising a python exception. */ +static PyObject * ATTRIBUTE_NONNULL(1) +getPyVirTypedParameter(const virTypedParameterPtr params, int nparams) +{ + PyObject *key, *val, *info; + int i; + + if ((info = PyDict_New()) == NULL) + return NULL; + + for (i = 0 ; i< nparams ; i++) { + switch ((virTypedParameterType) params[i].type) { + case VIR_TYPED_PARAM_INT: + val = PyInt_FromLong(params[i].value.i); + break; + + case VIR_TYPED_PARAM_UINT: + val = PyInt_FromLong(params[i].value.ui); + break; + + case VIR_TYPED_PARAM_LLONG: + val = PyLong_FromLongLong(params[i].value.l); + break; + + case VIR_TYPED_PARAM_ULLONG: + val = PyLong_FromUnsignedLongLong(params[i].value.ul); + break; + + case VIR_TYPED_PARAM_DOUBLE: + val = PyFloat_FromDouble(params[i].value.d); + break; + + case VIR_TYPED_PARAM_BOOLEAN: + val = PyBool_FromLong(params[i].value.b); + break; + + case VIR_TYPED_PARAM_STRING: + val = libvirt_constcharPtrWrap(params[i].value.s); + break; + + case VIR_TYPED_PARAM_LAST: + /* Shouldn't get here */ + return 0; + }
The effect of return 0 is the same as return NULL that will trigger the exception if defined before. Otherwise if the exception is not defined, the exception is as follows: "System Error: error return without exception set" In the case of having compiler to check out, it's ok here.
+ + key = libvirt_constcharPtrWrap(params[i].field); + if (!key || !val) + goto cleanup; + + if (PyDict_SetItem(info, key, val)< 0) { + Py_DECREF(info); + goto cleanup; + } + + Py_DECREF(key); + Py_DECREF(val); + } + return info; + +cleanup: + Py_XDECREF(key); + Py_XDECREF(val); + return NULL; +} + +/* 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. The caller + * must NOT clear the array before freeing it, as it points into info + * rather than allocated strings. Return NULL on failure, after + * raising a python exception. */ +static virTypedParameterPtr ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) +setPyVirTypedParameter(PyObject *info, + const virTypedParameterPtr params, int nparams) +{ + PyObject *key, *value; + Py_ssize_t pos = 0; + virTypedParameterPtr temp = NULL, ret = NULL; + int size, i; + + if ((size = PyDict_Size(info))< 0) + return NULL; + + /* Libvirt APIs use NULL array and 0 size as a special case; + * setting should have at least one parameter. */ + if (size == 0) { + PyErr_Format(PyExc_LookupError, "Dictionary must not be empty"); + return NULL; + } + + if (VIR_ALLOC_N(ret, size)< 0) { + PyErr_NoMemory(); + return NULL; + } + + temp =&ret[0]; + while (PyDict_Next(info,&pos,&key,&value)) { + char *keystr = NULL; + + if ((keystr = PyString_AsString(key)) == NULL) + goto cleanup; + + for (i = 0; i< nparams; i++) { + if (STREQ(params[i].field, keystr)) + break; + } + if (i == nparams) { + PyErr_Format(PyExc_LookupError, + "Attribute name \"%s\" could not be recognized", + keystr); + goto cleanup; + } + + ignore_value(virStrcpyStatic(temp->field, keystr)); + temp->type = params[i].type; + + switch((virTypedParameterType) params[i].type) { + case VIR_TYPED_PARAM_INT: + { + long long_val = PyInt_AsLong(value); + if ((long_val == -1)&& PyErr_Occurred()) + goto cleanup; + if ((int)long_val == long_val) { + temp->value.i = long_val; + } else { + PyErr_Format(PyExc_ValueError, + "The value of " + "attribute \"%s\" is out of int range", keystr); + goto cleanup; + } + } + break; + case VIR_TYPED_PARAM_UINT: + { + long long_val = PyInt_AsLong(value); + if ((long_val == -1)&& PyErr_Occurred()) + goto cleanup; + if ((unsigned int)long_val == long_val) { + temp->value.ui = long_val; + } else { + PyErr_Format(PyExc_ValueError, + "The value of " + "attribute \"%s\" is out of int range", keystr); + goto cleanup; + } + } + break; + case VIR_TYPED_PARAM_LLONG: + { + long long llong_val = PyLong_AsLongLong(value); + if ((llong_val == -1)&& PyErr_Occurred()) + goto cleanup; + temp->value.l = llong_val; + } + break; + case VIR_TYPED_PARAM_ULLONG: + { + unsigned long long ullong_val = PyLong_AsUnsignedLongLong(value); + if ((ullong_val == -1)&& PyErr_Occurred()) + goto cleanup; + temp->value.ul = ullong_val; + } + break; + case VIR_TYPED_PARAM_DOUBLE: + { + double double_val = PyFloat_AsDouble(value); + if ((double_val == -1)&& PyErr_Occurred()) + goto cleanup; + temp->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(value)) { + PyObject *hacktrue = PyBool_FromLong(1); + temp->value.b = hacktrue == value ? 1 : 0; + Py_DECREF(hacktrue); + } else { + PyErr_Format(PyExc_TypeError, + "The value type of " + "attribute \"%s\" must be bool", keystr); + goto cleanup; + } + } + break; + case VIR_TYPED_PARAM_STRING: + { + char *string_val = PyString_AsString(value); + if (!string_val) + goto cleanup; + temp->value.s = string_val; + break; + } + + case VIR_TYPED_PARAM_LAST: + /* Shouldn't get here */ + goto cleanup; + } + + temp++; + } + return ret; + +cleanup: + VIR_FREE(ret); + return NULL; +} + /************************************************************************ * * * Statistics * @@ -1007,6 +1226,121 @@ 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, size = 0; + unsigned int flags; + virTypedParameterPtr params, new_params; + + if (!PyArg_ParseTuple(args, + (char *)"OOi:virDomainSetNumaParameters", +&pyobj_domain,&info,&flags)) + return NULL; + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + if ((size = PyDict_Size(info))< 0) + return NULL; + + 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 (size == 0) { + PyErr_Format(PyExc_LookupError, + "Domain has no settable attributes"); + return NULL; + }
A typo, "if (nparams == 0)"
+ + if (VIR_ALLOC_N(params, nparams)< 0) + 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 cleanup; + } + + new_params = setPyVirTypedParameter(info, params, nparams); + if (!new_params) + goto cleanup; + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainSetNumaParameters(domain, new_params, size, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (i_retval< 0) { + ret = VIR_PY_INT_FAIL; + goto cleanup; + } + + ret = VIR_PY_INT_SUCCESS; + +cleanup: + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); + VIR_FREE(new_params); + return ret; +} + +static PyObject * +libvirt_virDomainGetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + virDomainPtr domain; + PyObject *pyobj_domain; + 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_NONE; + + if (!nparams) + return PyDict_New(); + + if (VIR_ALLOC_N(params, nparams)< 0) + 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_NONE; + goto cleanup; + } + + ret = getPyVirTypedParameter(params, nparams); + +cleanup: + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); + return ret; +} + +static PyObject * libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { virDomainPtr domain; @@ -5203,6 +5537,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},
Thanks for these comments, ACK. Guannan Ren
On 02/08/2012 09:29 PM, Guannan Ren wrote:
On 02/09/2012 09:41 AM, Eric Blake wrote:
From: Guannan Ren<gren@redhat.com>
*getPyVirTypedParameter *setPyVirTypedParameter *virDomainSetNumaParameters *virDomainGetNumaParameters
Signed-off-by: Eric Blake<eblake@redhat.com> ---
v5: Incorporate my review comments on v4
+ for (i = 0 ; i< nparams ; i++) { + switch ((virTypedParameterType) params[i].type) { + case VIR_TYPED_PARAM_INT: + val = PyInt_FromLong(params[i].value.i); + break; +
+ + case VIR_TYPED_PARAM_LAST: + /* Shouldn't get here */ + return 0; + }
The effect of return 0 is the same as return NULL that will trigger the exception if defined before. Otherwise if the exception is not defined, the exception is as follows: "System Error: error return without exception set" In the case of having compiler to check out, it's ok here.
Hmm; originally, I was returning 0 on success and -1 on failure, then I changed the signature to return NULL on failure and object on success, but not this line. 0 acts like NULL, but it would be better written as NULL. At any rate, my trick of doing switch ((virTypedParameterType) params[i].type) will ensure that at least gcc, with warnings, will warn us if we ever miss any cases known at compile time. Conversely, if we are talking to a newer server that knows a new type, we should not silently reject it, so this case really does need an error message, at which point we want to have a default handler, at which point my hack no longer helps (gcc only warns about uncovered enum values if there is no default case). I'll fix it.
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, size = 0; + unsigned int flags; + virTypedParameterPtr params, new_params; + + if (!PyArg_ParseTuple(args, + (char *)"OOi:virDomainSetNumaParameters", +&pyobj_domain,&info,&flags)) + return NULL; + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + if ((size = PyDict_Size(info))< 0) + return NULL; + + 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 (size == 0) { + PyErr_Format(PyExc_LookupError, + "Domain has no settable attributes"); + return NULL; + }
A typo, "if (nparams == 0)"
Good catch.
Thanks for these comments, ACK.
I'm squashing this in, then pushing. Are you still planning on retro-fitting other virTypedParam callers to use the new helper functions? diff --git i/python/libvirt-override.c w/python/libvirt-override.c index cb75cbe..e7c2bd5 100644 --- i/python/libvirt-override.c +++ w/python/libvirt-override.c @@ -81,7 +81,7 @@ getPyVirTypedParameter(const virTypedParameterPtr params, int nparams) return NULL; for (i = 0 ; i < nparams ; i++) { - switch ((virTypedParameterType) params[i].type) { + switch (params[i].type) { case VIR_TYPED_PARAM_INT: val = PyInt_FromLong(params[i].value.i); break; @@ -110,9 +110,14 @@ getPyVirTypedParameter(const virTypedParameterPtr params, int nparams) val = libvirt_constcharPtrWrap(params[i].value.s); break; - case VIR_TYPED_PARAM_LAST: - /* Shouldn't get here */ - return 0; + default: + /* Possible if a newer server has a bug and sent stuff we + * don't recognize. */ + PyErr_Format(PyExc_LookupError, + "Type value \"%d\" not recognized", + params[i].type); + val = NULL; + break; } key = libvirt_constcharPtrWrap(params[i].field); @@ -186,7 +191,7 @@ setPyVirTypedParameter(PyObject *info, ignore_value(virStrcpyStatic(temp->field, keystr)); temp->type = params[i].type; - switch((virTypedParameterType) params[i].type) { + switch(params[i].type) { case VIR_TYPED_PARAM_INT: { long long_val = PyInt_AsLong(value); @@ -267,8 +272,12 @@ setPyVirTypedParameter(PyObject *info, break; } - case VIR_TYPED_PARAM_LAST: - /* Shouldn't get here */ + default: + /* Possible if a newer server has a bug and sent stuff we + * don't recognize. */ + PyErr_Format(PyExc_LookupError, + "Type value \"%d\" not recognized", + params[i].type); goto cleanup; } @@ -1246,6 +1255,12 @@ libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, if ((size = PyDict_Size(info)) < 0) return NULL; + if (size == 0) { + PyErr_Format(PyExc_LookupError, + "Need non-empty dictionary to set attributes"); + return NULL; + } + LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetNumaParameters(domain, NULL, &nparams, flags); LIBVIRT_END_ALLOW_THREADS; @@ -1253,7 +1268,7 @@ libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, if (i_retval < 0) return VIR_PY_INT_FAIL; - if (size == 0) { + if (nparams == 0) { PyErr_Format(PyExc_LookupError, "Domain has no settable attributes"); return NULL; -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 02/10/2012 07:22 AM, Eric Blake wrote:
On 02/08/2012 09:29 PM, Guannan Ren wrote:
On 02/09/2012 09:41 AM, Eric Blake wrote:
From: Guannan Ren<gren@redhat.com>
*getPyVirTypedParameter *setPyVirTypedParameter *virDomainSetNumaParameters *virDomainGetNumaParameters
Signed-off-by: Eric Blake<eblake@redhat.com> ---
v5: Incorporate my review comments on v4
+ for (i = 0 ; i< nparams ; i++) { + switch ((virTypedParameterType) params[i].type) { + case VIR_TYPED_PARAM_INT: + val = PyInt_FromLong(params[i].value.i); + break; + + + case VIR_TYPED_PARAM_LAST: + /* Shouldn't get here */ + return 0; + } The effect of return 0 is the same as return NULL that will trigger the exception if defined before. Otherwise if the exception is not defined, the exception is as follows: "System Error: error return without exception set" In the case of having compiler to check out, it's ok here.
Hmm; originally, I was returning 0 on success and -1 on failure, then I changed the signature to return NULL on failure and object on success, but not this line. 0 acts like NULL, but it would be better written as NULL.
At any rate, my trick of doing switch ((virTypedParameterType) params[i].type) will ensure that at least gcc, with warnings, will warn us if we ever miss any cases known at compile time. Conversely, if we are talking to a newer server that knows a new type, we should not silently reject it, so this case really does need an error message, at which point we want to have a default handler, at which point my hack no longer helps (gcc only warns about uncovered enum values if there is no default case). I'll fix it.
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, size = 0; + unsigned int flags; + virTypedParameterPtr params, new_params; + + if (!PyArg_ParseTuple(args, + (char *)"OOi:virDomainSetNumaParameters", +&pyobj_domain,&info,&flags)) + return NULL; + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + if ((size = PyDict_Size(info))< 0) + return NULL; + + 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 (size == 0) { + PyErr_Format(PyExc_LookupError, + "Domain has no settable attributes"); + return NULL; + } A typo, "if (nparams == 0)" Good catch.
Thanks for these comments, ACK.
I'm squashing this in, then pushing. Are you still planning on retro-fitting other virTypedParam callers to use the new helper functions? yes, I would like to do that :) Thanks all your help till now.
Guannan Ren
participants (3)
-
Eric Blake -
Guannan Ren -
Osier Yang