Move iothreadspin information into iothreadids

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

Adjust the test output because our printing goes in order of the iothreadids
list now.

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>

src/conf/domain_conf.c

@@ -2103,11 +2103,25 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin)
 }
 
 
+static bool
+virDomainIOThreadIDArrayHasPin(virDomainDefPtr def)
+{
+    size_t i;
+
+    for (i = 0; i < def->niothreadids; i++) {
+        if (def->iothreadids[i]->cpumask)
+            return true;
+    }
+    return false;
+}
+
+
 void
 virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def)
 {
     if (!def)
         return;
+    virBitmapFree(def->cpumask);
     VIR_FREE(def);
 }
 
@@ -2330,9 +2344,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);
@@ -13324,74 +13335,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;
+        goto cleanup;
     }
 
-    /* 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;
+    if (!(iothrid = virDomainIOThreadIDFind(def, iothreadid))) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Cannot find 'iothread' : %u"),
+                       iothreadid);
     }
 
-    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->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;
 }
 
 
@@ -14275,27 +14289,9 @@ 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;
     }
     VIR_FREE(nodes);
 
@@ -14409,7 +14405,8 @@ virDomainDefParseXML(xmlDocPtr xml,
 
     if (virDomainNumatuneHasPlacementAuto(def->numa) &&
         !def->cpumask && !def->cputune.vcpupin &&
-        !def->cputune.emulatorpin && !def->cputune.iothreadspin)
+        !def->cputune.emulatorpin &&
+        !virDomainIOThreadIDArrayHasPin(def))
         def->placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO;
 
     if ((n = virXPathNodeSet("./resource", ctxt, &nodes)) < 0) {
@@ -20809,7 +20806,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
         def->cputune.period || def->cputune.quota ||
         def->cputune.emulatorpin ||
         def->cputune.emulator_period || def->cputune.emulator_quota ||
-        def->cputune.niothreadspin ||
+        virDomainIOThreadIDArrayHasPin(def) ||
         def->cputune.vcpusched || def->cputune.iothreadsched) {
         virBufferAddLit(buf, "<cputune>\n");
         cputune = true;
@@ -20863,16 +20860,17 @@ virDomainDefFormatInternal(virDomainDefPtr def,
         VIR_FREE(cpumask);
     }
 
-    for (i = 0; i < def->cputune.niothreadspin; i++) {
+    for (i = 0; i < def->niothreadids; i++) {
         char *cpumask;
-        /* Ignore the iothreadpin which inherit from "cpuset of "<vcpu>." */
-        if (virBitmapEqual(def->cpumask, def->cputune.iothreadspin[i]->cpumask))
+
+        /* Ignore iothreadids with no cpumask */
+        if (!def->iothreadids[i]->cpumask)
             continue;
 
         virBufferAsprintf(buf, "<iothreadpin iothread='%u' ",
-                          def->cputune.iothreadspin[i]->id);
+                          def->iothreadids[i]->iothread_id);
 
-        if (!(cpumask = virBitmapFormat(def->cputune.iothreadspin[i]->cpumask)))
+        if (!(cpumask = virBitmapFormat(def->iothreadids[i]->cpumask)))
             goto error;
 
         virBufferAsprintf(buf, "cpuset='%s'/>\n", cpumask);

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);
@@ -2074,8 +2075,6 @@ struct _virDomainCputune {
     size_t nvcpupin;
     virDomainPinDefPtr *vcpupin;
     virDomainPinDefPtr emulatorpin;
-    size_t niothreadspin;
-    virDomainPinDefPtr *iothreadspin;
 
     size_t nvcpusched;
     virDomainThreadSchedParamPtr vcpusched;

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;
@@ -1211,21 +1211,13 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
                                    VIR_CGROUP_CONTROLLER_CPUSET)) {
             virBitmapPtr cpumask = NULL;
 
-            /* default cpu masks */
-            if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)
+            if (def->iothreadids[i]->cpumask)
+                cpumask = def->iothreadids[i]->cpumask;
+            else if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)
                 cpumask = priv->autoCpuset;
             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;

src/qemu/qemu_driver.c

@@ -5991,19 +5991,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 {
@@ -6012,8 +6007,6 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef,
                 virBitmapSetAll(bitmap);
                 cpumask = bitmap;
             }
-        } else {
-            cpumask = pininfo->cpumask;
         }
         if (virBitmapToData(cpumask, &info_ret[i]->cpumap,
                             &info_ret[i]->cpumaplen) < 0)
@@ -6096,8 +6089,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] = "";
@@ -6147,33 +6138,19 @@ 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);
+                           _("iothread %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)
+        if (!(cpumask = virBitmapNewData(cpumap, maplen)))
             goto endjob;
-            newIOThreadsPinNum = 0;
-        }
 
-        if (virDomainPinAdd(&newIOThreadsPin, &newIOThreadsPinNum,
-                            cpumap, maplen, iothread_id) < 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("failed to update iothreadspin"));
-            goto endjob;
-        }
+        virBitmapFree(iothrid->cpumask);
+        iothrid->cpumask = cpumask;
 
         /* Configure the corresponding cpuset cgroup before set affinity. */
         if (virCgroupHasController(priv->cgroup,
@@ -6196,14 +6173,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;
 
@@ -6221,31 +6190,23 @@ 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)
+        if (!(cpumask = virBitmapNewData(cpumap, maplen)))
             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"));
-            goto endjob;
-        }
+
+        virBitmapFree(iothrid->cpumask);
+        iothrid->cpumask = cpumask;
 
         ret = virDomainSaveConfig(cfg->configDir, persistentDef);
         goto endjob;
@@ -6257,8 +6218,6 @@ qemuDomainPinIOThread(virDomainPtr dom,
     qemuDomainObjEndJob(driver, vm);
 
  cleanup:
-    if (newIOThreadsPin)
-        virDomainPinDefArrayFree(newIOThreadsPin, newIOThreadsPinNum);
     if (cgroup_iothread)
         virCgroupFree(&cgroup_iothread);
     if (event)

src/qemu/qemu_process.c

@@ -2443,22 +2443,16 @@ static int
 qemuProcessSetIOThreadsAffinity(virDomainObjPtr vm)
 {
     virDomainDefPtr def = vm->def;
-    virDomainPinDefPtr pininfo;
     size_t i;
     int ret = -1;
 
-    if (!def->cputune.niothreadspin)
-        return 0;
-
     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;

tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-iothreads.xml

@@ -12,8 +12,8 @@
     <vcpupin vcpu='1' cpuset='1'/>
     <vcpupin vcpu='0' cpuset='0'/>
     <emulatorpin cpuset='1'/>
-    <iothreadpin iothread='2' cpuset='3'/>
     <iothreadpin iothread='1' cpuset='2'/>
+    <iothreadpin iothread='2' cpuset='3'/>
   </cputune>
   <os>
     <type arch='i686' machine='pc'>hvm</type>