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