qemu: Use domain iothreadids to IOThread's 'thread_id'

Add 'thread_id' to the virDomainIOThreadIDDef as a means to store the
'thread_id' as returned from the live qemu monitor data.

Remove the iothreadpids list from _qemuDomainObjPrivate and replace with
the new iothreadids 'thread_id' element.

Rather than use the default numbering scheme of 1..number of iothreads
defined for the domain, use the iothreadid's list for the iothread_id

Since iothreadids list keeps track of the iothread_id's, these are
now used in place of the many places where a for loop would "know"
that the ID was "+ 1" from the array element.

The new tests ensure usage of the <iothreadid> values for an exact number
of iothreads and the usage of a smaller number of <iothreadid> values than
iothreads that exist (and usage of the default numbering scheme).

src/conf/domain_conf.h

@@ -2060,6 +2060,7 @@ typedef virDomainIOThreadIDDef *virDomainIOThreadIDDefPtr;
 struct _virDomainIOThreadIDDef {
     bool autofill;
     unsigned int iothread_id;
+    int thread_id;
 };
 
 void virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def);

src/qemu/qemu_cgroup.c

@@ -802,8 +802,9 @@ qemuRestoreCgroupState(virDomainObjPtr vm)
         virCgroupFree(&cgroup_temp);
     }
 
-    for (i = 0; i < priv->niothreadpids; i++) {
-        if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, i + 1,
+    for (i = 0; i < vm->def->niothreadids; i++) {
+        if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD,
+                               vm->def->iothreadids[i]->iothread_id,
                                false, &cgroup_temp) < 0 ||
             virCgroupSetCpusetMemoryMigrate(cgroup_temp, true) < 0 ||
             virCgroupGetCpusetMems(cgroup_temp, &nodeset) < 0 ||
@@ -1175,11 +1176,6 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
     if (priv->cgroup == NULL)
         return 0;
 
-    if (def->iothreads && priv->niothreadpids == 0) {
-        VIR_WARN("Unable to get iothreads' pids.");
-        return 0;
-    }
-
     if (virDomainNumatuneGetMode(vm->def->numa, -1) ==
         VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
         virDomainNumatuneMaybeFormatNodeset(vm->def->numa,
@@ -1187,16 +1183,18 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
                                             &mem_mask, -1) < 0)
         goto cleanup;
 
-    for (i = 0; i < priv->niothreadpids; i++) {
+    for (i = 0; i < def->niothreadids; i++) {
         /* IOThreads are numbered 1..n, although the array is 0..n-1,
          * so we will account for that here
          */
-        if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, i + 1,
+        if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD,
+                               def->iothreadids[i]->iothread_id,
                                true, &cgroup_iothread) < 0)
             goto cleanup;
 
         /* move the thread for iothread to sub dir */
-        if (virCgroupAddTask(cgroup_iothread, priv->iothreadpids[i]) < 0)
+        if (virCgroupAddTask(cgroup_iothread,
+                             def->iothreadids[i]->thread_id) < 0)
             goto cleanup;
 
         if (period || quota) {
@@ -1221,8 +1219,8 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
 
             /* specific cpu mask */
             for (j = 0; j < def->cputune.niothreadspin; j++) {
-                /* IOThreads are numbered/named 1..n */
-                if (def->cputune.iothreadspin[j]->id == i + 1) {
+                if (def->cputune.iothreadspin[j]->id ==
+                    def->iothreadids[i]->iothread_id) {
                     cpumask = def->cputune.iothreadspin[j]->cpumask;
                     break;
                 }

src/qemu/qemu_command.c

@@ -677,6 +677,24 @@ qemuOpenVhostNet(virDomainDefPtr def,
     return -1;
 }
 
+int
+qemuDomainParseIOThreadAlias(char *alias,
+                             unsigned int *iothread_id)
+{
+    unsigned int idval;
+
+    if (virStrToLong_ui(alias + strlen("iothread"),
+                        NULL, 10, &idval) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("failed to find iothread id for '%s'"),
+                       alias);
+        return -1;
+    }
+
+    *iothread_id = idval;
+    return 0;
+}
+
 int
 qemuNetworkPrepareDevices(virDomainDefPtr def)
 {
@@ -3985,11 +4003,11 @@ qemuCheckIOThreads(virDomainDefPtr def,
         return false;
     }
 
-    /* Value larger than iothreads available? */
-    if (disk->iothread > def->iothreads) {
+    /* Can we find the disk iothread in the iothreadid list? */
+    if (!virDomainIOThreadIDFind(def, disk->iothread)) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                       _("Disk iothread '%u' invalid only %u IOThreads"),
-                       disk->iothread, def->iothreads);
+                       _("Disk iothread '%u' not defined in iothreadid"),
+                       disk->iothread);
         return false;
     }
 
@@ -8794,14 +8812,15 @@ qemuBuildCommandLine(virConnectPtr conn,
 
     if (def->iothreads > 0 &&
         virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
-        /* Create named iothread objects starting with 1. These may be used
+        /* Create iothread objects using the defined iothreadids list
+         * and the defined id and name from the list. These may be used
          * by a disk definition which will associate to an iothread by
-         * supplying a value of 1 up to the number of iothreads available
-         * (since 0 would indicate to not use the feature).
+         * supplying a value of an id from the list
          */
-        for (i = 1; i <= def->iothreads; i++) {
+        for (i = 0; i < def->niothreadids; i++) {
             virCommandAddArg(cmd, "-object");
-            virCommandAddArgFormat(cmd, "iothread,id=iothread%zu", i);
+            virCommandAddArgFormat(cmd, "iothread,id=iothread%u",
+                                   def->iothreadids[i]->iothread_id);
         }
     }
 

src/qemu/qemu_command.h

@@ -238,6 +238,9 @@ int qemuOpenVhostNet(virDomainDefPtr def,
                      int *vhostfd,
                      size_t *vhostfdSize);
 
+int qemuDomainParseIOThreadAlias(char *alias,
+                                 unsigned int *iothread_id);
+
 int qemuNetworkPrepareDevices(virDomainDefPtr def);
 
 /*

src/qemu/qemu_domain.c

@@ -455,7 +455,6 @@ qemuDomainObjPrivateFree(void *data)
     virDomainChrSourceDefFree(priv->monConfig);
     qemuDomainObjFreeJob(priv);
     VIR_FREE(priv->vcpupids);
-    VIR_FREE(priv->iothreadpids);
     VIR_FREE(priv->lockState);
     VIR_FREE(priv->origname);
 
@@ -516,18 +515,6 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data)
         virBufferAddLit(buf, "</vcpus>\n");
     }
 
-    if (priv->niothreadpids) {
-        size_t i;
-        virBufferAddLit(buf, "<iothreads>\n");
-        virBufferAdjustIndent(buf, 2);
-        for (i = 0; i < priv->niothreadpids; i++) {
-            virBufferAsprintf(buf, "<iothread pid='%d'/>\n",
-                              priv->iothreadpids[i]);
-        }
-        virBufferAdjustIndent(buf, -2);
-        virBufferAddLit(buf, "</iothreads>\n");
-    }
-
     if (priv->qemuCaps) {
         size_t i;
         virBufferAddLit(buf, "<qemuCaps>\n");
@@ -648,29 +635,6 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data)
         VIR_FREE(nodes);
     }
 
-    n = virXPathNodeSet("./iothreads/iothread", ctxt, &nodes);
-    if (n < 0)
-        goto error;
-    if (n) {
-        priv->niothreadpids = n;
-        if (VIR_REALLOC_N(priv->iothreadpids, priv->niothreadpids) < 0)
-            goto error;
-
-        for (i = 0; i < n; i++) {
-            char *pidstr = virXMLPropString(nodes[i], "pid");
-            if (!pidstr)
-                goto error;
-
-            if (virStrToLong_i(pidstr, NULL, 10,
-                               &(priv->iothreadpids[i])) < 0) {
-                VIR_FREE(pidstr);
-                goto error;
-            }
-            VIR_FREE(pidstr);
-        }
-        VIR_FREE(nodes);
-    }
-
     if ((n = virXPathNodeSet("./qemuCaps/flag", ctxt, &nodes)) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "%s", _("failed to parse qemu capabilities flags"));

src/qemu/qemu_domain.h

@@ -162,9 +162,6 @@ struct _qemuDomainObjPrivate {
     int nvcpupids;
     int *vcpupids;
 
-    int niothreadpids;
-    int *iothreadpids;
-
     virDomainPCIAddressSetPtr pciaddrs;
     virDomainCCWAddressSetPtr ccwaddrs;
     virDomainVirtioSerialAddrSetPtr vioserialaddrs;

src/qemu/qemu_driver.c

@@ -5926,14 +5926,16 @@ qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver,
         goto endjob;
 
     for (i = 0; i < niothreads; i++) {
+        unsigned int iothread_id;
         virBitmapPtr map = NULL;
 
-        if (VIR_ALLOC(info_ret[i]) < 0)
+        if (qemuDomainParseIOThreadAlias(iothreads[i]->name,
+                                         &iothread_id) < 0)
             goto endjob;
 
-        if (virStrToLong_ui(iothreads[i]->name + strlen("iothread"), NULL, 10,
-                            &info_ret[i]->iothread_id) < 0)
+        if (VIR_ALLOC(info_ret[i]) < 0)
             goto endjob;
+        info_ret[i]->iothread_id = iothread_id;
 
         if (virProcessGetAffinity(iothreads[i]->thread_id, &map, hostcpus) < 0)
             goto endjob;
@@ -5988,19 +5990,19 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef,
     if (VIR_ALLOC_N(info_ret, targetDef->iothreads) < 0)
         goto cleanup;
 
-    for (i = 0; i < targetDef->iothreads; i++) {
+    for (i = 0; i < targetDef->niothreadids; i++) {
         virDomainPinDefPtr pininfo;
 
         if (VIR_ALLOC(info_ret[i]) < 0)
             goto cleanup;
 
-        /* IOThreads being counting at 1 */
-        info_ret[i]->iothread_id = i + 1;
+        /* 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,
-                                   i + 1);
+                                   targetDef->iothreadids[i]->iothread_id);
         if (!pininfo) {
             if (targetDef->cpumask) {
                 cpumask = targetDef->cpumask;
@@ -6144,16 +6146,11 @@ qemuDomainPinIOThread(virDomainPtr dom,
 
     if (flags & VIR_DOMAIN_AFFECT_LIVE) {
 
-        if (priv->iothreadpids == NULL) {
-            virReportError(VIR_ERR_OPERATION_INVALID,
-                           "%s", _("IOThread affinity is not supported"));
-            goto endjob;
-        }
+        virDomainIOThreadIDDefPtr iothrid;
 
-        if (iothread_id > priv->niothreadpids) {
+        if (!(iothrid = virDomainIOThreadIDFind(vm->def, iothread_id))) {
             virReportError(VIR_ERR_INVALID_ARG,
-                           _("iothread value out of range %d > %d"),
-                           iothread_id, priv->niothreadpids);
+                           _("iothread value %d not found"), iothread_id);
             goto endjob;
         }
 
@@ -6191,8 +6188,7 @@ qemuDomainPinIOThread(virDomainPtr dom,
                 goto endjob;
             }
         } else {
-            if (virProcessSetAffinity(priv->iothreadpids[iothread_id - 1],
-                                      pcpumap) < 0) {
+            if (virProcessSetAffinity(iothrid->thread_id, pcpumap) < 0) {
                 virReportError(VIR_ERR_SYSTEM_ERROR,
                                _("failed to set cpu affinity for IOThread %d"),
                                iothread_id);
@@ -10163,8 +10159,9 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm,
         virCgroupFree(&cgroup_temp);
     }
 
-    for (i = 0; i < priv->niothreadpids; i++) {
-        if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, i + 1,
+    for (i = 0; i < vm->def->niothreadids; i++) {
+        if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD,
+                               vm->def->iothreadids[i]->iothread_id,
                                false, &cgroup_temp) < 0 ||
             virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0)
             goto cleanup;

src/qemu/qemu_process.c

@@ -2247,12 +2247,21 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver,
         goto cleanup;
     }
 
-    if (VIR_ALLOC_N(priv->iothreadpids, niothreads) < 0)
-        goto cleanup;
-    priv->niothreadpids = niothreads;
+    for (i = 0; i < niothreads; i++) {
+        unsigned int iothread_id;
+        virDomainIOThreadIDDefPtr iothrid;
+
+        if (qemuDomainParseIOThreadAlias(iothreads[i]->name,
+                                         &iothread_id) < 0)
+            goto cleanup;
 
-    for (i = 0; i < priv->niothreadpids; i++)
-        priv->iothreadpids[i] = iothreads[i]->thread_id;
+        if (!(iothrid = virDomainIOThreadIDFind(vm->def, iothread_id))) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("iothread %d not found"), iothread_id);
+            goto cleanup;
+        }
+        iothrid->thread_id = iothreads[i]->thread_id;
+    }
 
     ret = 0;
 
@@ -2433,7 +2442,6 @@ qemuProcessSetEmulatorAffinity(virDomainObjPtr vm)
 static int
 qemuProcessSetIOThreadsAffinity(virDomainObjPtr vm)
 {
-    qemuDomainObjPrivatePtr priv = vm->privateData;
     virDomainDefPtr def = vm->def;
     virDomainPinDefPtr pininfo;
     size_t i;
@@ -2442,20 +2450,15 @@ qemuProcessSetIOThreadsAffinity(virDomainObjPtr vm)
     if (!def->cputune.niothreadspin)
         return 0;
 
-    if (priv->iothreadpids == NULL) {
-        virReportError(VIR_ERR_OPERATION_INVALID,
-                       "%s", _("IOThread affinity is not supported"));
-        return -1;
-    }
-
-    for (i = 0; i < def->iothreads; i++) {
-        /* set affinity only for existing vcpus */
+    for (i = 0; i < def->niothreadids; i++) {
+        /* set affinity only for existing iothreads */
         if (!(pininfo = virDomainPinFind(def->cputune.iothreadspin,
                                          def->cputune.niothreadspin,
-                                         i + 1)))
+                                         def->iothreadids[i]->iothread_id)))
             continue;
 
-        if (virProcessSetAffinity(priv->iothreadpids[i], pininfo->cpumask) < 0)
+        if (virProcessSetAffinity(def->iothreadids[i]->thread_id,
+                                  pininfo->cpumask) < 0)
             goto cleanup;
     }
     ret = 0;
@@ -2505,8 +2508,9 @@ qemuProcessSetSchedulers(virDomainObjPtr vm)
             return -1;
     }
 
-    for (i = 0; i < priv->niothreadpids; i++) {
-        if (qemuProcessSetSchedParams(i + 1, priv->iothreadpids[i],
+    for (i = 0; i < vm->def->niothreadids; i++) {
+        if (qemuProcessSetSchedParams(vm->def->iothreadids[i]->iothread_id,
+                                      vm->def->iothreadids[i]->thread_id,
                                       vm->def->cputune.niothreadsched,
                                       vm->def->cputune.iothreadsched) < 0)
             return -1;
@@ -5294,8 +5298,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
     virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
     VIR_FREE(priv->vcpupids);
     priv->nvcpupids = 0;
-    VIR_FREE(priv->iothreadpids);
-    priv->niothreadpids = 0;
+    for (i = 0; i < vm->def->niothreadids; i++)
+        vm->def->iothreadids[i]->thread_id = 0;
     virObjectUnref(priv->qemuCaps);
     priv->qemuCaps = NULL;
     VIR_FREE(priv->pidfile);

tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.args

+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M \
+pc -m 214 -smp 2 \
+-object iothread,id=iothread5 \
+-object iothread,id=iothread6 \
+-object iothread,id=iothread1 \
+-object iothread,id=iothread2 \
+-nographic -monitor \
+unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \
+/dev/HostVG/QEMUGuest1 -net none -serial none -parallel none

tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.xml

+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>2</vcpu>
+  <iothreads>4</iothreads>
+  <iothreadids>
+    <iothread id='5'/>
+    <iothread id='6'/>
+  </iothreadids>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <disk type='block' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='usb' index='0'/>
+    <controller type='ide' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>

tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.args

+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M \
+pc -m 214 -smp 2 \
+-object iothread,id=iothread2 \
+-object iothread,id=iothread4 \
+-nographic -monitor \
+unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \
+/dev/HostVG/QEMUGuest1 -net none -serial none -parallel none

tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.xml

+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>2</vcpu>
+  <iothreads>2</iothreads>
+  <iothreadids>
+    <iothread id='2'/>
+    <iothread id='4'/>
+  </iothreadids>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <disk type='block' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='usb' index='0'/>
+    <controller type='ide' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>

tests/qemuxml2argvtest.c

@@ -1217,6 +1217,8 @@ mymain(void)
     DO_TEST("smp", QEMU_CAPS_SMP_TOPOLOGY);
 
     DO_TEST("iothreads", QEMU_CAPS_OBJECT_IOTHREAD);
+    DO_TEST("iothreads-ids", QEMU_CAPS_OBJECT_IOTHREAD);
+    DO_TEST("iothreads-ids-partial", QEMU_CAPS_OBJECT_IOTHREAD);
     DO_TEST("iothreads-disk", QEMU_CAPS_OBJECT_IOTHREAD, QEMU_CAPS_DEVICE,
             QEMU_CAPS_DRIVE);
     DO_TEST("iothreads-disk-virtio-ccw", QEMU_CAPS_OBJECT_IOTHREAD, QEMU_CAPS_DEVICE,

tests/qemuxml2xmltest.c

@@ -493,6 +493,8 @@ mymain(void)
 
     DO_TEST("smp");
     DO_TEST("iothreads");
+    DO_TEST("iothreads-ids");
+    DO_TEST("iothreads-ids-partial");
     DO_TEST_DIFFERENT("cputune-iothreads");
     DO_TEST("iothreads-disk");
     DO_TEST("iothreads-disk-virtio-ccw");