[libvirt] [PATCH 0/2] Expose virNodeGetCPUStats and virNodeGetMemoryStats in python
These patches add two API functions to the libvirt python bindings, that were missing. The constants denoting a summary stat value instead of a separate per-node values was declared using a preprocessor macro: ... #define VIR_NODE_MEMORY_STATS_ALL_CELLS (-1) #define VIR_NODE_CPU_STATS_ALL_CPUS (-1) ... This inhibits automatic generation of these constants in the automaticaly generated python library code. If this change is too controversial, there's another option: Including these constants in the python library (using libvirt-override.py). Please let me know which of those options you prefer. Thanks Peter Peter Krempa (2): python: Expose binding for virNodeGetCPUStats() python: Expose binding for virNodeGetMemoryStats() include/libvirt/libvirt.h.in | 12 +++-- - change macro definitions to enums python/libvirt-override-api.xml | 14 ++++++ - add doc's about newly added functions python/libvirt-override.c | 95 +++++++++++++++++++++++++++++++++++++++ - add code for calling those functions and reclaiming return values. 3 files changed, 117 insertions(+), 4 deletions(-) -- 1.7.3.4
This patch adds binding for virNodeGetCPUStats method of libvirtd. Return value is represended as a python dictionary mapping fileld names to values. --- include/libvirt/libvirt.h.in | 6 +++- python/libvirt-override-api.xml | 7 +++++ python/libvirt-override.c | 47 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 0787f18..1291c59 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -361,9 +361,11 @@ struct _virNodeInfo { /** * VIR_NODE_CPU_STATS_ALL_CPUS: * - * Macro for the total CPU time/utilization + * Value for specifying request for the total CPU time/utilization */ -#define VIR_NODE_CPU_STATS_ALL_CPUS (-1) +typedef enum { + VIR_NODE_CPU_STATS_ALL_CPUS = -1, +} virNodeGetCPUStatsAllCPUs; /** * VIR_NODE_CPU_STATS_KERNEL: diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index ef02f34..e6dc967 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -77,6 +77,13 @@ <return type='int *' info='the list of information or None in case of error'/> <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/> </function> + <function name='virNodeGetCPUStats' file='python'> + <info>Extract node's CPU statistics.</info> + <return type='virNodeCPUStats' info='dictionary mapping field names to values or None in case of error'/> + <arg name='conn' type='virConnectPtr' info='pointer to hypervisor connection'/> + <arg name='cpuNum' type='int' info='number of node cpu. (VIR_NODE_CPU_STATS_ALL_CPUS means total cpu statistics)'/> + <arg name='flags' type='unsigned int' info='aditional flags'/> + </function> <function name='virDomainGetUUID' file='python'> <info>Extract the UUID unique Identifier of a domain.</info> <return type='char *' info='the 16 bytes string or None in case of error'/> diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 1759bae..c2abc9c 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -2256,6 +2256,52 @@ libvirt_virNodeGetCellsFreeMemory(PyObject *self ATTRIBUTE_UNUSED, PyObject *arg return(py_retval); } +static PyObject * +libvirt_virNodeGetCPUStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) +{ + PyObject *ret; + PyObject *pyobj_conn; + virConnectPtr conn; + unsigned int flags; + int cpuNum, c_retval, i; + int nparams = 0; + virNodeCPUStatsPtr stats = NULL; + + if (!PyArg_ParseTuple(args, (char *)"Oii:virNodeGetCPUStats", &pyobj_conn, &cpuNum, &flags)) + return(NULL); + conn = (virConnectPtr)(PyvirConnect_Get(pyobj_conn)); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virNodeGetCPUStats(conn, cpuNum, NULL, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + if (c_retval < 0) + return VIR_PY_NONE; + + if (nparams) { + if (!(stats = malloc(sizeof(*stats) * nparams))) + return VIR_PY_NONE; + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virNodeGetCPUStats(conn, cpuNum, stats, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + if (c_retval < 0) { + free(stats); + return VIR_PY_NONE; + } + } + if (!(ret = PyDict_New())) { + free(stats); + return VIR_PY_NONE; + } + for (i = 0; i < nparams; i++) { + PyDict_SetItem(ret, + libvirt_constcharPtrWrap(stats[i].field), + libvirt_ulonglongWrap(stats[i].value)); + } + + free(stats); + return ret; +} static PyObject * libvirt_virConnectListStoragePools(PyObject *self ATTRIBUTE_UNUSED, @@ -4773,6 +4819,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virDomainGetControlInfo", libvirt_virDomainGetControlInfo, METH_VARARGS, NULL}, {(char *) "virDomainGetBlockInfo", libvirt_virDomainGetBlockInfo, METH_VARARGS, NULL}, {(char *) "virNodeGetInfo", libvirt_virNodeGetInfo, METH_VARARGS, NULL}, + {(char *) "virNodeGetCPUStats", libvirt_virNodeGetCPUStats, METH_VARARGS, NULL}, {(char *) "virDomainGetUUID", libvirt_virDomainGetUUID, METH_VARARGS, NULL}, {(char *) "virDomainGetUUIDString", libvirt_virDomainGetUUIDString, METH_VARARGS, NULL}, {(char *) "virDomainLookupByUUID", libvirt_virDomainLookupByUUID, METH_VARARGS, NULL}, -- 1.7.3.4
On 11/28/2011 10:19 AM, Peter Krempa wrote:
This patch adds binding for virNodeGetCPUStats method of libvirtd. Return value is represended as a python dictionary mapping fileld names
s/represended/represented/ s/fileld/field/
to values. --- include/libvirt/libvirt.h.in | 6 +++- python/libvirt-override-api.xml | 7 +++++ python/libvirt-override.c | 47 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 0787f18..1291c59 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -361,9 +361,11 @@ struct _virNodeInfo { /** * VIR_NODE_CPU_STATS_ALL_CPUS: * - * Macro for the total CPU time/utilization + * Value for specifying request for the total CPU time/utilization */ -#define VIR_NODE_CPU_STATS_ALL_CPUS (-1) +typedef enum { + VIR_NODE_CPU_STATS_ALL_CPUS = -1, +} virNodeGetCPUStatsAllCPUs;
Safe change to make, and if it helps python generation, then it should be okay.
/** * VIR_NODE_CPU_STATS_KERNEL: diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index ef02f34..e6dc967 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -77,6 +77,13 @@ <return type='int *' info='the list of information or None in case of error'/> <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/> </function> + <function name='virNodeGetCPUStats' file='python'> + <info>Extract node's CPU statistics.</info> + <return type='virNodeCPUStats' info='dictionary mapping field names to values or None in case of error'/> + <arg name='conn' type='virConnectPtr' info='pointer to hypervisor connection'/> + <arg name='cpuNum' type='int' info='number of node cpu. (VIR_NODE_CPU_STATS_ALL_CPUS means total cpu statistics)'/> + <arg name='flags' type='unsigned int' info='aditional flags'/>
s/aditional/additional/ ACK with nits fixed, and worth including in 0.9.8. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
This patch adds binding for virNodeGetMemoryStats method of libvirtd. Return value is represended as a python dictionary mapping fileld names to values. --- include/libvirt/libvirt.h.in | 6 +++- python/libvirt-override-api.xml | 7 +++++ python/libvirt-override.c | 48 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 1291c59..e3afeed 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -431,9 +431,11 @@ struct _virNodeCPUStats { /** * VIR_NODE_MEMORY_STATS_ALL_CELLS: * - * Macro for the total memory of all cells. + * Value for specifying request for the total memory of all cells. */ -#define VIR_NODE_MEMORY_STATS_ALL_CELLS (-1) +typedef enum { + VIR_NODE_MEMORY_STATS_ALL_CELLS = -1, +} virNodeGetMemoryStatsAllCells; /** * VIR_NODE_MEMORY_STATS_TOTAL: diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index e6dc967..1aa689f 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -84,6 +84,13 @@ <arg name='cpuNum' type='int' info='number of node cpu. (VIR_NODE_CPU_STATS_ALL_CPUS means total cpu statistics)'/> <arg name='flags' type='unsigned int' info='aditional flags'/> </function> + <function name='virNodeGetMemoryStats' file='python'> + <info>Extract node's memory statistics.</info> + <return type='virNodeMemoryStats' info='dictionary mapping field names to values or None in case of error'/> + <arg name='conn' type='virConnectPtr' info='pointer to hypervisor connection'/> + <arg name='cellNum' type='int' info='number of node cell. (VIR_NODE_MEMORY_STATS_ALL_CELLS means total cell statistics)'/> + <arg name='flags' type='unsigned int' info='aditional flags'/> + </function> <function name='virDomainGetUUID' file='python'> <info>Extract the UUID unique Identifier of a domain.</info> <return type='char *' info='the 16 bytes string or None in case of error'/> diff --git a/python/libvirt-override.c b/python/libvirt-override.c index c2abc9c..7b554a5 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -2304,6 +2304,53 @@ libvirt_virNodeGetCPUStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) } static PyObject * +libvirt_virNodeGetMemoryStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) +{ + PyObject *ret; + PyObject *pyobj_conn; + virConnectPtr conn; + unsigned int flags; + int cellNum, c_retval, i; + int nparams = 0; + virNodeMemoryStatsPtr stats = NULL; + + if (!PyArg_ParseTuple(args, (char *)"Oii:virNodeGetMemoryStats", &pyobj_conn, &cellNum, &flags)) + return(NULL); + conn = (virConnectPtr)(PyvirConnect_Get(pyobj_conn)); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virNodeGetMemoryStats(conn, cellNum, NULL, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + if (c_retval < 0) + return VIR_PY_NONE; + + if (nparams) { + if (!(stats = malloc(sizeof(*stats) * nparams))) + return VIR_PY_NONE; + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virNodeGetMemoryStats(conn, cellNum, stats, &nparams, flags); + LIBVIRT_END_ALLOW_THREADS; + if (c_retval < 0) { + free(stats); + return VIR_PY_NONE; + } + } + if (!(ret = PyDict_New())) { + free(stats); + return VIR_PY_NONE; + } + for (i = 0; i < nparams; i++) { + PyDict_SetItem(ret, + libvirt_constcharPtrWrap(stats[i].field), + libvirt_ulonglongWrap(stats[i].value)); + } + + free(stats); + return ret; +} + +static PyObject * libvirt_virConnectListStoragePools(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *py_retval; @@ -4820,6 +4867,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virDomainGetBlockInfo", libvirt_virDomainGetBlockInfo, METH_VARARGS, NULL}, {(char *) "virNodeGetInfo", libvirt_virNodeGetInfo, METH_VARARGS, NULL}, {(char *) "virNodeGetCPUStats", libvirt_virNodeGetCPUStats, METH_VARARGS, NULL}, + {(char *) "virNodeGetMemoryStats", libvirt_virNodeGetMemoryStats, METH_VARARGS, NULL}, {(char *) "virDomainGetUUID", libvirt_virDomainGetUUID, METH_VARARGS, NULL}, {(char *) "virDomainGetUUIDString", libvirt_virDomainGetUUIDString, METH_VARARGS, NULL}, {(char *) "virDomainLookupByUUID", libvirt_virDomainLookupByUUID, METH_VARARGS, NULL}, -- 1.7.3.4
On 11/28/2011 10:19 AM, Peter Krempa wrote:
This patch adds binding for virNodeGetMemoryStats method of libvirtd. Return value is represended as a python dictionary mapping fileld
s/represended/represented/ s/fileld/field/ (copy-and-paste from the last patch :)
+++ b/python/libvirt-override-api.xml @@ -84,6 +84,13 @@ <arg name='cpuNum' type='int' info='number of node cpu. (VIR_NODE_CPU_STATS_ALL_CPUS means total cpu statistics)'/> <arg name='flags' type='unsigned int' info='aditional flags'/> </function> + <function name='virNodeGetMemoryStats' file='python'> + <info>Extract node's memory statistics.</info> + <return type='virNodeMemoryStats' info='dictionary mapping field names to values or None in case of error'/> + <arg name='conn' type='virConnectPtr' info='pointer to hypervisor connection'/> + <arg name='cellNum' type='int' info='number of node cell. (VIR_NODE_MEMORY_STATS_ALL_CELLS means total cell statistics)'/> + <arg name='flags' type='unsigned int' info='aditional flags'/>
s/aditional/additional/
+ if (!(ret = PyDict_New())) { + free(stats); + return VIR_PY_NONE; + } + for (i = 0; i < nparams; i++) { + PyDict_SetItem(ret, + libvirt_constcharPtrWrap(stats[i].field), + libvirt_ulonglongWrap(stats[i].value)); + }
Copy and paste, so not a problem with this patch any more so than the other functions that used the same code pattern, but can PyDict_SetItem fail? If so, should be be reclaiming the entries added so far before returning overall failure, instead of silently truncating the return dictionary by omitting the entries that weren't inserted properly? ACK with spelling nits fixed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Dňa 2.12.2011 18:23, Eric Blake wrote / napísal(a):
On 11/28/2011 10:19 AM, Peter Krempa wrote:
+ if (!(ret = PyDict_New())) { + free(stats); + return VIR_PY_NONE; + } + for (i = 0; i< nparams; i++) { + PyDict_SetItem(ret, + libvirt_constcharPtrWrap(stats[i].field), + libvirt_ulonglongWrap(stats[i].value)); + }
Copy and paste, so not a problem with this patch any more so than the other functions that used the same code pattern, but can PyDict_SetItem fail? If so, should be be reclaiming the entries added so far before returning overall failure, instead of silently truncating the return dictionary by omitting the entries that weren't inserted properly?
Well, I was wondering myself why there's no check for insertion of the item into the dictionary as it is apparently allocating (tons of) memory. I was following the pattern used in already existing code. The better approach would be: + for (i = 0; i< nparams; i++) { + if (PyDict_SetItem(ret, + libvirt_constcharPtrWrap(stats[i].field), + libvirt_ulonglongWrap(stats[i].value))<0){ + Py_XDECREF(ret); + return VIR_PY_NONE; + } + } Free the reference and return an error. Maybe it would be useful to cause an exception, but I'm not a python binding guru. Should I make it more robust? Or maybe we should clean this up later in all instances of improper dictionary insertions? Peter
On 12/02/2011 06:23 PM, Eric Blake wrote:
Copy and paste, so not a problem with this patch any more so than the other functions that used the same code pattern, but can PyDict_SetItem fail? If so, should be be reclaiming the entries added so far before returning overall failure, instead of silently truncating the return dictionary by omitting the entries that weren't inserted properly?
ACK with spelling nits fixed.
Thanks. Pushed with spelling fixed, leaving cleanup of unchecked return value of PyDict_SetItem for a later patch. Peter
participants (2)
-
Eric Blake -
Peter Krempa