[libvirt] [PATCHv5] python: refactoring virTypedParameter conversion for NUMA tuning APIs
Guannan Ren
gren at redhat.com
Fri Feb 10 05:01:34 UTC 2012
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 at redhat.com>
>>>
>>> *getPyVirTypedParameter
>>> *setPyVirTypedParameter
>>> *virDomainSetNumaParameters
>>> *virDomainGetNumaParameters
>>>
>>> Signed-off-by: Eric Blake<eblake at 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
More information about the libvir-list
mailing list