Adding a new XML element 'iothreadids' in order to allow defining specific IOThread ID's rather than relying on the algorithm to assign IOThread ID's starting at 1 and incrementing to iothreads count. This will allow future patches to be able to add new IOThreads by a specific iothread_id and of course delete any exisiting IOThread. Each iothreadids element will have 'n' <iothread> children elements which will have attribute "id". The "id" will allow for definition of any "valid" (eg > 0) iothread_id value. On input, if any <iothreadids> <iothread>'s are provided, they will be marked so that we only print out what we read in. On input, if no <iothreadids> are provided, the PostParse code will self generate a list of ID's starting at 1 and going to the number of iothreads defined for the domain (just like the current algorithm numbering scheme). A future patch will rework the existing algorithm to make use of the iothreadids list. On output, only print out the <iothreadids> if they were read in. Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 30 +++++++ docs/schemas/domaincommon.rng | 12 +++ src/conf/domain_conf.c | 203 +++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 18 ++++ src/libvirt_private.syms | 4 + 5 files changed, 265 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e921749..518f7c5 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -521,6 +521,18 @@ ... </domain> </pre> +<pre> +<domain> + ... + <iothreadids> + <iothread id="2"/> + <iothread id="4"/> + <iothread id="6"/> + <iothread id="8"/> + </iothreadids> + ... +</domain> +</pre> <dl> <dt><code>iothreads</code></dt> @@ -530,7 +542,25 @@ virtio-blk-pci and virtio-blk-ccw target storage devices. There should be only 1 or 2 IOThreads per host CPU. There may be more than one supported device assigned to each IOThread. + <span class="since">Since 1.2.8</span> </dd> + <dt><code>iothreadids</code></dt> + <dd> + The optional <code>iothreadids</code> element provides the capability + to specifically define the IOThread ID's for the domain. By default, + IOThread ID's are sequentially numbered starting from 1 through the + number of <code>iothreads</code> defined for the domain. The + <code>id</code> attribute is used to define the IOThread ID. The + <code>id</code> attribute must be a positive integer greater than 0. + If there are less <code>iothreadids</code> defined than + <code>iothreads</code> defined for the domain, then libvirt will + sequentially fill <code>iothreadids</code> starting at 1 avoiding + any predefined <code>id</code>. If there are more + <code>iothreadids</code> defined than <code>iothreads</code> + defined for the domain, then the <code>iothreads</code> value + will be adjusted accordingly. + <span class="since">Since 1.2.15</span> + </dd> </dl> <h3><a name="elementsCPUTuning">CPU Tuning</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 19461f5..7072954 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -539,6 +539,18 @@ </optional> <optional> + <element name="iothreadids"> + <zeroOrMore> + <element name="iothread"> + <attribute name="id"> + <ref name="unsignedInt"/> + </attribute> + </element> + </zeroOrMore> + </element> + </optional> + + <optional> <ref name="blkiotune"/> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fb7594f..b84a447 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2102,6 +2102,32 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin) return NULL; } + +void +virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def) +{ + if (!def) + return; + VIR_FREE(def); +} + + +static void +virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def, + int nids) +{ + size_t i; + + if (!def) + return; + + for (i = 0; i < nids; i++) + virDomainIOThreadIDDefFree(def[i]); + + VIR_FREE(def); +} + + void virDomainPinDefFree(virDomainPinDefPtr def) { @@ -2298,6 +2324,8 @@ void virDomainDefFree(virDomainDefPtr def) virCPUDefFree(def->cpu); + virDomainIOThreadIDDefArrayFree(def->iothreadids, def->niothreadids); + virDomainPinDefArrayFree(def->cputune.vcpupin, def->cputune.nvcpupin); virDomainPinDefFree(def->cputune.emulatorpin); @@ -13169,6 +13197,54 @@ virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt, return idmap; } +/* Parse the XML definition for an IOThread ID + * + * Format is : + * + * <iothreads>4</iothreads> + * <iothreadids> + * <iothread id='1'/> + * <iothread id='3'/> + * <iothread id='5'/> + * <iothread id='7'/> + * </iothreadids> + */ +static virDomainIOThreadIDDefPtr +virDomainIOThreadIDDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt) +{ + virDomainIOThreadIDDefPtr iothrid; + xmlNodePtr oldnode = ctxt->node; + char *tmp = NULL; + + if (VIR_ALLOC(iothrid) < 0) + return NULL; + + ctxt->node = node; + + if (!(tmp = virXPathString("string(./@id)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing 'id' attribute in <iothread> element")); + goto error; + } + if (virStrToLong_uip(tmp, NULL, 10, &iothrid->iothread_id) < 0 || + iothrid->iothread_id == 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid iothread 'id' value '%s'"), tmp); + goto error; + } + + cleanup: + VIR_FREE(tmp); + ctxt->node = oldnode; + return iothrid; + + error: + virDomainIOThreadIDDefFree(iothrid); + goto cleanup; +} + + /* Parse the XML definition for a vcpupin * * vcpupin has the form of @@ -13975,6 +14051,49 @@ virDomainDefParseXML(xmlDocPtr xml, } VIR_FREE(tmp); + /* Extract any iothread id's defined */ + if ((n = virXPathNodeSet("./iothreadids/iothread", ctxt, &nodes)) < 0) + goto error; + + if (n > def->iothreads) + def->iothreads = n; + + if (n && VIR_ALLOC_N(def->iothreadids, n) < 0) + goto error; + + for (i = 0; i < n; i++) { + virDomainIOThreadIDDefPtr iothrid = NULL; + if (!(iothrid = virDomainIOThreadIDDefParseXML(nodes[i], ctxt))) + goto error; + + if (virDomainIOThreadIDFind(def, iothrid->iothread_id)) { + virReportError(VIR_ERR_XML_ERROR, + _("duplicate iothread id '%u' found"), + iothrid->iothread_id); + virDomainIOThreadIDDefFree(iothrid); + goto error; + } + def->iothreadids[def->niothreadids++] = iothrid; + } + VIR_FREE(nodes); + + /* If no iothreadid's or not fully populated, let's finish the job + * here rather than in PostParseCallback + */ + if (def->iothreads && def->iothreads != def->niothreadids) { + unsigned int iothread_id = 1; + while (def->niothreadids != def->iothreads) { + if (!virDomainIOThreadIDFind(def, iothread_id)) { + virDomainIOThreadIDDefPtr iothrid; + + if (!(iothrid = virDomainIOThreadIDAdd(def, iothread_id))) + goto error; + iothrid->autofill = true; + } + iothread_id++; + } + } + /* Extract cpu tunables. */ if ((n = virXPathULong("string(./cputune/shares[1])", ctxt, &def->cputune.shares)) < -1) { @@ -17259,6 +17378,67 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def) return 0; } +virDomainIOThreadIDDefPtr +virDomainIOThreadIDFind(virDomainDefPtr def, + unsigned int iothread_id) +{ + size_t i; + + if (!def->iothreadids || !def->niothreadids) + return NULL; + + for (i = 0; i < def->niothreadids; i++) { + if (iothread_id == def->iothreadids[i]->iothread_id) + return def->iothreadids[i]; + } + + return NULL; +} + +virDomainIOThreadIDDefPtr +virDomainIOThreadIDAdd(virDomainDefPtr def, + unsigned int iothread_id) +{ + virDomainIOThreadIDDefPtr iothrid = NULL; + + if (virDomainIOThreadIDFind(def, iothread_id)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot duplicate iothread_id '%u' in iothreadids"), + iothread_id); + return NULL; + } + + if (VIR_ALLOC(iothrid) < 0) + goto error; + + iothrid->iothread_id = iothread_id; + + if (VIR_APPEND_ELEMENT_COPY(def->iothreadids, def->niothreadids, + iothrid) < 0) + goto error; + + return iothrid; + + error: + virDomainIOThreadIDDefFree(iothrid); + return NULL; +} + +void +virDomainIOThreadIDDel(virDomainDefPtr def, + unsigned int iothread_id) +{ + int n; + + for (n = 0; n < def->niothreadids; n++) { + if (def->iothreadids[n]->iothread_id == iothread_id) { + virDomainIOThreadIDDefFree(def->iothreadids[n]); + VIR_DELETE_ELEMENT(def->iothreadids, n, def->niothreadids); + return; + } + } +} + /* Check if vcpupin with same id already exists. */ bool virDomainPinIsDuplicate(virDomainPinDefPtr *def, @@ -20601,8 +20781,27 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAsprintf(buf, " current='%u'", def->vcpus); virBufferAsprintf(buf, ">%u</vcpu>\n", def->maxvcpus); - if (def->iothreads > 0) - virBufferAsprintf(buf, "<iothreads>%u</iothreads>\n", def->iothreads); + if (def->iothreads > 0) { + virBufferAsprintf(buf, "<iothreads>%u</iothreads>\n", + def->iothreads); + /* Only print out iothreadids if we read at least one */ + for (i = 0; i < def->niothreadids; i++) { + if (!def->iothreadids[i]->autofill) + break; + } + if (i < def->niothreadids) { + virBufferAddLit(buf, "<iothreadids>\n"); + virBufferAdjustIndent(buf, 2); + for (i = 0; i < def->niothreadids; i++) { + if (def->iothreadids[i]->autofill) + continue; + virBufferAsprintf(buf, "<iothread id='%u'/>\n", + def->iothreadids[i]->iothread_id); + } + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</iothreadids>\n"); + } + } if (def->cputune.sharesSpecified || (def->cputune.nvcpupin && !virDomainIsAllVcpupinInherited(def)) || diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9955052..28a003e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2054,6 +2054,16 @@ struct _virDomainHugePage { unsigned long long size; /* hugepage size in KiB */ }; +typedef struct _virDomainIOThreadIDDef virDomainIOThreadIDDef; +typedef virDomainIOThreadIDDef *virDomainIOThreadIDDefPtr; + +struct _virDomainIOThreadIDDef { + bool autofill; + unsigned int iothread_id; +}; + +void virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def); + typedef struct _virDomainCputune virDomainCputune; typedef virDomainCputune *virDomainCputunePtr; @@ -2145,6 +2155,8 @@ struct _virDomainDef { virBitmapPtr cpumask; unsigned int iothreads; + size_t niothreadids; + virDomainIOThreadIDDefPtr *iothreadids; virDomainCputune cputune; @@ -2604,6 +2616,12 @@ bool virDomainDefCheckABIStability(virDomainDefPtr src, int virDomainDefAddImplicitControllers(virDomainDefPtr def); +virDomainIOThreadIDDefPtr virDomainIOThreadIDFind(virDomainDefPtr def, + unsigned int iothread_id); +virDomainIOThreadIDDefPtr virDomainIOThreadIDAdd(virDomainDefPtr def, + unsigned int iothread_id); +void virDomainIOThreadIDDel(virDomainDefPtr def, unsigned int iothread_id); + unsigned int virDomainDefFormatConvertXMLFlags(unsigned int flags); char *virDomainDefFormat(virDomainDefPtr def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e6555f1..1c345fc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -324,6 +324,10 @@ virDomainHubTypeToString; virDomainHypervTypeFromString; virDomainHypervTypeToString; virDomainInputDefFree; +virDomainIOThreadIDAdd; +virDomainIOThreadIDDefFree; +virDomainIOThreadIDDel; +virDomainIOThreadIDFind; virDomainLeaseDefFree; virDomainLeaseIndex; virDomainLeaseInsert; -- 2.1.0

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). Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.h | 1 + src/qemu/qemu_cgroup.c | 22 ++++++------- src/qemu/qemu_command.c | 38 +++++++++++++++++----- src/qemu/qemu_command.h | 3 ++ src/qemu/qemu_domain.c | 36 -------------------- src/qemu/qemu_domain.h | 3 -- src/qemu/qemu_driver.c | 35 +++++++++----------- src/qemu/qemu_process.c | 37 ++++++++++----------- .../qemuxml2argv-iothreads-ids-partial.args | 10 ++++++ .../qemuxml2argv-iothreads-ids-partial.xml | 33 +++++++++++++++++++ .../qemuxml2argv-iothreads-ids.args | 8 +++++ .../qemuxml2argv-iothreads-ids.xml | 33 +++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ tests/qemuxml2xmltest.c | 2 ++ 14 files changed, 164 insertions(+), 99 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.xml diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 28a003e..c7966a4 100644 --- a/src/conf/domain_conf.h +++ b/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); diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index bf0621f..8e63011 100644 --- a/src/qemu/qemu_cgroup.c +++ b/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; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 29b876e..cc96d5b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -678,6 +678,24 @@ qemuOpenVhostNet(virDomainDefPtr def, } 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) { int ret = -1; @@ -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; } @@ -4209,6 +4227,7 @@ qemuBuildDriveDevStr(virDomainDefPtr def, if (disk->iothread) virBufferAsprintf(&opt, ",iothread=iothread%u", disk->iothread); } + qemuBuildIoEventFdStr(&opt, disk->ioeventfd, qemuCaps); if (disk->event_idx && virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_EVENT_IDX)) { @@ -8794,14 +8813,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); } } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index a29db41..538ccdf 100644 --- a/src/qemu/qemu_command.h +++ b/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); /* diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6c480c6..2cf6ec4 100644 --- a/src/qemu/qemu_domain.c +++ b/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")); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index d550ae3..5f86177 100644 --- a/src/qemu/qemu_domain.h +++ b/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; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 989c20c..330ffdf 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5850,14 +5850,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; @@ -5912,19 +5914,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; @@ -6068,16 +6070,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; } @@ -6115,8 +6112,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); @@ -10087,8 +10083,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; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6707170..0d15432 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2267,12 +2267,16 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver, goto cleanup; } - if (VIR_ALLOC_N(priv->iothreadpids, niothreads) < 0) - goto cleanup; - priv->niothreadpids = niothreads; + for (i = 0; i < vm->def->niothreadids; i++) { + unsigned int iothread_id; - for (i = 0; i < priv->niothreadpids; i++) - priv->iothreadpids[i] = iothreads[i]->thread_id; + if (qemuDomainParseIOThreadAlias(iothreads[i]->name, + &iothread_id) < 0) + goto cleanup; + + vm->def->iothreadids[i]->thread_id = iothreads[i]->thread_id; + vm->def->iothreadids[i]->iothread_id = iothread_id; + } ret = 0; @@ -2453,7 +2457,6 @@ qemuProcessSetEmulatorAffinity(virDomainObjPtr vm) static int qemuProcessSetIOThreadsAffinity(virDomainObjPtr vm) { - qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDefPtr def = vm->def; virDomainPinDefPtr pininfo; size_t i; @@ -2462,20 +2465,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; @@ -2525,8 +2523,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; @@ -5314,8 +5313,6 @@ void qemuProcessStop(virQEMUDriverPtr driver, virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); VIR_FREE(priv->vcpupids); priv->nvcpupids = 0; - VIR_FREE(priv->iothreadpids); - priv->niothreadpids = 0; virObjectUnref(priv->qemuCaps); priv->qemuCaps = NULL; VIR_FREE(priv->pidfile); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.args b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.args new file mode 100644 index 0000000..444cd17 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.args @@ -0,0 +1,10 @@ +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 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.xml b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.xml new file mode 100644 index 0000000..c631677 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.xml @@ -0,0 +1,33 @@ +<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> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.args b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.args new file mode 100644 index 0000000..68998f6 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.args @@ -0,0 +1,8 @@ +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 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.xml b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.xml new file mode 100644 index 0000000..d70e74b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.xml @@ -0,0 +1,33 @@ +<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> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 055ceee..0763068 100644 --- a/tests/qemuxml2argvtest.c +++ b/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, diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 45464cc..b611afd 100644 --- a/tests/qemuxml2xmltest.c +++ b/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"); -- 2.1.0

Since it's only ever referenced in domain_conf.c, make the function static, but also will need to move it to somewhere before it's referenced rather than forward referencing it. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 38 +++++++++++++++++++------------------- src/conf/domain_conf.h | 4 ---- src/libvirt_private.syms | 1 - 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b84a447..ddb0d83 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13245,6 +13245,25 @@ virDomainIOThreadIDDefParseXML(xmlNodePtr node, } +/* Check if pin with same id already exists. */ +static bool +virDomainPinIsDuplicate(virDomainPinDefPtr *def, + int npin, + int id) +{ + size_t i; + + if (!def || !npin) + return false; + + for (i = 0; i < npin; i++) { + if (def[i]->id == id) + return true; + } + + return false; +} + /* Parse the XML definition for a vcpupin * * vcpupin has the form of @@ -17439,25 +17458,6 @@ virDomainIOThreadIDDel(virDomainDefPtr def, } } -/* Check if vcpupin with same id already exists. */ -bool -virDomainPinIsDuplicate(virDomainPinDefPtr *def, - int npin, - int id) -{ - size_t i; - - if (!def || !npin) - return false; - - for (i = 0; i < npin; i++) { - if (def[i]->id == id) - return true; - } - - return false; -} - virDomainPinDefPtr virDomainPinFind(virDomainPinDefPtr *def, int npin, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c7966a4..2cc7ffd 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1947,10 +1947,6 @@ void virDomainPinDefArrayFree(virDomainPinDefPtr *def, int npin); virDomainPinDefPtr *virDomainPinDefCopy(virDomainPinDefPtr *src, int npin); -bool virDomainPinIsDuplicate(virDomainPinDefPtr *def, - int npin, - int id); - virDomainPinDefPtr virDomainPinFind(virDomainPinDefPtr *def, int npin, int id); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1c345fc..f3d2c38 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -414,7 +414,6 @@ virDomainPinDefCopy; virDomainPinDefFree; virDomainPinDel; virDomainPinFind; -virDomainPinIsDuplicate; virDomainPMSuspendedReasonTypeFromString; virDomainPMSuspendedReasonTypeToString; virDomainRedirdevBusTypeFromString; -- 2.1.0

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. Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 10 +- src/conf/domain_conf.c | 112 ++++++++++----------- src/conf/domain_conf.h | 3 +- src/qemu/qemu_cgroup.c | 16 +-- src/qemu/qemu_driver.c | 75 ++++---------- src/qemu/qemu_process.c | 10 +- .../qemuxml2xmlout-cputune-iothreads.xml | 2 +- 7 files changed, 86 insertions(+), 142 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 ddb0d83..129908d 100644 --- a/src/conf/domain_conf.c +++ b/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); @@ -13323,74 +13334,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; } @@ -14274,27 +14288,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); @@ -14408,7 +14404,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) { @@ -20808,7 +20805,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; @@ -20862,16 +20859,19 @@ 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 or a cpumask that + * inherits from "cpuset of "<vcpu>." */ + if (!def->iothreadids[i]->cpumask || + virBitmapEqual(def->cpumask, 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); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2cc7ffd..f60f7a0 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); @@ -2074,8 +2075,6 @@ struct _virDomainCputune { size_t nvcpupin; 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 8e63011..e24989b 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; @@ -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; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 330ffdf..1fac0b8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5915,19 +5915,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 { @@ -5936,8 +5931,6 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef, virBitmapSetAll(bitmap); cpumask = bitmap; } - } else { - cpumask = pininfo->cpumask; } if (virBitmapToData(cpumask, &info_ret[i]->cpumap, &info_ret[i]->cpumaplen) < 0) @@ -6020,8 +6013,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] = ""; @@ -6071,33 +6062,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) - 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; - } + + virBitmapFree(iothrid->cpumask); + iothrid->cpumask = cpumask; /* Configure the corresponding cpuset cgroup before set affinity. */ if (virCgroupHasController(priv->cgroup, @@ -6120,14 +6097,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; @@ -6145,31 +6114,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) - 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; - } + + virBitmapFree(iothrid->cpumask); + iothrid->cpumask = cpumask; ret = virDomainSaveConfig(cfg->configDir, persistentDef); goto endjob; @@ -6181,8 +6142,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 0d15432..b920b15 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2458,22 +2458,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; diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-iothreads.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-iothreads.xml index 3684483..dc65564 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-iothreads.xml +++ b/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> -- 2.1.0

https://bugzilla.redhat.com/show_bug.cgi?id=1161617 Add command to allow adding and removing IOThreads from the domain including the configuration and live domain. $ virsh iothreadadd --help NAME iothreadadd - add an IOThread to the guest domain SYNOPSIS iothreadadd <domain> <id> [--config] [--live] [--current] DESCRIPTION Add an IOThread to the guest domain. OPTIONS [--domain] <string> domain name, id or uuid [--id] <number> iothread for the new IOThread --config affect next boot --live affect running domain --current affect current domain $ virsh iothreaddel --help NAME iothreaddel - delete an IOThread from the guest domain SYNOPSIS iothreaddel <domain> <id> [--config] [--live] [--current] DESCRIPTION Delete an IOThread from the guest domain. OPTIONS [--domain] <string> domain name, id or uuid [--id] <number> iothread_id for the IOThread to delete --config affect next boot --live affect running domain --current affect current domain Assuming a running $dom with multiple IOThreads assigned and that that the $dom has disks assigned to IOThread 1 and IOThread 2: $ virsh iothreadinfo $dom IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 3 0-1 $ virsh iothreadadd $dom 1 error: invalid argument: an IOThread is already using iothread_id '1' in iothreadpids $ virsh iothreadadd $dom 1 --config error: invalid argument: an IOThread is already using iothread_id '1' in persistent iothreadids $ virsh iothreadadd $dom 4 $ virsh iothreadinfo $dom IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 3 0-1 4 0-3 $ virsh iothreadinfo $dom --config IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 3 0-1 $ virsh iothreadadd $dom 4 --config $ virsh iothreadinfo $dom --config IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 3 0-1 4 0-3 Assuming the same original configuration $ virsh iothreaddel $dom 1 error: invalid argument: cannot remove IOThread 1 since it is being used by disk path '/home/vm-images/f18' $ virsh iothreaddel $dom 3 $ virsh iothreadinfo $dom IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 $ virsh iothreadinfo $dom --config IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 3 0-1 Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh-domain.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 31 ++++++++++ 2 files changed, 195 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index a190050..513f6df 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6896,6 +6896,158 @@ cmdIOThreadPin(vshControl *ctl, const vshCmd *cmd) } /* + * "iothreadadd" command + */ +static const vshCmdInfo info_iothreadadd[] = { + {.name = "help", + .data = N_("add an IOThread to the guest domain") + }, + {.name = "desc", + .data = N_("Add an IOThread to the guest domain.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_iothreadadd[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = "id", + .type = VSH_OT_INT, + .flags = VSH_OFLAG_REQ, + .help = N_("iothread for the new IOThread") + }, + {.name = "config", + .type = VSH_OT_BOOL, + .help = N_("affect next boot") + }, + {.name = "live", + .type = VSH_OT_BOOL, + .help = N_("affect running domain") + }, + {.name = "current", + .type = VSH_OT_BOOL, + .help = N_("affect current domain") + }, + {.name = NULL} +}; + +static bool +cmdIOThreadAdd(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + int iothread_id = 0; + bool ret = false; + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + + VSH_EXCLUSIVE_OPTIONS_VAR(current, live); + VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptInt(cmd, "id", &iothread_id) < 0 || iothread_id <= 0) { + vshError(ctl, _("Invalid IOThread id value: '%d'"), iothread_id); + goto cleanup; + } + + if (virDomainAddIOThread(dom, iothread_id, flags) < 0) + goto cleanup; + + ret = true; + + cleanup: + virDomainFree(dom); + return ret; +} + +/* + * "iothreaddel" command + */ +static const vshCmdInfo info_iothreaddel[] = { + {.name = "help", + .data = N_("delete an IOThread from the guest domain") + }, + {.name = "desc", + .data = N_("Delete an IOThread from the guest domain.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_iothreaddel[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = "id", + .type = VSH_OT_INT, + .flags = VSH_OFLAG_REQ, + .help = N_("iothread_id for the IOThread to delete") + }, + {.name = "config", + .type = VSH_OT_BOOL, + .help = N_("affect next boot") + }, + {.name = "live", + .type = VSH_OT_BOOL, + .help = N_("affect running domain") + }, + {.name = "current", + .type = VSH_OT_BOOL, + .help = N_("affect current domain") + }, + {.name = NULL} +}; + +static bool +cmdIOThreadDel(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + int iothread_id = 0; + bool ret = false; + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + + VSH_EXCLUSIVE_OPTIONS_VAR(current, live); + VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptInt(cmd, "id", &iothread_id) < 0 || iothread_id <= 0) { + vshError(ctl, _("Invalid IOThread id value: '%d'"), iothread_id); + goto cleanup; + } + + if (virDomainDelIOThread(dom, iothread_id, flags) < 0) + goto cleanup; + + ret = true; + + cleanup: + virDomainFree(dom); + return ret; +} + +/* * "cpu-compare" command */ static const vshCmdInfo info_cpu_compare[] = { @@ -12845,6 +12997,18 @@ const vshCmdDef domManagementCmds[] = { .info = info_iothreadpin, .flags = 0 }, + {.name = "iothreadadd", + .handler = cmdIOThreadAdd, + .opts = opts_iothreadadd, + .info = info_iothreadadd, + .flags = 0 + }, + {.name = "iothreaddel", + .handler = cmdIOThreadDel, + .opts = opts_iothreaddel, + .info = info_iothreaddel, + .flags = 0 + }, {.name = "send-key", .handler = cmdSendKey, .opts = opts_send_key, diff --git a/tools/virsh.pod b/tools/virsh.pod index d642a69..091abc6 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1416,6 +1416,37 @@ If no flag is specified, behavior is different depending on hypervisor. B<Note>: The expression is sequentially evaluated, so "0-15,^8" is identical to "9-14,0-7,15" but not identical to "^8,0-15". +=item B<iothreadadd> I<domain> I<iothread_id> +[[I<--config>] [I<--live>] | [I<--current>]] + +Add a new IOThread to the domain using the specified I<iothread_id>. +If the I<iothread_id> already exists, the command will fail. The +I<iothread_id> must be greater than zero. + +If I<--live> is specified, affect a running guest. If the guest is not +running an error is returned. +If I<--config> is specified, affect the next boot of a persistent guest. +If I<--current> is specified or I<--live> and I<--config> are not specified, +affect the current guest state. +Both I<--live> and I<--config> flags may be given, but if not flag is +specified, behavior is different depending on hypervisor. + +=item B<iothreaddel> I<domain> I<iothread_id> +[[I<--config>] [I<--live>] | [I<--current>]] + +Delete an IOThread from the domain using the specified I<iothread_id>. +If an IOThread is currently assigned to a disk resource such as via the +B<attach-disk> command, then the attempt to remove the IOThread will fail. +If the I<iothread_id> does not exist an error will occur. + +If I<--live> is specified, affect a running guest. If the guest is not +running an error is returned. +If I<--config> is specified, affect the next boot of a persistent guest. +If I<--current> is specified or I<--live> and I<--config> are not specified, +affect the current guest state. +Both I<--live> and I<--config> flags may be given, but if not flag is +specified, behavior is different depending on hypervisor. + =item B<managedsave> I<domain> [I<--bypass-cache>] [{I<--running> | I<--paused>}] [I<--verbose>] -- 2.1.0