[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv5] python: refactoring virTypedParameter conversion for NUMA tuning APIs



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]