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

Remove the iothreadspin array from cputune and replace with a cpumask to be stored in the iothreadids list Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- docs/formatdomain.html.in | 10 ++-- src/conf/domain_conf.c | 118 +++++++++++++++++++++------------------------- src/conf/domain_conf.h | 2 +- src/qemu/qemu_cgroup.c | 13 ++--- src/qemu/qemu_driver.c | 79 +++++++++---------------------- src/qemu/qemu_process.c | 7 +-- 6 files changed, 86 insertions(+), 143 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 518f7c5..7af6bd7 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -624,11 +624,11 @@ and attribute <code>cpuset</code> of element <code>vcpu</code> is not specified, the IOThreads are pinned to all the physical CPUs by default. There are two required attributes, the attribute - <code>iothread</code> specifies the IOThread id and the attribute - <code>cpuset</code> specifying which physical CPUs to pin to. The - <code>iothread</code> value begins at "1" through the number of - <a href="#elementsIOThreadsAllocation"><code>iothreads</code></a> - allocated to the domain. A value of "0" is not permitted. + <code>iothread</code> specifies the IOThread ID and the attribute + <code>cpuset</code> specifying which physical CPUs to pin to. See + the <code>iothreadids</code> + <a href="#elementsIOThreadsAllocation"><code>description</code></a> + for valid <code>iothread</code> values.

<span class="since">Since 1.2.9</span> </dd> <dt><code>shares</code></dt> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bd25d52..969e56f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2118,8 +2118,10 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin) void virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def) { - if (def) + if (def) { + virBitmapFree(def->cpumask); VIR_FREE(def);

+ } } @@ -2341,9 +2343,6 @@ void virDomainDefFree(virDomainDefPtr def) virDomainPinDefFree(def->cputune.emulatorpin); - virDomainPinDefArrayFree(def->cputune.iothreadspin, - def->cputune.niothreadspin); - for (i = 0; i < def->cputune.nvcpusched; i++) virBitmapFree(def->cputune.vcpusched[i].ids); VIR_FREE(def->cputune.vcpusched); @@ -13316,74 +13315,77 @@ virDomainVcpuPinDefParseXML(xmlNodePtr node, * and an iothreadspin has the form * <iothreadpin iothread='1' cpuset='2'/> */ -static virDomainPinDefPtr +static int virDomainIOThreadPinDefParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, - int iothreads) + virDomainDefPtr def) { - virDomainPinDefPtr def; + int ret = -1; + virDomainIOThreadIDDefPtr iothrid; + virBitmapPtr cpumask; xmlNodePtr oldnode = ctxt->node; unsigned int iothreadid; char *tmp = NULL; - if (VIR_ALLOC(def) < 0) - return NULL; - ctxt->node = node; if (!(tmp = virXPathString("string(./@iothread)", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing iothread id in iothreadpin")); - goto error; + goto cleanup; } if (virStrToLong_uip(tmp, NULL, 10, &iothreadid) < 0) { virReportError(VIR_ERR_XML_ERROR, _("invalid setting for iothread '%s'"), tmp); - goto error; + goto cleanup; } VIR_FREE(tmp); if (iothreadid == 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("zero is an invalid iothread id value")); - goto error; - } - - /* IOThreads are numbered "iothread1...iothread<n>", where - * "n" is the iothreads value */ - if (iothreadid > iothreads) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("iothread id must not exceed iothreads"));

- goto error; + goto cleanup; } - def->id = iothreadid; - if (!(tmp = virXMLPropString(node, "cpuset"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing cpuset for iothreadpin")); - goto error; + goto cleanup; } - if (virBitmapParse(tmp, 0, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) - goto error; + if (virBitmapParse(tmp, 0, &cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) + goto cleanup; - if (virBitmapIsAllClear(def->cpumask)) { + if (virBitmapIsAllClear(cpumask)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Invalid value of 'cpuset': %s"), tmp); - goto error; + goto cleanup; + } + + if (!(iothrid = virDomainIOThreadIDFind(def, iothreadid))) { + if (!(iothrid = virDomainIOThreadIDAdd(def, iothreadid)))

+ goto cleanup; + iothrid->autofill = true; + } + + if (iothrid->cpumask) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("duplicate iothreadpin for same iothread '%u'"), + iothreadid); + goto cleanup; } + iothrid->cpumask = cpumask; + cpumask = NULL; + ret = 0; + cleanup: VIR_FREE(tmp); + virBitmapFree(cpumask); ctxt->node = oldnode; - return def; - - error: - VIR_FREE(def); - goto cleanup; + return ret; } @@ -14250,28 +14252,11 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; } - if (n && VIR_ALLOC_N(def->cputune.iothreadspin, n) < 0) - goto error; - for (i = 0; i < n; i++) { - virDomainPinDefPtr iothreadpin = NULL; - iothreadpin = virDomainIOThreadPinDefParseXML(nodes[i], ctxt, - def->iothreads); - if (!iothreadpin) + if (virDomainIOThreadPinDefParseXML(nodes[i], ctxt, def) < 0) goto error; - - if (virDomainPinIsDuplicate(def->cputune.iothreadspin, - def->cputune.niothreadspin, - iothreadpin->id)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("duplicate iothreadpin for same iothread")); - virDomainPinDefFree(iothreadpin); - goto error; - } - - def->cputune.iothreadspin[def->cputune.niothreadspin++] = - iothreadpin; } + def->cputune.niothreadspin = n;

VIR_FREE(nodes); if ((n = virXPathNodeSet("./cputune/vcpusched", ctxt, &nodes)) < 0) { @@ -14384,7 +14369,7 @@ virDomainDefParseXML(xmlDocPtr xml, if (virDomainNumatuneHasPlacementAuto(def->numa) && !def->cpumask && !def->cputune.vcpupin && - !def->cputune.emulatorpin && !def->cputune.iothreadspin) + !def->cputune.emulatorpin && !def->cputune.niothreadspin)

def->placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO; if ((n = virXPathNodeSet("./resource", ctxt, &nodes)) < 0) { @@ -20837,20 +20822,25 @@ virDomainDefFormatInternal(virDomainDefPtr def, VIR_FREE(cpumask); } - for (i = 0; i < def->cputune.niothreadspin; i++) { - char *cpumask; - /* Ignore the iothreadpin which inherit from "cpuset of "<vcpu>." */ - if (virBitmapEqual(def->cpumask, def->cputune.iothreadspin[i]->cpumask)) - continue; + if (def->cputune.niothreadspin) {

+ for (i = 0; i < def->niothreadids; i++) { + char *cpumask; - virBufferAsprintf(buf, "<iothreadpin iothread='%u' ", - def->cputune.iothreadspin[i]->id); + /* Ignore iothreadids with no cpumask or a cpumask that + * inherits from "cpuset of "<vcpu>." */ + if (!def->iothreadids[i]->cpumask || + virBitmapEqual(def->cpumask, def->iothreadids[i]->cpumask)) + continue;

- if (!(cpumask = virBitmapFormat(def->cputune.iothreadspin[i]->cpumask))) - goto error; + virBufferAsprintf(buf, "<iothreadpin iothread='%u' ", + def->iothreadids[i]->iothread_id); - virBufferAsprintf(buf, "cpuset='%s'/>\n", cpumask); - VIR_FREE(cpumask); + if (!(cpumask = virBitmapFormat(def->iothreadids[i]->cpumask))) + goto error; + + virBufferAsprintf(buf, "cpuset='%s'/>\n", cpumask); + VIR_FREE(cpumask); + } } for (i = 0; i < def->cputune.nvcpusched; i++) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cc98f3d..c71e1b8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2057,6 +2057,7 @@ struct _virDomainIOThreadIDDef { bool autofill; unsigned int iothread_id; int thread_id; + virBitmapPtr cpumask; }; void virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def); @@ -2075,7 +2076,6 @@ struct _virDomainCputune { virDomainPinDefPtr *vcpupin; virDomainPinDefPtr emulatorpin; size_t niothreadspin;

- virDomainPinDefPtr *iothreadspin; size_t nvcpusched; virDomainThreadSchedParamPtr vcpusched; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index cdf0aaf..5282449 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1149,7 +1149,7 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm) virCgroupPtr cgroup_iothread = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDefPtr def = vm->def; - size_t i, j; + size_t i; unsigned long long period = vm->def->cputune.period; long long quota = vm->def->cputune.quota; char *mem_mask = NULL; @@ -1214,18 +1214,11 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm) /* default cpu masks */ if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) cpumask = priv->autoCpuset; + else if (def->iothreadids[i]->cpumask) + cpumask = def->iothreadids[i]->cpumask;

else cpumask = def->cpumask; - /* specific cpu mask */ - for (j = 0; j < def->cputune.niothreadspin; j++) { - if (def->cputune.iothreadspin[j]->id == - def->iothreadids[i]->iothread_id) { - cpumask = def->cputune.iothreadspin[j]->cpumask; - break; - }

- } - if (cpumask && qemuSetupCgroupCpusetCpus(cgroup_iothread, cpumask) < 0) goto cleanup; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0f95cc7..ee13d08 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5894,19 +5894,14 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef, goto cleanup; for (i = 0; i < targetDef->niothreadids; i++) { - virDomainPinDefPtr pininfo; - if (VIR_ALLOC(info_ret[i]) < 0) goto cleanup; /* IOThread ID's are taken from the iothreadids list */ info_ret[i]->iothread_id = targetDef->iothreadids[i]->iothread_id; - /* Initialize the cpumap */ - pininfo = virDomainPinFind(targetDef->cputune.iothreadspin, - targetDef->cputune.niothreadspin, - targetDef->iothreadids[i]->iothread_id); - if (!pininfo) { + cpumask = targetDef->iothreadids[i]->cpumask; + if (!cpumask) { if (targetDef->cpumask) { cpumask = targetDef->cpumask; } else { @@ -5915,8 +5910,6 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef, virBitmapSetAll(bitmap); cpumask = bitmap; } - } else { - cpumask = pininfo->cpumask; } if (virBitmapToData(cpumask, &info_ret[i]->cpumap, &info_ret[i]->cpumaplen) < 0) @@ -5999,8 +5992,6 @@ qemuDomainPinIOThread(virDomainPtr dom, virDomainDefPtr persistentDef = NULL; virBitmapPtr pcpumap = NULL; qemuDomainObjPrivatePtr priv; - virDomainPinDefPtr *newIOThreadsPin = NULL; - size_t newIOThreadsPinNum = 0; virCgroupPtr cgroup_iothread = NULL; virObjectEventPtr event = NULL; char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = ""; @@ -6050,33 +6041,21 @@ qemuDomainPinIOThread(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_LIVE) { virDomainIOThreadIDDefPtr iothrid; + virBitmapPtr cpumask; if (!(iothrid = virDomainIOThreadIDFind(vm->def, iothread_id))) { virReportError(VIR_ERR_INVALID_ARG, - _("iothread value %d not found"), iothread_id); + _("iothreadid %d not found"), iothread_id);

goto endjob; } - if (vm->def->cputune.iothreadspin) { - newIOThreadsPin = - virDomainPinDefCopy(vm->def->cputune.iothreadspin, - vm->def->cputune.niothreadspin); - if (!newIOThreadsPin) - goto endjob; - - newIOThreadsPinNum = vm->def->cputune.niothreadspin; - } else { - if (VIR_ALLOC(newIOThreadsPin) < 0) - goto endjob; - newIOThreadsPinNum = 0; - } - - if (virDomainPinAdd(&newIOThreadsPin, &newIOThreadsPinNum, - cpumap, maplen, iothread_id) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to update iothreadspin")); + if (!(cpumask = virBitmapNewData(cpumap, maplen))) goto endjob; - } + + if (!iothrid->cpumask) + vm->def->cputune.niothreadspin++;

+ virBitmapFree(iothrid->cpumask); + iothrid->cpumask = cpumask; /* Configure the corresponding cpuset cgroup before set affinity. */ if (virCgroupHasController(priv->cgroup, @@ -6099,14 +6078,6 @@ qemuDomainPinIOThread(virDomainPtr dom, } } - if (vm->def->cputune.iothreadspin) - virDomainPinDefArrayFree(vm->def->cputune.iothreadspin, - vm->def->cputune.niothreadspin); - - vm->def->cputune.iothreadspin = newIOThreadsPin; - vm->def->cputune.niothreadspin = newIOThreadsPinNum; - newIOThreadsPin = NULL; - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto endjob; @@ -6124,31 +6095,25 @@ qemuDomainPinIOThread(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + virDomainIOThreadIDDefPtr iothrid; + virBitmapPtr cpumask; + /* Coverity didn't realize that targetDef must be set if we got here. */ sa_assert(persistentDef); - if (iothread_id > persistentDef->iothreads) { + if (!(iothrid = virDomainIOThreadIDFind(persistentDef, iothread_id))) { virReportError(VIR_ERR_INVALID_ARG, - _("iothread value out of range %d > %d"), - iothread_id, persistentDef->iothreads); + _("iothreadid %d not found"), iothread_id); goto endjob; } - if (!persistentDef->cputune.iothreadspin) { - if (VIR_ALLOC(persistentDef->cputune.iothreadspin) < 0) - goto endjob; - persistentDef->cputune.niothreadspin = 0; - } - if (virDomainPinAdd(&persistentDef->cputune.iothreadspin, - &persistentDef->cputune.niothreadspin, - cpumap, - maplen, - iothread_id) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to update or add iothreadspin xml " - "of a persistent domain")); + if (!(cpumask = virBitmapNewData(cpumap, maplen))) goto endjob; - } + + if (!iothrid->cpumask) + persistentDef->cputune.niothreadspin++;

+ virBitmapFree(iothrid->cpumask); + iothrid->cpumask = cpumask; ret = virDomainSaveConfig(cfg->configDir, persistentDef); goto endjob;