[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