[libvirt] [PATCH v4 4/9] Move iothreadspin information into iothreadids

Tuesday, 21 April 2015

Remove the iothreadspin array from cputune and replace with a cpumask
to be stored in the iothreadids list

Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt;
---
 docs/formatdomain.html.in |  10 ++--
 src/conf/domain_conf.c    | 118 +++++++++++++++++++++-------------------------
 src/conf/domain_conf.h    |   2 +-
 src/qemu/qemu_cgroup.c    |  13 ++---
 src/qemu/qemu_driver.c    |  79 +++++++++----------------------
 src/qemu/qemu_process.c   |   7 +--
 6 files changed, 86 insertions(+), 143 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 518f7c5..7af6bd7 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -624,11 +624,11 @@
          and attribute <code>cpuset</code> of element
<code>vcpu</code> is
          not specified, the IOThreads are pinned to all the physical CPUs
          by default. There are two required attributes, the attribute
-         <code>iothread</code> specifies the IOThread id and the attribute
-         <code>cpuset</code> specifying which physical CPUs to pin to. The
-         <code>iothread</code> value begins at "1" through the
number of
-          <a
href="#elementsIOThreadsAllocation"><code>iothreads</code></a>
-         allocated to the domain. A value of "0" is not permitted.
+         <code>iothread</code> specifies the IOThread ID and the attribute
+         <code>cpuset</code> specifying which physical CPUs to pin to. See
+         the <code>iothreadids</code>
+         <a
href="#elementsIOThreadsAllocation"><code>description</code></a>
+         for valid <code>iothread</code> values.
         <span class="since">Since 1.2.9</span>
        </dd>
       <dt><code>shares</code></dt>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index bd25d52..969e56f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2118,8 +2118,10 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin)
 void
 virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def)
 {
-    if (def)
+    if (def) {
+        virBitmapFree(def->cpumask);
         VIR_FREE(def);
+    }
 }
 
 
@@ -2341,9 +2343,6 @@ void virDomainDefFree(virDomainDefPtr def)
 
     virDomainPinDefFree(def->cputune.emulatorpin);
 
-    virDomainPinDefArrayFree(def->cputune.iothreadspin,
-                                 def->cputune.niothreadspin);
-
     for (i = 0; i < def->cputune.nvcpusched; i++)
         virBitmapFree(def->cputune.vcpusched[i].ids);
     VIR_FREE(def->cputune.vcpusched);
@@ -13316,74 +13315,77 @@ virDomainVcpuPinDefParseXML(xmlNodePtr node,
  * and an iothreadspin has the form
  *   <iothreadpin iothread='1' cpuset='2'/>
  */
-static virDomainPinDefPtr
+static int
 virDomainIOThreadPinDefParseXML(xmlNodePtr node,
                                 xmlXPathContextPtr ctxt,
-                                int iothreads)
+                                virDomainDefPtr def)
 {
-    virDomainPinDefPtr def;
+    int ret = -1;
+    virDomainIOThreadIDDefPtr iothrid;
+    virBitmapPtr cpumask;
     xmlNodePtr oldnode = ctxt->node;
     unsigned int iothreadid;
     char *tmp = NULL;
 
-    if (VIR_ALLOC(def) < 0)
-        return NULL;
-
     ctxt->node = node;
 
     if (!(tmp = virXPathString("string(./@iothread)", ctxt))) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
                        _("missing iothread id in iothreadpin"));
-        goto error;
+        goto cleanup;
     }
 
     if (virStrToLong_uip(tmp, NULL, 10, &iothreadid) < 0) {
         virReportError(VIR_ERR_XML_ERROR,
                        _("invalid setting for iothread '%s'"), tmp);
-        goto error;
+        goto cleanup;
     }
     VIR_FREE(tmp);
 
     if (iothreadid == 0) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
                        _("zero is an invalid iothread id value"));
-        goto error;
-    }
-
-    /* IOThreads are numbered "iothread1...iothread<n>", where
-     * "n" is the iothreads value */
-    if (iothreadid > iothreads) {
-        virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("iothread id must not exceed iothreads"));
-        goto error;
+        goto cleanup;
     }
 
-    def->id = iothreadid;
-
     if (!(tmp = virXMLPropString(node, "cpuset"))) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("missing cpuset for iothreadpin"));
-        goto error;
+        goto cleanup;
     }
 
-    if (virBitmapParse(tmp, 0, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0)
-        goto error;
+    if (virBitmapParse(tmp, 0, &cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0)
+        goto cleanup;
 
-    if (virBitmapIsAllClear(def->cpumask)) {
+    if (virBitmapIsAllClear(cpumask)) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("Invalid value of 'cpuset': %s"),
                        tmp);
-        goto error;
+        goto cleanup;
+    }
+
+    if (!(iothrid = virDomainIOThreadIDFind(def, iothreadid))) {
+        if (!(iothrid = virDomainIOThreadIDAdd(def, iothreadid)))
+            goto cleanup;
+        iothrid->autofill = true;
+    }
+
+    if (iothrid->cpumask) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("duplicate iothreadpin for same iothread
'%u'"),
+                       iothreadid);
+        goto cleanup;
     }
 
+    iothrid->cpumask = cpumask;
+    cpumask = NULL;
+    ret = 0;
+
  cleanup:
     VIR_FREE(tmp);
+    virBitmapFree(cpumask);
     ctxt->node = oldnode;
-    return def;
-
- error:
-    VIR_FREE(def);
-    goto cleanup;
+    return ret;
 }
 
 
@@ -14250,28 +14252,11 @@ virDomainDefParseXML(xmlDocPtr xml,
         goto error;
     }
 
-    if (n && VIR_ALLOC_N(def->cputune.iothreadspin, n) < 0)
-        goto error;
-
     for (i = 0; i < n; i++) {
-        virDomainPinDefPtr iothreadpin = NULL;
-        iothreadpin = virDomainIOThreadPinDefParseXML(nodes[i], ctxt,
-                                                      def->iothreads);
-        if (!iothreadpin)
+        if (virDomainIOThreadPinDefParseXML(nodes[i], ctxt, def) < 0)
             goto error;
-
-        if (virDomainPinIsDuplicate(def->cputune.iothreadspin,
-                                    def->cputune.niothreadspin,
-                                    iothreadpin->id)) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("duplicate iothreadpin for same iothread"));
-            virDomainPinDefFree(iothreadpin);
-            goto error;
-        }
-
-        def->cputune.iothreadspin[def->cputune.niothreadspin++] =
-            iothreadpin;
     }
+    def->cputune.niothreadspin = n;
     VIR_FREE(nodes);
 
     if ((n = virXPathNodeSet("./cputune/vcpusched", ctxt, &nodes)) < 0)
{
@@ -14384,7 +14369,7 @@ virDomainDefParseXML(xmlDocPtr xml,
 
     if (virDomainNumatuneHasPlacementAuto(def->numa) &&
         !def->cpumask && !def->cputune.vcpupin &&
-        !def->cputune.emulatorpin && !def->cputune.iothreadspin)
+        !def->cputune.emulatorpin && !def->cputune.niothreadspin)
         def->placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO;
 
     if ((n = virXPathNodeSet("./resource", ctxt, &nodes)) < 0) {
@@ -20837,20 +20822,25 @@ virDomainDefFormatInternal(virDomainDefPtr def,
         VIR_FREE(cpumask);
     }
 
-    for (i = 0; i < def->cputune.niothreadspin; i++) {
-        char *cpumask;
-        /* Ignore the iothreadpin which inherit from "cpuset of
"<vcpu>." */
-        if (virBitmapEqual(def->cpumask,
def->cputune.iothreadspin[i]->cpumask))
-            continue;
+    if (def->cputune.niothreadspin) {
+        for (i = 0; i < def->niothreadids; i++) {
+            char *cpumask;
 
-        virBufferAsprintf(buf, "<iothreadpin iothread='%u' ",
-                          def->cputune.iothreadspin[i]->id);
+            /* Ignore iothreadids with no cpumask or a cpumask that
+             * inherits from "cpuset of "<vcpu>." */
+            if (!def->iothreadids[i]->cpumask ||
+                virBitmapEqual(def->cpumask, def->iothreadids[i]->cpumask))
+                continue;
 
-        if (!(cpumask = virBitmapFormat(def->cputune.iothreadspin[i]->cpumask)))
-            goto error;
+            virBufferAsprintf(buf, "<iothreadpin iothread='%u' ",
+                              def->iothreadids[i]->iothread_id);
 
-        virBufferAsprintf(buf, "cpuset='%s'/>\n", cpumask);
-        VIR_FREE(cpumask);
+            if (!(cpumask = virBitmapFormat(def->iothreadids[i]->cpumask)))
+                goto error;
+
+            virBufferAsprintf(buf, "cpuset='%s'/>\n", cpumask);
+            VIR_FREE(cpumask);
+        }
     }
 
     for (i = 0; i < def->cputune.nvcpusched; i++) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index cc98f3d..c71e1b8 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2057,6 +2057,7 @@ struct _virDomainIOThreadIDDef {
     bool autofill;
     unsigned int iothread_id;
     int thread_id;
+    virBitmapPtr cpumask;
 };
 
 void virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def);
@@ -2075,7 +2076,6 @@ struct _virDomainCputune {
     virDomainPinDefPtr *vcpupin;
     virDomainPinDefPtr emulatorpin;
     size_t niothreadspin;
-    virDomainPinDefPtr *iothreadspin;
 
     size_t nvcpusched;
     virDomainThreadSchedParamPtr vcpusched;
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index cdf0aaf..5282449 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -1149,7 +1149,7 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
     virCgroupPtr cgroup_iothread = NULL;
     qemuDomainObjPrivatePtr priv = vm->privateData;
     virDomainDefPtr def = vm->def;
-    size_t i, j;
+    size_t i;
     unsigned long long period = vm->def->cputune.period;
     long long quota = vm->def->cputune.quota;
     char *mem_mask = NULL;
@@ -1214,18 +1214,11 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
             /* default cpu masks */
             if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)
                 cpumask = priv->autoCpuset;
+            else if (def->iothreadids[i]->cpumask)
+                cpumask = def->iothreadids[i]->cpumask;
             else
                 cpumask = def->cpumask;
 
-            /* specific cpu mask */
-            for (j = 0; j < def->cputune.niothreadspin; j++) {
-                if (def->cputune.iothreadspin[j]->id ==
-                    def->iothreadids[i]->iothread_id) {
-                    cpumask = def->cputune.iothreadspin[j]->cpumask;
-                    break;
-                }
-            }
-
             if (cpumask &&
                 qemuSetupCgroupCpusetCpus(cgroup_iothread, cpumask) < 0)
                 goto cleanup;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0f95cc7..ee13d08 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5894,19 +5894,14 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef,
         goto cleanup;
 
     for (i = 0; i < targetDef->niothreadids; i++) {
-        virDomainPinDefPtr pininfo;
-
         if (VIR_ALLOC(info_ret[i]) < 0)
             goto cleanup;
 
         /* IOThread ID's are taken from the iothreadids list */
         info_ret[i]->iothread_id = targetDef->iothreadids[i]->iothread_id;
 
-        /* Initialize the cpumap */
-        pininfo = virDomainPinFind(targetDef->cputune.iothreadspin,
-                                   targetDef->cputune.niothreadspin,
-                                   targetDef->iothreadids[i]->iothread_id);
-        if (!pininfo) {
+        cpumask = targetDef->iothreadids[i]->cpumask;
+        if (!cpumask) {
             if (targetDef->cpumask) {
                 cpumask = targetDef->cpumask;
             } else {
@@ -5915,8 +5910,6 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef,
                 virBitmapSetAll(bitmap);
                 cpumask = bitmap;
             }
-        } else {
-            cpumask = pininfo->cpumask;
         }
         if (virBitmapToData(cpumask, &info_ret[i]->cpumap,
                             &info_ret[i]->cpumaplen) < 0)
@@ -5999,8 +5992,6 @@ qemuDomainPinIOThread(virDomainPtr dom,
     virDomainDefPtr persistentDef = NULL;
     virBitmapPtr pcpumap = NULL;
     qemuDomainObjPrivatePtr priv;
-    virDomainPinDefPtr *newIOThreadsPin = NULL;
-    size_t newIOThreadsPinNum = 0;
     virCgroupPtr cgroup_iothread = NULL;
     virObjectEventPtr event = NULL;
     char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = "";
@@ -6050,33 +6041,21 @@ qemuDomainPinIOThread(virDomainPtr dom,
     if (flags & VIR_DOMAIN_AFFECT_LIVE) {
 
         virDomainIOThreadIDDefPtr iothrid;
+        virBitmapPtr cpumask;
 
         if (!(iothrid = virDomainIOThreadIDFind(vm->def, iothread_id))) {
             virReportError(VIR_ERR_INVALID_ARG,
-                           _("iothread value %d not found"), iothread_id);
+                           _("iothreadid %d not found"), iothread_id);
             goto endjob;
         }
 
-        if (vm->def->cputune.iothreadspin) {
-            newIOThreadsPin =
-                virDomainPinDefCopy(vm->def->cputune.iothreadspin,
-                                    vm->def->cputune.niothreadspin);
-            if (!newIOThreadsPin)
-                goto endjob;
-
-            newIOThreadsPinNum = vm->def->cputune.niothreadspin;
-        } else {
-            if (VIR_ALLOC(newIOThreadsPin) < 0)
-                goto endjob;
-            newIOThreadsPinNum = 0;
-        }
-
-        if (virDomainPinAdd(&newIOThreadsPin, &newIOThreadsPinNum,
-                            cpumap, maplen, iothread_id) < 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("failed to update iothreadspin"));
+        if (!(cpumask = virBitmapNewData(cpumap, maplen)))
             goto endjob;
-        }
+
+        if (!iothrid->cpumask)
+            vm->def->cputune.niothreadspin++;
+        virBitmapFree(iothrid->cpumask);
+        iothrid->cpumask = cpumask;
 
         /* Configure the corresponding cpuset cgroup before set affinity. */
         if (virCgroupHasController(priv->cgroup,
@@ -6099,14 +6078,6 @@ qemuDomainPinIOThread(virDomainPtr dom,
             }
         }
 
-        if (vm->def->cputune.iothreadspin)
-            virDomainPinDefArrayFree(vm->def->cputune.iothreadspin,
-                                     vm->def->cputune.niothreadspin);
-
-        vm->def->cputune.iothreadspin = newIOThreadsPin;
-        vm->def->cputune.niothreadspin = newIOThreadsPinNum;
-        newIOThreadsPin = NULL;
-
         if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
             goto endjob;
 
@@ -6124,31 +6095,25 @@ qemuDomainPinIOThread(virDomainPtr dom,
     }
 
     if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+        virDomainIOThreadIDDefPtr iothrid;
+        virBitmapPtr cpumask;
+
         /* Coverity didn't realize that targetDef must be set if we got here. */
         sa_assert(persistentDef);
 
-        if (iothread_id > persistentDef->iothreads) {
+        if (!(iothrid = virDomainIOThreadIDFind(persistentDef, iothread_id))) {
             virReportError(VIR_ERR_INVALID_ARG,
-                           _("iothread value out of range %d > %d"),
-                           iothread_id, persistentDef->iothreads);
+                           _("iothreadid %d not found"), iothread_id);
             goto endjob;
         }
 
-        if (!persistentDef->cputune.iothreadspin) {
-            if (VIR_ALLOC(persistentDef->cputune.iothreadspin) < 0)
-                goto endjob;
-            persistentDef->cputune.niothreadspin = 0;
-        }
-        if (virDomainPinAdd(&persistentDef->cputune.iothreadspin,
-                            &persistentDef->cputune.niothreadspin,
-                            cpumap,
-                            maplen,
-                            iothread_id) < 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("failed to update or add iothreadspin xml "
-                             "of a persistent domain"));
+        if (!(cpumask = virBitmapNewData(cpumap, maplen)))
             goto endjob;
-        }
+
+        if (!iothrid->cpumask)
+            persistentDef->cputune.niothreadspin++;
+        virBitmapFree(iothrid->cpumask);
+        iothrid->cpumask = cpumask;
 
         ret = virDomainSaveConfig(cfg->configDir, persistentDef);
         goto endjob;
@@ -6160,8 +6125,6 @@ qemuDomainPinIOThread(virDomainPtr dom,
     qemuDomainObjEndJob(driver, vm);
 
  cleanup:
-    if (newIOThreadsPin)
-        virDomainPinDefArrayFree(newIOThreadsPin, newIOThreadsPinNum);
     if (cgroup_iothread)
         virCgroupFree(&cgroup_iothread);
     if (event)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 717d113..09263e9 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2458,7 +2458,6 @@ static int
 qemuProcessSetIOThreadsAffinity(virDomainObjPtr vm)
 {
     virDomainDefPtr def = vm->def;
-    virDomainPinDefPtr pininfo;
     size_t i;
     int ret = -1;
 
@@ -2467,13 +2466,11 @@ qemuProcessSetIOThreadsAffinity(virDomainObjPtr vm)
 
     for (i = 0; i < def->niothreadids; i++) {
         /* set affinity only for existing iothreads */
-        if (!(pininfo = virDomainPinFind(def->cputune.iothreadspin,
-                                         def->cputune.niothreadspin,
-                                         def->iothreadids[i]->iothread_id)))
+        if (!def->iothreadids[i]->cpumask)
             continue;
 
         if (virProcessSetAffinity(def->iothreadids[i]->thread_id,
-                                  pininfo->cpumask) < 0)
+                                  def->iothreadids[i]->cpumask) < 0)
             goto cleanup;
     }
     ret = 0;
-- 
2.1.0


    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

[libvirt] [PATCH v4 4/9] Move iothreadspin information into iothreadids