[libvirt] [PATCH v2] python: add virDomainGetCPUStats python binding API

[Guannan Ren]

On 03/17/2012 01:26 AM, Eric Blake wrote:
> On 03/14/2012 07:03 AM, Guannan Ren wrote:
>>      dom.getCPUStats(True, 0)
>>        [{'cpu_time': 92913537401L, 'system_time': 5470000000L, 'user_time': 310000000L}]
>>
>>      dom.getCPUStats(False, 0)
>>        [{'cpu_time': 39476858499L}, {'cpu_time': 10627048370L}, {'cpu_time': 21270945682L}, {'cpu_time': 21556420641L}]
>>
>>      *generator.py Add a new naming rule
>>      *libvirt-override-api.xml The API function description
>>      *libvirt-override.c Implement it.
>> ---
>>   python/generator.py             |    5 +-
>>   python/libvirt-override-api.xml |   10 +++
>>   python/libvirt-override.c       |  164 +++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 178 insertions(+), 1 deletions(-)
>>
>> +++ b/python/libvirt-override-api.xml
>> @@ -149,6 +149,16 @@
>>         <arg name='path' type='char *' info='the path for the block device'/>
>>         <arg name='flags' type='int' info='flags (unused; pass 0)'/>
>>       </function>
>> +<function name='virDomainGetCPUStats' file='python'>
>> +<info>Extracts CPU statistics for a running domain, On success it will return a list of data of dictionary type.
> s/, On/. On/
>
> Long lines; can you wrap this to fit in 80 columns?
>
>> +   If boolean total is True, the first element of the list refers to CPU0 on the host, second element is CPU1, and so on.
> s/total is True/total is False/
>
>> +   The format of data struct is like [{cpu_time:xxx},{cpu_time:xxx}, ...]
>> +   If it is False, it returns total domain CPU statistics like [{cpu_time:xxx, user_time:xxx, system_time:xxx}]</info>
> s/False/True/
>
>> +
>> +    if (!PyBool_Check(totalbool)) {
>> +        PyErr_Format(PyExc_TypeError,
>> +                    "The \"total\" attribute must be bool");
>> +        return NULL;
>> +    }
>> +
>> +    if ((ret = PyList_New(0)) == NULL)
>> +        return NULL;
>> +
>> +    if (totalbool == Py_False) {
> Per other code in libvirt-override.c, you can't compare totalbool (type
> PyObject) with Py_False, at least not on all compilers.  You need
> something like this instead:
>
>              /* 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);

      Yes, it did report warning in compiling as follows due to the case 
from PyIntObject* to PyObject*
      warning :dereferencing type-punned pointer might break 
strict-aliasing rules [-Wstrict-aliasing]

      GCC command line to reproduce the error:
      gcc -Wstrict-aliasing=1 -O2 cpythonexample.c

      Actually PyObject_IsTrue() is a more light-weight approach to do 
the checking instead of
      creating a intermediate PyObject * for the compare. But for the 
more portability, It is still better to
      choose the above comparing approach for boolean value.

      Guannan Ren