[libvirt] [PATCH] python: return dictionay without value in case of no blockjob

Currently, when there is no blockjob, dom.blockJobInfo('vda') still reports error because it didn't distinguish return value 0 from -1. libvirt.libvirtError: virDomainGetBlockJobInfo() failed virDomainGetBlockJobInfo() API return value: -1 in case of failure, 0 when nothing found, 1 found. And use PyDict_SetItemString instead of PyDict_SetItem when key is string type. PyDict_SetItemString increments key/value reference count, so calling Py_DECREF() for value. For key, we don't need to do this, because PyDict_SetItemString will handle it internally. --- python/libvirt-override.c | 54 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 15 deletions(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index fd9ebb8..c1e8661 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -4354,33 +4354,57 @@ libvirt_virDomainGetBlockJobInfo(PyObject *self ATTRIBUTE_UNUSED, unsigned int flags; virDomainBlockJobInfo info; int c_ret; - PyObject *ret; + PyObject *type = NULL, *bandwidth = NULL, *cur = NULL, *end = NULL; + PyObject *dict; if (!PyArg_ParseTuple(args, (char *)"Ozi:virDomainGetBlockJobInfo", &pyobj_domain, &path, &flags)) return NULL; domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); -LIBVIRT_BEGIN_ALLOW_THREADS; + if ((dict = PyDict_New()) == NULL) + return NULL; + + LIBVIRT_BEGIN_ALLOW_THREADS; c_ret = virDomainGetBlockJobInfo(domain, path, &info, flags); -LIBVIRT_END_ALLOW_THREADS; + LIBVIRT_END_ALLOW_THREADS; - if (c_ret != 1) + if (c_ret == 0) { + return dict; + } else if (c_ret < 0) { + Py_DECREF(dict); return VIR_PY_NONE; + } - if ((ret = PyDict_New()) == NULL) - return VIR_PY_NONE; + if ((type = libvirt_intWrap(info.type)) == NULL || + PyDict_SetItemString(dict, "type", type) < 0) + goto error; + Py_DECREF(type); - PyDict_SetItem(ret, libvirt_constcharPtrWrap("type"), - libvirt_intWrap(info.type)); - PyDict_SetItem(ret, libvirt_constcharPtrWrap("bandwidth"), - libvirt_ulongWrap(info.bandwidth)); - PyDict_SetItem(ret, libvirt_constcharPtrWrap("cur"), - libvirt_ulonglongWrap(info.cur)); - PyDict_SetItem(ret, libvirt_constcharPtrWrap("end"), - libvirt_ulonglongWrap(info.end)); + if ((bandwidth = libvirt_ulongWrap(info.bandwidth)) == NULL || + PyDict_SetItemString(dict, "bandwidth", bandwidth) < 0) + goto error; + Py_DECREF(bandwidth); - return ret; + if ((cur = libvirt_ulonglongWrap(info.cur)) == NULL || + PyDict_SetItemString(dict, "cur", cur) < 0) + goto error; + Py_DECREF(cur); + + if ((end = libvirt_ulonglongWrap(info.end)) == NULL || + PyDict_SetItemString(dict, "end", end) < 0) + goto error; + Py_DECREF(end); + + return dict; + +error: + Py_DECREF(dict); + Py_XDECREF(type); + Py_XDECREF(bandwidth); + Py_XDECREF(cur); + Py_XDECREF(end); + return NULL; } static PyObject * -- 1.8.1.4

On 17.05.2013 08:30, Guannan Ren wrote:
s/dictionay/dictionary/ in $SUBJ
Currently, when there is no blockjob, dom.blockJobInfo('vda') still reports error because it didn't distinguish return value 0 from -1.
s/didn't/doesn't/
libvirt.libvirtError: virDomainGetBlockJobInfo() failed
virDomainGetBlockJobInfo() API return value: -1 in case of failure, 0 when nothing found, 1 found.
And use PyDict_SetItemString instead of PyDict_SetItem when key is string type. PyDict_SetItemString increments key/value reference
s/string type/of string type/
count, so calling Py_DECREF() for value. For key, we don't need to
s/calling/call/
do this, because PyDict_SetItemString will handle it internally. --- python/libvirt-override.c | 54 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 15 deletions(-)
ACK with the commit message fixed. Michal

On 07/13/2013 01:36 AM, Michal Privoznik wrote:
On 17.05.2013 08:30, Guannan Ren wrote: s/dictionay/dictionary/ in $SUBJ
Currently, when there is no blockjob, dom.blockJobInfo('vda') still reports error because it didn't distinguish return value 0 from -1. s/didn't/doesn't/
libvirt.libvirtError: virDomainGetBlockJobInfo() failed
virDomainGetBlockJobInfo() API return value: -1 in case of failure, 0 when nothing found, 1 found.
And use PyDict_SetItemString instead of PyDict_SetItem when key is string type. PyDict_SetItemString increments key/value reference s/string type/of string type/
count, so calling Py_DECREF() for value. For key, we don't need to s/calling/call/
do this, because PyDict_SetItemString will handle it internally. --- python/libvirt-override.c | 54 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 15 deletions(-) ACK with the commit message fixed.
Michal
Thanks for this review. I pushed with these fixed Guannan

On 07/15/2013 04:13 AM, Guannan Ren wrote:
On 07/13/2013 01:36 AM, Michal Privoznik wrote:
On 17.05.2013 08:30, Guannan Ren wrote: s/dictionay/dictionary/ in $SUBJ
Currently, when there is no blockjob, dom.blockJobInfo('vda') still reports error because it didn't distinguish return value 0 from -1. s/didn't/doesn't/
libvirt.libvirtError: virDomainGetBlockJobInfo() failed
virDomainGetBlockJobInfo() API return value: -1 in case of failure, 0 when nothing found, 1 found.
And use PyDict_SetItemString instead of PyDict_SetItem when key is string type. PyDict_SetItemString increments key/value reference s/string type/of string type/
count, so calling Py_DECREF() for value. For key, we don't need to s/calling/call/
do this, because PyDict_SetItemString will handle it internally. --- python/libvirt-override.c | 54 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 15 deletions(-) ACK with the commit message fixed.
Michal
Thanks for this review. I pushed with these fixed
As I hit the same issue in Fedora 19 (https://bugzilla.redhat.com/show_bug.cgi?id=999077), I have now backported this to v1.0.5-maint. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Eric Blake
-
Guannan Ren
-
Michal Privoznik