On Wed, Mar 18, 2015 at 10:32:54AM +0000, Daniel P. Berrange wrote:
From: Nehal J Wani <nehaljw.kkd1@gmail.com>
examples/Makefile.am: * Add new file domipaddrs.py
examples/README: * Add documentation for the python example
libvirt-override-api.xml: * Add new symbol for virDomainInterfacesAddresses
libvirt-override.c: * Hand written python api
Example: $ python examples/domipaddrs.py qemu:///system f18 Interface MAC address Protocol Address vnet0 52:54:00:20:70:3d ipv4 192.168.105.240/16
In v11: - Cope with hwaddr being NULL by filling in PY_NONE --- MANIFEST.in | 1 + examples/README | 1 + examples/domipaddrs.py | 57 ++++++++++++++++++++++++ generator.py | 2 + libvirt-override-api.xml | 9 +++- libvirt-override.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++ sanitytest.py | 3 ++ 7 files changed, 182 insertions(+), 1 deletion(-) create mode 100755 examples/domipaddrs.py
[...]
diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml index 4660c9f..b197639 100644 --- a/libvirt-override-api.xml +++ b/libvirt-override-api.xml @@ -678,5 +678,12 @@ <arg name='flags' type='unsigned int' info='unused, pass 0'/> <return type='char *' info="list of mounted filesystems information"/> </function> - </symbols> + <function name='virDomainInterfaceAddresses' file='python'> + <info>returns a dictionary of domain interfaces along with their MAC and IP addresses</info> + <arg name='dom' type='virDomainPtr' info='pointer to the domain'/> + <arg name='source' type='unsigned int' info='the data source'/> + <arg name='flags' type='unsigned int' info='extra flags; not used yet, so callers should always pass 0'/> + <return type='char *' info="dictionary of domain interfaces along with their MAC and IP addresses"/> + </function> +</symbols>
Probably just a mistake with the </symbols> change.
</api> diff --git a/libvirt-override.c b/libvirt-override.c index 1241305..548be24 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -5120,6 +5120,113 @@ cleanup: return py_retval; }
+ +static PyObject * +libvirt_virDomainInterfaceAddresses(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *py_retval = VIR_PY_NONE; + virDomainPtr domain; + PyObject *pyobj_domain; + unsigned int source; + unsigned int flags; + virDomainInterfacePtr *ifaces = NULL; + int ifaces_count = 0; + size_t i, j; + int ret; + bool full_free = true; + + if (!PyArg_ParseTuple(args, (char *) "Oii:virDomainInterfacePtr", + &pyobj_domain, &source, &flags)) + return NULL;
You cannot return NULL without Py_DECREF(py_retval) because VIR_PY_NONE takes reference to Py_None.
+ + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + LIBVIRT_BEGIN_ALLOW_THREADS; + ifaces_count = virDomainInterfaceAddresses(domain, &ifaces, source, flags); + ret = ifaces_count; + LIBVIRT_END_ALLOW_THREADS; + if (ret < 0) + goto cleanup;
The ret variable is pointless here.
+ + if (!(py_retval = PyDict_New())) + goto no_memory; + + for (i = 0; i < ifaces_count; i++) { + virDomainInterfacePtr iface = ifaces[i]; + PyObject *py_addrs = NULL; + PyObject *py_iface = NULL; + + if (!(py_iface = PyDict_New())) + goto no_memory; + + if (iface->naddrs) { + if (!(py_addrs = PyList_New(iface->naddrs))) { + Py_DECREF(py_iface); + goto no_memory; + } + } else { + py_addrs = VIR_PY_NONE; + } + + for (j = 0; j < iface->naddrs; j++) { + virDomainIPAddressPtr addr = &(iface->addrs[j]); + PyObject *py_addr = PyDict_New(); + int type = addr->type; + + if (!addr) {
You've probably meant py_addr.
+ Py_DECREF(py_iface); + Py_DECREF(py_addrs); + goto no_memory; + } + + PyDict_SetItem(py_addr, libvirt_constcharPtrWrap("addr"), + libvirt_constcharPtrWrap(addr->addr)); + PyDict_SetItem(py_addr, libvirt_constcharPtrWrap("prefix"), + libvirt_intWrap(addr->prefix)); + PyDict_SetItem(py_addr, libvirt_constcharPtrWrap("type"), + libvirt_intWrap(type)); +
All the libvirt_*Wrap and Py*_SetItem functions can fail. In that case they returns NULL/-1 and set an exception and we should check that and in case of error return NULL.
+ PyList_SetItem(py_addrs, j, py_addr); + } + + PyDict_SetItem(py_iface, libvirt_constcharPtrWrap("addrs"), + py_addrs); + if (iface->hwaddr) { + PyDict_SetItem(py_iface, libvirt_constcharPtrWrap("hwaddr"), + libvirt_constcharPtrWrap(iface->hwaddr)); + } else { + PyDict_SetItem(py_iface, libvirt_constcharPtrWrap("hwaddr"), + VIR_PY_NONE); + } + + PyDict_SetItem(py_retval, libvirt_charPtrWrap(iface->name), + py_iface); + } + + full_free = false; + +cleanup: + if (ifaces && ifaces_count > 0) { + for (i = 0; full_free && i < ifaces_count; i++) { + /* + * We don't want to free values we've just shared with python variables unless + * there was an error and hence we are returning PY_NONE or equivalent + */
Actually I don't think that this statement is true. All the values that we've shared with python are copied and therefore can be freed.
+ virDomainInterfaceFree(ifaces[i]); + } + } + VIR_FREE(ifaces); + + return py_retval; + +no_memory: + Py_XDECREF(py_retval); + py_retval = PyErr_NoMemory();
All the paths leading to no_memory already set python exception and we should not just overwrite it. Keep the original exception instead as it doesn't have to be always NoMemmory.
+ goto cleanup; +} + + /******************************************* * Helper functions to avoid importing modules * for every callback @@ -8750,6 +8857,9 @@ static PyMethodDef libvirtMethods[] = { #if LIBVIR_CHECK_VERSION(1, 2, 11) {(char *) "virDomainGetFSInfo", libvirt_virDomainGetFSInfo, METH_VARARGS, NULL}, #endif /* LIBVIR_CHECK_VERSION(1, 2, 11) */ +#if LIBVIR_CHECK_VERSION(1, 2, 14) + {(char *) "virDomainInterfaceAddresses", libvirt_virDomainInterfaceAddresses, METH_VARARGS, NULL}, +#endif /* LIBVIR_CHECK_VERSION(1, 2, 14) */ {NULL, NULL, 0, NULL} };
diff --git a/sanitytest.py b/sanitytest.py index 0e6e0e5..2b609a9 100644 --- a/sanitytest.py +++ b/sanitytest.py @@ -145,6 +145,9 @@ for cname in wantfunctions: if name[0:26] == "virDomainIOThreadsInfoFree": continue
+ if name[0:33] == "virDomainInterfaceFree": + continue
You've probably just hit a wrong key as it should be 22.
+ if name[0:21] == "virDomainListGetStats": name = "virConnectDomainListGetStats"
-- 2.1.0
I'll send a v12 with all the changes for review. The rewrite also uses implicit free by assigning all created python objects to the main py_retval dict. Then the Py_XDECREF will free everything for us. Pavel