From e342a8e4885d43c7c598dc5d3a1146b5d8d0051b Mon Sep 17 00:00:00 2001

Message-Id: <e342a8e4885d43c7c598dc5d3a1146b5d8d0051b@dist-git>

From: John Ferlan <jferlan@redhat.com>

Date: Thu, 18 Sep 2014 09:29:55 -0400

Subject: [PATCH] domain_conf: Add iothreadpin to cputune

https://bugzilla.redhat.com/show_bug.cgi?id=1101574

Add an option 'iothreadpin' to the <cpuset> to allow for setting the

CPU affinity for each IOThread.

The iothreadspin will mimic the vcpupin with respect to being able to

assign each iothread to a specific CPU, although iothreads ids start

at 1 while vcpu ids start at 0. This matches the iothread naming scheme.

(cherry picked from commit 938fb12fad6d15c9fdb73f998c4e0ec1e278721f)

Signed-off-by: John Ferlan <jferlan@redhat.com>

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 docs/formatdomain.html.in                          |  18 +++

 docs/schemas/domaincommon.rng                      |  10 ++

 src/conf/domain_conf.c                             | 124 +++++++++++++++++++--

 src/conf/domain_conf.h                             |   2 +

 .../qemuxml2argv-cputune-iothreads.xml             |  38 +++++++

 tests/qemuxml2xmltest.c                            |   1 +

 6 files changed, 182 insertions(+), 11 deletions(-)

 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreads.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in

index b947848..8c03ebb 100644

--- a/docs/formatdomain.html.in

+++ b/docs/formatdomain.html.in

@@ -526,6 +526,8 @@

     <vcpupin vcpu="2" cpuset="2,3"/>

     <vcpupin vcpu="3" cpuset="0,4"/>

     <emulatorpin cpuset="1-3"/>

+    <iothreadpin iothread="1" cpuset="5,6"/>

+    <iothreadpin iothread="2" cpuset="7,8"/>

     <shares>2048</shares>

     <period>1000000</period>

     <quota>-1</quota>

@@ -567,6 +569,22 @@

          attribute placement of element vcpu is

          "auto".

+       
iothreadpin

+       

+         The optional iothreadpin element specifies which of host

+         physical CPUs the IOThreads will be pinned to. If this is omitted

+         and attribute cpuset of element vcpu is

+         not specified, the IOThreads are pinned to all the physical CPUs

+         by default. There are two required attributes, the attribute

+         iothread specifies the IOThread id and the attribute

+         cpuset specifying which physical CPUs to pin to. The

+         iothread value begins at "1" through the number of

+          iothreads

+         allocated to the domain. A value of "0" is not permitted.

+         NB, iothreadpin is not allowed if attribute

+         placement of element vcpu is "auto".

+        Since 1.2.9

+       

       
shares

         The optional shares element specifies the proportional

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng

index 31974ac..d61d3ad 100644

--- a/docs/schemas/domaincommon.rng

+++ b/docs/schemas/domaincommon.rng

@@ -810,6 +810,16 @@

           </attribute>

         </element>

       </optional>

+      <zeroOrMore>

+        <element name="iothreadpin">

+          <attribute name="iothread">

+            <ref name="unsignedInt"/>

+          </attribute>

+          <attribute name="cpuset">

+            <ref name="cpuset"/>

+          </attribute>

+        </element>

+      </zeroOrMore>

     </element>

   </define>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c

index 7d78b0b..c03affe 100644

--- a/src/conf/domain_conf.c

+++ b/src/conf/domain_conf.c

@@ -2169,6 +2169,9 @@ void virDomainDefFree(virDomainDefPtr def)

     virDomainVcpuPinDefFree(def->cputune.emulatorpin);

+    virDomainVcpuPinDefArrayFree(def->cputune.iothreadspin,

+                                 def->cputune.niothreadspin);

+

     virDomainNumatuneFree(def->numatune);

     virSysinfoDefFree(def->sysinfo);

@@ -11415,6 +11418,9 @@ virDomainPanicDefParseXML(xmlNodePtr node)

  * and emulatorpin has the form of

  *   <emulatorpin cpuset='0'/>

  *

+ * and an iothreadspin has the form

+ *   <iothreadpin iothread='1' cpuset='2'/>

+ *

  * A vcpuid of -1 is valid and only valid for emulatorpin. So callers

  * have to check the returned cpuid for validity.

  */

@@ -11422,11 +11428,13 @@ static virDomainVcpuPinDefPtr

 virDomainVcpuPinDefParseXML(xmlNodePtr node,

                             xmlXPathContextPtr ctxt,

                             int maxvcpus,

-                            bool emulator)

+                            bool emulator,

+                            bool iothreads)

 {

     virDomainVcpuPinDefPtr def;

     xmlNodePtr oldnode = ctxt->node;

     int vcpuid = -1;

+    unsigned int iothreadid;

     char *tmp = NULL;

     int ret;

@@ -11435,7 +11443,7 @@ virDomainVcpuPinDefParseXML(xmlNodePtr node,

     ctxt->node = node;

-    if (!emulator) {

+    if (!emulator && !iothreads) {

         ret = virXPathInt("string(./@vcpu)", ctxt, &vcpuid);

         if ((ret == -2) || (vcpuid < -1)) {

             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

@@ -11456,10 +11464,41 @@ virDomainVcpuPinDefParseXML(xmlNodePtr node,

         def->vcpuid = vcpuid;

     }

+    if (iothreads && (tmp = virXPathString("string(./@iothread)", ctxt))) {

+        if (virStrToLong_uip(tmp, NULL, 10, &iothreadid) < 0) {

+            virReportError(VIR_ERR_XML_ERROR,

+                           _("invalid setting for iothread '%s'"), tmp);

+            goto error;

+        }

+        VIR_FREE(tmp);

+

+        if (iothreadid == 0) {

+            virReportError(VIR_ERR_XML_ERROR, "%s",

+                           _("zero is an invalid iothread id value"));

+            goto error;

+        }

+

+        /* NB: maxvcpus is actually def->iothreads

+         * IOThreads are numbered "iothread1...iothread<n>", where

+         * "n" is the iothreads value

+         */

+        if (iothreadid > maxvcpus) {

+            virReportError(VIR_ERR_XML_ERROR, "%s",

+                           _("iothread id must not exceed iothreads"));

+            goto error;

+        }

+

+        /* Rather than creating our own structure we are reusing the vCPU */

+        def->vcpuid = iothreadid;

+    }

+

     if (!(tmp = virXMLPropString(node, "cpuset"))) {

         if (emulator)

             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

                            _("missing cpuset for emulatorpin"));

+        else if (iothreads)

+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

+                           _("missing cpuset for iothreadpin"));

         else

             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

                            _("missing cpuset for vcpupin"));

@@ -12138,7 +12177,7 @@ virDomainDefParseXML(xmlDocPtr xml,

     for (i = 0; i < n; i++) {

         virDomainVcpuPinDefPtr vcpupin = NULL;

         vcpupin = virDomainVcpuPinDefParseXML(nodes[i], ctxt,

-                                              def->maxvcpus, false);

+                                              def->maxvcpus, false, false);

         if (!vcpupin)

             goto error;

@@ -12213,8 +12252,9 @@ virDomainDefParseXML(xmlDocPtr xml,

                 goto error;

             }

-            def->cputune.emulatorpin = virDomainVcpuPinDefParseXML(nodes[0], ctxt,

-                                                                   0, true);

+            def->cputune.emulatorpin = virDomainVcpuPinDefParseXML(nodes[0],

+                                                                   ctxt, 0,

+                                                                   true, false);

             if (!def->cputune.emulatorpin)

                 goto error;

@@ -12224,6 +12264,49 @@ virDomainDefParseXML(xmlDocPtr xml,

     }

     VIR_FREE(nodes);

+

+    if ((n = virXPathNodeSet("./cputune/iothreadpin", ctxt, &nodes)) < 0) {

+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

+                       _("cannot extract iothreadpin nodes"));

+        goto error;

+    }

+

+    /* Ignore iothreadpin if <vcpu> placement is "auto", they

+     * conflict with each other, and <vcpu> placement can't be

+     * simply ignored, as <numatune>'s placement defaults to it.

+     */

+    if (n) {

+        if (def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) {

+            if (VIR_ALLOC_N(def->cputune.iothreadspin, n) < 0)

+                goto error;

+

+            for (i = 0; i < n; i++) {

+                virDomainVcpuPinDefPtr iothreadpin = NULL;

+                iothreadpin = virDomainVcpuPinDefParseXML(nodes[i], ctxt,

+                                                          def->iothreads,

+                                                          false, true);

+                if (!iothreadpin)

+                    goto error;

+

+                if (virDomainVcpuPinIsDuplicate(def->cputune.iothreadspin,

+                                                def->cputune.niothreadspin,

+                                                iothreadpin->vcpuid)) {

+                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

+                                   _("duplicate iothreadpin for same iothread"));

+                    virDomainVcpuPinDefFree(iothreadpin);

+                    goto error;

+                }

+

+                def->cputune.iothreadspin[def->cputune.niothreadspin++] =

+                    iothreadpin;

+            }

+        } else {

+            VIR_WARN("Ignore iothreadpin for <vcpu> placement is 'auto'");

+        }

+    }

+    VIR_FREE(nodes);

+

+

     /* analysis of cpu handling */

     if ((node = virXPathNode("./cpu[1]", ctxt)) != NULL) {

         xmlNodePtr oldnode = ctxt->node;

@@ -17917,6 +18000,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,

     int n;

     size_t i;

     bool blkio = false;

+    bool cputune = false;

     virCheckFlags(DUMPXML_FLAGS |

                   VIR_DOMAIN_XML_INTERNAL_STATUS |

@@ -18108,8 +18192,11 @@ virDomainDefFormatInternal(virDomainDefPtr def,

         (def->cputune.nvcpupin && !virDomainIsAllVcpupinInherited(def)) ||

         def->cputune.period || def->cputune.quota ||

         def->cputune.emulatorpin ||

-        def->cputune.emulator_period || def->cputune.emulator_quota)

+        def->cputune.emulator_period || def->cputune.emulator_quota ||

+        def->cputune.niothreadspin) {

         virBufferAddLit(buf, "<cputune>\n");

+        cputune = true;

+    }

     virBufferAdjustIndent(buf, 2);

     if (def->cputune.sharesSpecified)

@@ -18160,12 +18247,27 @@ virDomainDefFormatInternal(virDomainDefPtr def,

         virBufferAsprintf(buf, "cpuset='%s'/>\n", cpumask);

         VIR_FREE(cpumask);

     }

+

+    for (i = 0; i < def->cputune.niothreadspin; i++) {

+        char *cpumask;

+        /* Ignore the iothreadpin which inherit from "cpuset of "<vcpu>." */

+        if (def->cpumask &&

+            virBitmapEqual(def->cpumask,

+                           def->cputune.iothreadspin[i]->cpumask))

+            continue;

+

+        virBufferAsprintf(buf, "

+                          def->cputune.iothreadspin[i]->vcpuid);

+

+        if (!(cpumask = virBitmapFormat(def->cputune.iothreadspin[i]->cpumask)))

+            goto error;

+

+        virBufferAsprintf(buf, "cpuset='%s'/>\n", cpumask);

+        VIR_FREE(cpumask);

+    }

+

     virBufferAdjustIndent(buf, -2);

-    if (def->cputune.sharesSpecified ||

-        (def->cputune.nvcpupin && !virDomainIsAllVcpupinInherited(def)) ||

-        def->cputune.period || def->cputune.quota ||

-        def->cputune.emulatorpin ||

-        def->cputune.emulator_period || def->cputune.emulator_quota)

+    if (cputune)

         virBufferAddLit(buf, "</cputune>\n");

     if (virDomainNumatuneFormatXML(buf, def->numatune) < 0)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h

index c93b73c..decd4be 100644

--- a/src/conf/domain_conf.h

+++ b/src/conf/domain_conf.h

@@ -1953,6 +1953,8 @@ struct _virDomainDef {

         size_t nvcpupin;

         virDomainVcpuPinDefPtr *vcpupin;

         virDomainVcpuPinDefPtr emulatorpin;

+        size_t niothreadspin;

+        virDomainVcpuPinDefPtr *iothreadspin;

     } cputune;

     virDomainNumatunePtr numatune;

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreads.xml b/tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreads.xml

new file mode 100644

index 0000000..435d0ae

--- /dev/null

+++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreads.xml

@@ -0,0 +1,38 @@

+<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>

+  <cputune>

+    <shares>2048</shares>

+    <period>1000000</period>

+    <quota>-1</quota>

+    <vcpupin vcpu='0' cpuset='0'/>

+    <vcpupin vcpu='1' cpuset='1'/>

+    <emulatorpin cpuset='1'/>

+    <iothreadpin iothread='1' cpuset='2'/>

+    <iothreadpin iothread='2' cpuset='3'/>

+  </cputune>

+  <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'>

+      <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='virtio'/>

+  </devices>

+</domain>

diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c

index 1e06f38..843e66d 100644

--- a/tests/qemuxml2xmltest.c

+++ b/tests/qemuxml2xmltest.c

@@ -303,6 +303,7 @@ mymain(void)

     DO_TEST("smp");

     DO_TEST("iothreads");

+    DO_TEST("cputune-iothreads");

     DO_TEST("iothreads-disk");

     DO_TEST("lease");

     DO_TEST("event_idx");

-- 

2.1.0