On Thu, Feb 19, 2015 at 07:59:38AM -0500, John Ferlan wrote:
> Add support for the libvirt_virDomainGetIOThreadsInfo method. This
> code mostly follows the libvirt_virDomainGetVcpuPinInfo method, but
> also takes some from the libvirt_virNodeGetCPUMap method with respect
> to building the cpumap into the returned tuple rather than two separate
> tuples which vcpu pinning generates
>
> Assuming two domains, one with IOThreads defined (eg, 'iothr-gst') and
> one without ('noiothr-gst'), execute the following in an 'iothr.py' file:
>
> import libvirt
> con=libvirt.open("qemu:///system")
> dom=con.lookupByName('iothr-gst')
> print dom.getIOThreadsInfo()
> dom2=con.lookupByName('noiothr-gst')
> print dom2.getIOThreadsInfo()
>
> $ python iothr.py
> [(1, [False, False, True, False], ['/home/vm-images/iothr-vol1']), (2, [False, False, False, True], ['/home/vm-images/iothr-vol2']), (3, [True, False, False, False], [])]
> Traceback (most recent call last):
> File "iothr.py", line 6, in <module>
> print dom2.getIOThreadsInfo()
> File "/usr/lib64/python2.7/site-packages/libvirt.py", line 1197, in getIOThreadsInfo
> if ret is None: raise libvirtError ('virDomainGetIOThreadsInfo() failed', dom=self)
> libvirt.libvirtError: virDomainGetIOThreadsInfo() failed
>
> $
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> generator.py | 5 +++
> libvirt-override-api.xml | 6 +++
> libvirt-override.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++
> sanitytest.py | 3 ++
> 4 files changed, 110 insertions(+)
>
> diff --git a/generator.py b/generator.py
> index 0d48980..327c896 100755
> --- a/generator.py
> +++ b/generator.py
> @@ -435,6 +435,7 @@ skip_impl = (
> 'virDomainGetVcpuPinInfo',
> 'virDomainGetEmulatorPinInfo',
> 'virDomainPinEmulator',
> + 'virDomainGetIOThreadsInfo',
> 'virSecretGetValue',
> 'virSecretSetValue',
> 'virSecretGetUUID',
> @@ -592,6 +593,7 @@ skip_function = (
> 'virNetworkDHCPLeaseFree', # only useful in C, python code uses list
> 'virDomainStatsRecordListFree', # only useful in C, python uses dict
> 'virDomainFSInfoFree', # only useful in C, python code uses list
> + 'virDomainIOThreadsInfoFree', # only useful in C, python code uses list
> )
>
> lxc_skip_function = (
> @@ -1144,6 +1146,9 @@ def nameFixup(name, classe, type, file):
> elif name[0:20] == "virDomainGetCPUStats":
> func = name[9:]
> func = func[0:1].lower() + func[1:]
> + elif name[0:25] == "virDomainGetIOThreadsInfo":
> + func = name[9:]
> + func = func[0:1].lower() + func[1:]
> elif name[0:18] == "virDomainGetFSInfo":
> func = name[12:]
> func = func[0:2].lower() + func[2:]
> diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml
> index 439cb40..e512311 100644
> --- a/libvirt-override-api.xml
> +++ b/libvirt-override-api.xml
> @@ -278,6 +278,12 @@
> <arg name='cpumap' type='unsigned char *' info='pointer to a bit map of real CPUs (in 8-bit bytes) (IN) Each bit set to 1 means that corresponding CPU is usable. Bytes are stored in little-endian order: CPU0-7, 8-15... In each byte, lowest CPU number is least significant bit.'/>
> <arg name='flags' type='int' info='flags to specify'/>
> </function>
> + <function name='virDomainGetIOThreadsInfo' file='python'>
> + <info>Query the CPU affinity setting of the IOThreads of the domain</info>
> + <arg name='domain' type='virDomainPtr' info='pointer to domain object, or NULL for Domain0'/>
> + <arg name='flags' type='int' info='an OR'ed set of virDomainModificationImpact'/>
> + <return type='char *' info="list of IOThreads information including the iothread_id, the cpumap, the cpumap length, number of associated resources, and a list of each resource assigned to the iothread_id."/>
> + </function>
> <function name='virDomainSetSchedulerParameters' file='python'>
> <info>Change the scheduler parameters</info>
> <return type='int' info='-1 in case of error, 0 in case of success.'/>
> diff --git a/libvirt-override.c b/libvirt-override.c
> index 88cb527..17f3bf3 100644
> --- a/libvirt-override.c
> +++ b/libvirt-override.c
> @@ -1990,6 +1990,99 @@ libvirt_virDomainGetEmulatorPinInfo(PyObject *self ATTRIBUTE_UNUSED,
> }
> #endif /* LIBVIR_CHECK_VERSION(0, 10, 0) */
>
> +#if LIBVIR_CHECK_VERSION(1, 2, 13)
> +static PyObject *
> +libvirt_virDomainGetIOThreadsInfo(PyObject *self ATTRIBUTE_UNUSED,
> + PyObject *args) {
> + virDomainPtr domain;
> + PyObject *pyobj_domain;
> + PyObject *py_retval = NULL;
> + PyObject *iothrtpl = NULL;
> + PyObject *iothrmap = NULL;
> + PyObject *resources = NULL;
> + virDomainIOThreadsInfoPtr *iothrinfo = NULL;
> + unsigned int flags;
> + size_t pcpu, i, j;
> + int niothreads, cpunum;
> +
> + if (!PyArg_ParseTuple(args, (char *)"OI:virDomainGetIOThreadsInfo",
> + &pyobj_domain, &flags))
> + return NULL;
> + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
> +
> + if ((cpunum = getPyNodeCPUCount(virDomainGetConnect(domain))) < 0)
> + return VIR_PY_NONE;
> +
> + LIBVIRT_BEGIN_ALLOW_THREADS;
> + niothreads = virDomainGetIOThreadsInfo(domain, &iothrinfo, flags);
> + LIBVIRT_END_ALLOW_THREADS;
> + /* On error or when there's no IOThreads, we return VIR_PY_NONE */
> + if (niothreads <= 0)
> + goto cleanup;
I'm not sure whether we should raise an exception if there are no iothreads
defined for domain.
> +
> + /* convert to a Python list */
> + if ((py_retval = PyList_New(niothreads)) == NULL)
> + goto cleanup;
PyList_New will returns NULL in case of error and sets an exception. In the
case the exception is set we should also return NULL instead of None.
> +
> + for (i = 0; i < niothreads; i++) {
> + virDomainIOThreadsInfoPtr iothr = iothrinfo[i];
> +
> + if (iothr == NULL)
> + goto cleanup;
> + if ((iothrtpl = PyTuple_New(3)) == NULL)
> + goto cleanup;
> + PyList_SetItem(py_retval, i, iothrtpl);
PyList_SetItem steals reference to iothrtpl object. In the cleanup section you
call PY_XDECREF on py_retval which will correctly free the list, but also all
the items inside the list including the iothrtpl tuple. Then you call PY_XDECREF
on iothrtpl and it tries to free it again -> double_free.
The common work-flow is:
if (list = PyList_New())
goto cleanup;
if (tuple = PyTuple_New())
goto cleanup;
if (PyTuple_SetItem())
goto cleanup;
if (PyList_SetItem(list, tuple))
goto cleanup;
tuple = NULL;
cleanup:
PY_XDECREF(list);
PY_XDECREF(tuple);
> +
> + /* 0: IOThread ID */
> + PyTuple_SetItem(iothrtpl, 0, libvirt_uintWrap(iothr->iothread_id));
> +
> + /* 1: CPU map */
> + if ((iothrmap = PyList_New(cpunum)) == NULL)
> + goto cleanup;
> +
> + for (pcpu = 0; pcpu < cpunum; pcpu++) {
> + PyObject *pyused;
> + if ((pyused = PyBool_FromLong(VIR_CPU_USED(iothr->cpumap,
> + pcpu))) == NULL)
> + goto cleanup;
This one doesn't set an error thus it's correct to return VIR_PY_NONE.
> + if (PyList_SetItem(iothrmap, pcpu, pyused) < 0) {
> + Py_XDECREF(pyused);
> + goto cleanup;
The PyList_SetItem also sets in case of error an exception. The rule is if you
sets an exception you should return NULL.
> + }
> + }
> + if (PyTuple_SetItem(iothrtpl, 1, iothrmap) < 0)
> + goto cleanup;
> +
> + /* 2: Resources - may be empty */
> + if ((resources = PyList_New(0)) == NULL)
> + goto cleanup;
> +
> + PyTuple_SetItem(iothrtpl, 2, resources);
> + for (j = 0; j < iothr->nresources; j++) {
> + if (PyList_Append(resources,
> + libvirt_constcharPtrWrap(iothr->resources[j])) < 0)
> + goto cleanup;
> + }
> + }
> +
> + for (i = 0; i < niothreads; i++)
> + virDomainIOThreadsInfoFree(iothrinfo[i]);
> + VIR_FREE(iothrinfo);
> +
> + return py_retval;
> +
> +cleanup:
> + for (i = 0; i < niothreads; i++)
> + virDomainIOThreadsInfoFree(iothrinfo[i]);
> + VIR_FREE(iothrinfo);
> + Py_XDECREF(py_retval);
> + Py_XDECREF(iothrtpl);
> + Py_XDECREF(iothrmap);
> + Py_XDECREF(resources);
> + return VIR_PY_NONE;
> +}
> +
> +#endif /* LIBVIR_CHECK_VERSION(1, 2, 13) */
>
> /************************************************************************
> * *
> @@ -8483,6 +8576,9 @@ static PyMethodDef libvirtMethods[] = {
> {(char *) "virDomainGetEmulatorPinInfo", libvirt_virDomainGetEmulatorPinInfo, METH_VARARGS, NULL},
> {(char *) "virDomainPinEmulator", libvirt_virDomainPinEmulator, METH_VARARGS, NULL},
> #endif /* LIBVIR_CHECK_VERSION(0, 10, 0) */
> +#if LIBVIR_CHECK_VERSION(1, 2, 13)
> + {(char *) "virDomainGetIOThreadsInfo", libvirt_virDomainGetIOThreadsInfo, METH_VARARGS, NULL},
> +#endif /* LIBVIR_CHECK_VERSION(1, 2, 13) */
> {(char *) "virConnectListStoragePools", libvirt_virConnectListStoragePools, METH_VARARGS, NULL},
> {(char *) "virConnectListDefinedStoragePools", libvirt_virConnectListDefinedStoragePools, METH_VARARGS, NULL},
> #if LIBVIR_CHECK_VERSION(0, 10, 2)
> diff --git a/sanitytest.py b/sanitytest.py
> index f021e5a..53209b8 100644
> --- a/sanitytest.py
> +++ b/sanitytest.py
> @@ -142,6 +142,9 @@ for cname in wantfunctions:
> if name[0:19] == "virDomainFSInfoFree":
> continue
>
> + if name[0:26] == "virDomainIOThreadsInfoFree":
> + continue
> +
> if name[0:21] == "virDomainListGetStats":
> name = "virConnectDomainListGetStats"
>
> --
> 2.1.0
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list