Re: [libvirt-python PATCH 1/2] Support virDomainGetIOThreadsInfo and virDomainIOThreadsInfoFree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Mar 06, 2015 at 09:18:45AM -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.ioThreadsInfo()
> dom2=con.lookupByName('noiothr-gst')
> print dom2.ioThreadsInfo()
> 
> $ python iothr.py
> [(1, [False, False, True, False]), (2, [False, False, False, True]), (3, [True, True, True, True])]
> []
> $
> 
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
>  generator.py             |  5 +++
>  libvirt-override-api.xml |  6 +++
>  libvirt-override.c       | 97 ++++++++++++++++++++++++++++++++++++++++++++++++
>  sanitytest.py            |  5 +++
>  4 files changed, 113 insertions(+)
> 
> diff --git a/generator.py b/generator.py
> index 0d48980..8165a18 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[12:]
> +        func = func[0:2].lower() + func[2:]
>      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 144479c..bb61a46 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&apos;ed set of virDomainModificationImpact'/>
> +      <return type='char *' info="list of IOThreads information including the iothread_id, the cpumap, and the cpumap length for each 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..e5f1b87 100644
> --- a/libvirt-override.c
> +++ b/libvirt-override.c
> @@ -1990,6 +1990,100 @@ libvirt_virDomainGetEmulatorPinInfo(PyObject *self ATTRIBUTE_UNUSED,
>  }
>  #endif /* LIBVIR_CHECK_VERSION(0, 10, 0) */
>  
> +#if LIBVIR_CHECK_VERSION(1, 2, 14)
> +static PyObject *
> +libvirt_virDomainGetIOThreadsInfo(PyObject *self ATTRIBUTE_UNUSED,
> +                                  PyObject *args) {

You've probably missed the braces here :)            ^

> +    virDomainPtr domain;
> +    PyObject *pyobj_domain;
> +    PyObject *py_retval = NULL;
> +    PyObject *error = NULL;
> +    virDomainIOThreadInfoPtr *iothrinfo = NULL;
> +    unsigned int flags;
> +    size_t pcpu, i;
> +    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;
> +
> +    if (niothreads < 0) {
> +        error = VIR_PY_NONE;
> +        goto cleanup;
> +    }
> +
> +    /* convert to a Python list */
> +    if ((py_retval = PyList_New(niothreads)) == NULL)
> +        goto cleanup;
> +
> +    /* NOTE: If there are zero IOThreads we will return an empty list */
> +    for (i = 0; i < niothreads; i++) {
> +        PyObject *iothrtpl = NULL;
> +        PyObject *iothrid = NULL;
> +        PyObject *iothrmap = NULL;
> +        virDomainIOThreadInfoPtr iothr = iothrinfo[i];
> +
> +        if (iothr == NULL) {
> +            error = VIR_PY_NONE;
> +            goto cleanup;
> +        }
> +
> +        if ((iothrtpl = PyTuple_New(2)) == NULL ||
> +            PyList_SetItem(py_retval, i, iothrtpl) < 0) {
> +            Py_XDECREF(iothrtpl);
> +            goto cleanup;
> +        }
> +
> +        /* 0: IOThread ID */
> +        if ((iothrid = libvirt_uintWrap(iothr->iothread_id)) == NULL ||
> +            PyTuple_SetItem(iothrtpl, 0, iothrid) < 0) {
> +            Py_XDECREF(iothrid);
> +            goto cleanup;
> +        }
> +
> +        /* 1: CPU map */
> +        if ((iothrmap = PyList_New(cpunum)) == NULL ||
> +            PyTuple_SetItem(iothrtpl, 1, iothrmap) < 0) {
> +            Py_XDECREF(iothrmap);
> +            goto cleanup;
> +        }
> +        for (pcpu = 0; pcpu < cpunum; pcpu++) {
> +            PyObject *pyused;
> +            if ((pyused = PyBool_FromLong(VIR_CPU_USED(iothr->cpumap,
> +                                                       pcpu))) == NULL) {
> +                error = VIR_PY_NONE;
> +                goto cleanup;
> +            }
> +            if (PyList_SetItem(iothrmap, pcpu, pyused) < 0) {
> +                Py_XDECREF(pyused);
> +                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);
> +    return error;
> +}

I would rather use py_retval also for the error path and define py_iothrinfo
instead error.  It would let us to have only one cleanup code and from my point
of view it's cleaner code.  It's just a suggestion and I'll give you ACK with or
without this change.

Something like this:

diff --git a/libvirt-override.c b/libvirt-override.c
index 461a750..64eb084 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -1997,7 +1997,7 @@ libvirt_virDomainGetIOThreadsInfo(PyObject *self ATTRIBUTE_UNUSED,
     virDomainPtr domain;
     PyObject *pyobj_domain;
     PyObject *py_retval = NULL;
-    PyObject *error = NULL;
+    PyObject *py_iothrinfo = NULL;
     virDomainIOThreadInfoPtr *iothrinfo = NULL;
     unsigned int flags;
     size_t pcpu, i;
@@ -2016,12 +2016,12 @@ libvirt_virDomainGetIOThreadsInfo(PyObject *self ATTRIBUTE_UNUSED,
     LIBVIRT_END_ALLOW_THREADS;
 
     if (niothreads < 0) {
-        error = VIR_PY_NONE;
+        py_retval = VIR_PY_NONE;
         goto cleanup;
     }
 
     /* convert to a Python list */
-    if ((py_retval = PyList_New(niothreads)) == NULL)
+    if ((py_iothrinfo = PyList_New(niothreads)) == NULL)
         goto cleanup;
 
     /* NOTE: If there are zero IOThreads we will return an empty list */
@@ -2032,12 +2032,12 @@ libvirt_virDomainGetIOThreadsInfo(PyObject *self ATTRIBUTE_UNUSED,
         virDomainIOThreadInfoPtr iothr = iothrinfo[i];
 
         if (iothr == NULL) {
-            error = VIR_PY_NONE;
+            py_retval = VIR_PY_NONE;
             goto cleanup;
         }
 
         if ((iothrtpl = PyTuple_New(2)) == NULL ||
-            PyList_SetItem(py_retval, i, iothrtpl) < 0) {
+            PyList_SetItem(py_iothrinfo, i, iothrtpl) < 0) {
             Py_XDECREF(iothrtpl);
             goto cleanup;
         }
@@ -2059,7 +2059,7 @@ libvirt_virDomainGetIOThreadsInfo(PyObject *self ATTRIBUTE_UNUSED,
             PyObject *pyused;
             if ((pyused = PyBool_FromLong(VIR_CPU_USED(iothr->cpumap,
                                                        pcpu))) == NULL) {
-                error = VIR_PY_NONE;
+                py_retval = VIR_PY_NONE;
                 goto cleanup;
             }
             if (PyList_SetItem(iothrmap, pcpu, pyused) < 0) {
@@ -2069,18 +2069,15 @@ libvirt_virDomainGetIOThreadsInfo(PyObject *self ATTRIBUTE_UNUSED,
         }
     }
 
-    for (i = 0; i < niothreads; i++)
-        virDomainIOThreadsInfoFree(iothrinfo[i]);
-    VIR_FREE(iothrinfo);
-
-    return py_retval;
+    py_retval = py_iothrinfo;
+    py_iothrinfo = NULL;
 
 cleanup:
     for (i = 0; i < niothreads; i++)
         virDomainIOThreadsInfoFree(iothrinfo[i]);
     VIR_FREE(iothrinfo);
-    Py_XDECREF(py_retval);
-    return error;
+    Py_XDECREF(py_iothrinfo);
+    return py_retval;
 }
 
 static PyObject *

> +
> +#endif /* LIBVIR_CHECK_VERSION(1, 2, 14) */
>  
>  /************************************************************************
>   *									*
> @@ -8483,6 +8577,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, 14)
> +    {(char *) "virDomainGetIOThreadsInfo", libvirt_virDomainGetIOThreadsInfo, METH_VARARGS, NULL},
> +#endif /* LIBVIR_CHECK_VERSION(1, 2, 14) */
>      {(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..0e6e0e5 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"
>  
> @@ -276,6 +279,8 @@ for name in sorted(basicklassmap):
>          func = "nwfilter" + func[8:]
>      if func[0:8] == "fSFreeze" or func[0:6] == "fSThaw" or func[0:6] == "fSInfo":
>          func = "fs" + func[2:]
> +    if func[0:13] == "iOThreadsInfo":
> +        func = "ioThreadsInfo"
>  
>      if klass == "virNetwork":
>          func = func.replace("dHCP", "DHCP")
> -- 
> 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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]