Pass a bitmap of enabled guest vCPUs to virCgroupGetPercpuStats so that non-continuous vCPU topologies can be used. --- Notes: v3: - don't allocate @guestvcpus when qemu doesn't report vcpu pids src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 8 +++++++- src/util/vircgroup.c | 16 ++++++++-------- src/util/vircgroup.h | 3 ++- tests/vircgrouptest.c | 2 +- 5 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index b3399d9..41639ac 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5706,7 +5706,7 @@ lxcDomainGetCPUStats(virDomainPtr dom, params, nparams); else ret = virCgroupGetPercpuStats(priv->cgroup, params, - nparams, start_cpu, ncpus, 0); + nparams, start_cpu, ncpus, NULL); cleanup: virObjectUnlock(vm); return ret; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f2db7d7..78f177a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18205,6 +18205,7 @@ qemuDomainGetCPUStats(virDomainPtr domain, virDomainObjPtr vm = NULL; int ret = -1; qemuDomainObjPrivatePtr priv; + virBitmapPtr guestvcpus = NULL; virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); @@ -18228,13 +18229,18 @@ qemuDomainGetCPUStats(virDomainPtr domain, goto cleanup; } + if (qemuDomainHasVcpuPids(vm) && + !(guestvcpus = virDomainDefGetOnlineVcpumap(vm->def))) + goto cleanup; + if (start_cpu == -1) ret = virCgroupGetDomainTotalCpuStats(priv->cgroup, params, nparams); else ret = virCgroupGetPercpuStats(priv->cgroup, params, nparams, - start_cpu, ncpus, priv->nvcpupids); + start_cpu, ncpus, guestvcpus); cleanup: + virBitmapFree(guestvcpus); virDomainObjEndAPI(&vm); return ret; } diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 2f54cf2..f625cbc 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3161,17 +3161,17 @@ virCgroupDenyDevicePath(virCgroupPtr group, const char *path, int perms) */ static int virCgroupGetPercpuVcpuSum(virCgroupPtr group, - unsigned int nvcpupids, + virBitmapPtr guestvcpus, unsigned long long *sum_cpu_time, size_t nsum, virBitmapPtr cpumap) { int ret = -1; - size_t i; + ssize_t i = -1; char *buf = NULL; virCgroupPtr group_vcpu = NULL; - for (i = 0; i < nvcpupids; i++) { + while ((i = virBitmapNextSetBit(guestvcpus, i)) >= 0) { char *pos; unsigned long long tmp; ssize_t j; @@ -3233,7 +3233,7 @@ virCgroupGetPercpuStats(virCgroupPtr group, unsigned int nparams, int start_cpu, unsigned int ncpus, - unsigned int nvcpupids) + virBitmapPtr guestvcpus) { int ret = -1; size_t i; @@ -3248,7 +3248,7 @@ virCgroupGetPercpuStats(virCgroupPtr group, /* return the number of supported params */ if (nparams == 0 && ncpus != 0) { - if (nvcpupids == 0) + if (!guestvcpus) return CGROUP_NB_PER_CPU_STAT_PARAM; else return CGROUP_NB_PER_CPU_STAT_PARAM + 1; @@ -3303,11 +3303,11 @@ virCgroupGetPercpuStats(virCgroupPtr group, /* return percpu vcputime in index 1 */ param_idx = 1; - if (nvcpupids > 0 && param_idx < nparams) { + if (guestvcpus && param_idx < nparams) { if (VIR_ALLOC_N(sum_cpu_time, need_cpus) < 0) goto cleanup; - if (virCgroupGetPercpuVcpuSum(group, nvcpupids, sum_cpu_time, need_cpus, - cpumap) < 0) + if (virCgroupGetPercpuVcpuSum(group, guestvcpus, sum_cpu_time, + need_cpus, cpumap) < 0) goto cleanup; for (i = start_cpu; i < need_cpus; i++) { diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index bec88dc..b1bf731 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -26,6 +26,7 @@ # define __VIR_CGROUP_H__ # include "virutil.h" +# include "virbitmap.h" struct virCgroup; typedef struct virCgroup *virCgroupPtr; @@ -246,7 +247,7 @@ virCgroupGetPercpuStats(virCgroupPtr group, unsigned int nparams, int start_cpu, unsigned int ncpus, - unsigned int nvcpupids); + virBitmapPtr guestvcpus); int virCgroupGetDomainTotalCpuStats(virCgroupPtr group, diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 7ea6e13..322f6cb 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -656,7 +656,7 @@ static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED) if ((rv = virCgroupGetPercpuStats(cgroup, params, - 1, 0, EXPECTED_NCPUS, 0)) < 0) { + 1, 0, EXPECTED_NCPUS, NULL)) < 0) { fprintf(stderr, "Failed call to virCgroupGetPercpuStats for /virtualmachines cgroup: %d\n", -rv); goto cleanup; } -- 2.6.2

Now that qemuDomainDetectVcpuPids is able to refresh the vCPU pid information it can be reused in the hotplug and hotunplug code paths rather than open-coding a very similar algorithm. A slight algorithm change is necessary for unplug since the vCPU needs to be marked offline prior to calling the thread detector function and eventually rolled back if something fails. --- Notes: v3: correctly handle return values and pointer sanity in the unplug case src/qemu/qemu_driver.c | 76 +++++++++++++++++--------------------------------- 1 file changed, 25 insertions(+), 51 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 78f177a..af44b0a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4770,11 +4770,10 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, int ret = -1; int rc; int oldvcpus = virDomainDefGetVcpus(vm->def); - pid_t *cpupids = NULL; - int ncpupids = 0; virCgroupPtr cgroup_vcpu = NULL; char *mem_mask = NULL; virDomainNumatuneMemMode mem_mode; + pid_t vcpupid; if (!(vcpuinfo = virDomainDefGetVcpu(vm->def, vcpu))) return -1; @@ -4789,9 +4788,6 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, rc = qemuMonitorSetCPU(priv->mon, vcpu, true); - if (rc == 0) - ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids); - if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; @@ -4802,23 +4798,15 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, vcpuinfo->online = true; - if (ncpupids < 0) - goto cleanup; + if ((rc = qemuDomainDetectVcpuPids(driver, vm, QEMU_ASYNC_JOB_NONE)) <= 0) { + /* vcpu pids were not detected, skip setting of affinity */ + if (rc == 0) + ret = 0; - /* failure to re-detect vCPU pids after hotplug due to lack of support was - * historically deemed not fatal. We need to skip the rest of the steps though. */ - if (ncpupids == 0) { - ret = 0; goto cleanup; } - if (ncpupids != oldvcpus + 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("got wrong number of vCPU pids from QEMU monitor. " - "got %d, wanted %d"), - ncpupids, oldvcpus + 1); - goto cleanup; - } + vcpupid = qemuDomainGetVcpuPid(vm, vcpu); if (virDomainNumatuneGetMode(vm->def->numa, -1, &mem_mode) == 0 && mem_mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT && @@ -4828,11 +4816,9 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, goto cleanup; if (priv->cgroup) { - cgroup_vcpu = - qemuDomainAddCgroupForThread(priv->cgroup, - VIR_CGROUP_THREAD_VCPU, - vcpu, mem_mask, - cpupids[vcpu]); + cgroup_vcpu = qemuDomainAddCgroupForThread(priv->cgroup, + VIR_CGROUP_THREAD_VCPU, + vcpu, mem_mask, vcpupid); if (!cgroup_vcpu) goto cleanup; } @@ -4844,26 +4830,20 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, &vm->def->cputune.nvcpupin) < 0) goto cleanup; - if (qemuDomainHotplugPinThread(vm->def->cpumask, vcpu, cpupids[vcpu], + if (qemuDomainHotplugPinThread(vm->def->cpumask, vcpu, vcpupid, cgroup_vcpu) < 0) { goto cleanup; } } - if (qemuProcessSetSchedParams(vcpu, cpupids[vcpu], + if (qemuProcessSetSchedParams(vcpu, vcpupid, vm->def->cputune.nvcpusched, vm->def->cputune.vcpusched) < 0) goto cleanup; - priv->nvcpupids = ncpupids; - VIR_FREE(priv->vcpupids); - priv->vcpupids = cpupids; - cpupids = NULL; - ret = 0; cleanup: - VIR_FREE(cpupids); VIR_FREE(mem_mask); virCgroupFree(&cgroup_vcpu); return ret; @@ -4880,8 +4860,6 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, int ret = -1; int rc; int oldvcpus = virDomainDefGetVcpus(vm->def); - pid_t *cpupids = NULL; - int ncpupids = 0; if (!(vcpuinfo = virDomainDefGetVcpu(vm->def, vcpu))) return -1; @@ -4892,30 +4870,32 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, return -1; } + vcpuinfo->online = false; + qemuDomainObjEnterMonitor(driver, vm); rc = qemuMonitorSetCPU(priv->mon, vcpu, false); - if (rc == 0) - ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids); - if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; - virDomainAuditVcpu(vm, oldvcpus, oldvcpus - 1, "update", - rc == 0 && ncpupids == oldvcpus -1); - - if (rc < 0 || ncpupids < 0) + if (rc < 0) { + virDomainAuditVcpu(vm, oldvcpus, oldvcpus - 1, "update", false); + vcpuinfo->online = true; goto cleanup; + } - /* check if hotunplug has failed */ - if (ncpupids != oldvcpus - 1) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("qemu didn't unplug vCPU '%u' properly"), vcpu); + if ((rc = qemuDomainDetectVcpuPids(driver, vm, QEMU_ASYNC_JOB_NONE)) < 0) { + /* rollback only if domain didn't exit */ + if (rc == -2) + goto cleanup; + + virDomainAuditVcpu(vm, oldvcpus, oldvcpus - 1, "update", false); + vcpuinfo->online = true; goto cleanup; } - vcpuinfo->online = false; + virDomainAuditVcpu(vm, oldvcpus, oldvcpus - 1, "update", true); if (qemuDomainDelCgroupForThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, vcpu) < 0) @@ -4926,15 +4906,9 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, &vm->def->cputune.nvcpupin, vcpu); - priv->nvcpupids = ncpupids; - VIR_FREE(priv->vcpupids); - priv->vcpupids = cpupids; - cpupids = NULL; - ret = 0; cleanup: - VIR_FREE(cpupids); return ret; } -- 2.6.2

Now with the new struct the data can be stored in a much saner place. --- Notes: v3: - fixed prlsdkCheckUnsupportedParams - removed unrelated style change - fixed (possibly) coverity warning from v1 v2: - clear bitmap pointer after free to avoid use after free v3: - fixed prlsdkCheckUnsupportedParams - removed unrelated style change v2: - clear bitmap pointer after free to avoid use after free v2: - clear bitmap pointer after free to avoid use after free src/conf/domain_conf.c | 136 +++++++++++++++++++--------------------------- src/conf/domain_conf.h | 3 +- src/libxl/libxl_domain.c | 17 +++--- src/libxl/libxl_driver.c | 36 ++++++------- src/qemu/qemu_cgroup.c | 15 ++---- src/qemu/qemu_driver.c | 137 ++++++++++++++++++++++------------------------- src/qemu/qemu_process.c | 36 ++++++------- src/test/test_driver.c | 43 ++++++--------- src/vz/vz_sdk.c | 16 +++--- 9 files changed, 190 insertions(+), 249 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6914466..6319856 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1289,6 +1289,9 @@ virDomainVcpuInfoClear(virDomainVcpuInfoPtr info) { if (!info) return; + + virBitmapFree(info->cpumask); + info->cpumask = NULL; } @@ -1422,7 +1425,14 @@ virDomainDefGetVcpu(virDomainDefPtr def, static bool virDomainDefHasVcpuPin(const virDomainDef *def) { - return !!def->cputune.vcpupin; + size_t i; + + for (i = 0; i < def->maxvcpus; i++) { + if (def->vcpus[i].cpumask) + return true; + } + + return false; } @@ -2593,8 +2603,6 @@ void virDomainDefFree(virDomainDefPtr def) virDomainIOThreadIDDefArrayFree(def->iothreadids, def->niothreadids); - virDomainPinDefArrayFree(def->cputune.vcpupin, def->cputune.nvcpupin); - virBitmapFree(def->cputune.emulatorpin); for (i = 0; i < def->cputune.nvcpusched; i++) @@ -14135,83 +14143,68 @@ 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 * <vcpupin vcpu='0' cpuset='0'/> */ -static virDomainPinDefPtr -virDomainVcpuPinDefParseXML(xmlNodePtr node, - xmlXPathContextPtr ctxt) +static int +virDomainVcpuPinDefParseXML(virDomainDefPtr def, + xmlNodePtr node) { - virDomainPinDefPtr def; - xmlNodePtr oldnode = ctxt->node; + virDomainVcpuInfoPtr vcpu; unsigned int vcpuid; char *tmp = NULL; + int ret = -1; - if (VIR_ALLOC(def) < 0) - return NULL; - - ctxt->node = node; - - if (!(tmp = virXPathString("string(./@vcpu)", ctxt))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing vcpu id in vcpupin")); - goto error; + if (!(tmp = virXMLPropString(node, "vcpu"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", _("missing vcpu id in vcpupin")); + goto cleanup; } if (virStrToLong_uip(tmp, NULL, 10, &vcpuid) < 0) { virReportError(VIR_ERR_XML_ERROR, _("invalid setting for vcpu '%s'"), tmp); - goto error; + goto cleanup; } VIR_FREE(tmp); - def->id = vcpuid; + if (!(vcpu = virDomainDefGetVcpu(def, vcpuid)) || + !vcpu->online) { + /* To avoid the regression when daemon loading domain confs, we can't + * simply error out if <vcpupin> nodes greater than current vcpus. + * Ignore them instead. */ + VIR_WARN("Ignoring vcpupin for missing vcpus"); + ret = 0; + goto cleanup; + } if (!(tmp = virXMLPropString(node, "cpuset"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing cpuset for vcpupin")); + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing cpuset for vcpupin")); + goto cleanup; + } - goto error; + if (vcpu->cpumask) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("duplicate vcpupin for vcpu '%d'"), vcpuid); + goto cleanup; } - if (virBitmapParse(tmp, 0, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) - goto error; + if (virBitmapParse(tmp, 0, &vcpu->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) + goto cleanup; - if (virBitmapIsAllClear(def->cpumask)) { + if (virBitmapIsAllClear(vcpu->cpumask)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Invalid value of 'cpuset': %s"), tmp); - goto error; + goto cleanup; } + ret = 0; + cleanup: VIR_FREE(tmp); - ctxt->node = oldnode; - return def; - - error: - VIR_FREE(def); - goto cleanup; + return ret; } @@ -15167,34 +15160,9 @@ virDomainDefParseXML(xmlDocPtr xml, if ((n = virXPathNodeSet("./cputune/vcpupin", ctxt, &nodes)) < 0) goto error; - if (n && VIR_ALLOC_N(def->cputune.vcpupin, n) < 0) - goto error; - for (i = 0; i < n; i++) { - virDomainPinDefPtr vcpupin; - if (!(vcpupin = virDomainVcpuPinDefParseXML(nodes[i], ctxt))) + if (virDomainVcpuPinDefParseXML(def, nodes[i])) goto error; - - if (virDomainPinIsDuplicate(def->cputune.vcpupin, - def->cputune.nvcpupin, - vcpupin->id)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("duplicate vcpupin for same vcpu")); - virDomainPinDefFree(vcpupin); - goto error; - } - - if (vcpupin->id >= virDomainDefGetVcpus(def)) { - /* To avoid the regression when daemon loading - * domain confs, we can't simply error out if - * <vcpupin> nodes greater than current vcpus, - * ignoring them instead. - */ - VIR_WARN("Ignore vcpupin for missing vcpus"); - virDomainPinDefFree(vcpupin); - } else { - def->cputune.vcpupin[def->cputune.nvcpupin++] = vcpupin; - } } VIR_FREE(nodes); @@ -21870,15 +21838,19 @@ virDomainDefFormatInternal(virDomainDefPtr def, "</emulator_quota>\n", def->cputune.emulator_quota); - for (i = 0; i < def->cputune.nvcpupin; i++) { + for (i = 0; i < def->maxvcpus; i++) { char *cpumask; - virBufferAsprintf(&childrenBuf, "<vcpupin vcpu='%u' ", - def->cputune.vcpupin[i]->id); + virDomainVcpuInfoPtr vcpu = def->vcpus + i; - if (!(cpumask = virBitmapFormat(def->cputune.vcpupin[i]->cpumask))) + if (!vcpu->cpumask) + continue; + + if (!(cpumask = virBitmapFormat(vcpu->cpumask))) goto error; - virBufferAsprintf(&childrenBuf, "cpuset='%s'/>\n", cpumask); + virBufferAsprintf(&childrenBuf, + "<vcpupin vcpu='%zu' cpuset='%s'/>\n", i, cpumask); + VIR_FREE(cpumask); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 83a080b..f735282 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2137,8 +2137,6 @@ struct _virDomainCputune { long long quota; unsigned long long emulator_period; long long emulator_quota; - size_t nvcpupin; - virDomainPinDefPtr *vcpupin; virBitmapPtr emulatorpin; size_t nvcpusched; @@ -2153,6 +2151,7 @@ typedef virDomainVcpuInfo *virDomainVcpuInfoPtr; struct _virDomainVcpuInfo { bool online; + virBitmapPtr cpumask; }; typedef struct _virDomainBlkiotune virDomainBlkiotune; diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 67b230a..632acfd 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -821,7 +821,7 @@ int libxlDomainSetVcpuAffinities(libxlDriverPrivatePtr driver, virDomainObjPtr vm) { libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); - virDomainPinDefPtr pin; + virDomainVcpuInfoPtr vcpu; libxl_bitmap map; virBitmapPtr cpumask = NULL; size_t i; @@ -830,13 +830,12 @@ libxlDomainSetVcpuAffinities(libxlDriverPrivatePtr driver, virDomainObjPtr vm) libxl_bitmap_init(&map); for (i = 0; i < virDomainDefGetVcpus(vm->def); ++i) { - pin = virDomainPinFind(vm->def->cputune.vcpupin, - vm->def->cputune.nvcpupin, - i); + vcpu = virDomainDefGetVcpu(vm->def, i); - if (pin && pin->cpumask) - cpumask = pin->cpumask; - else + if (!vcpu->online) + continue; + + if (!(cpumask = vcpu->cpumask)) cpumask = vm->def->cpumask; if (!cpumask) @@ -845,9 +844,9 @@ libxlDomainSetVcpuAffinities(libxlDriverPrivatePtr driver, virDomainObjPtr vm) if (virBitmapToData(cpumask, &map.map, (int *)&map.size) < 0) goto cleanup; - if (libxl_set_vcpuaffinity(cfg->ctx, vm->def->id, pin->id, &map) != 0) { + if (libxl_set_vcpuaffinity(cfg->ctx, vm->def->id, i, &map) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to pin vcpu '%d' with libxenlight"), pin->id); + _("Failed to pin vcpu '%zu' with libxenlight"), i); goto cleanup; } diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 8965abd..2b88596 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2329,6 +2329,7 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu, libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainDefPtr targetDef = NULL; virBitmapPtr pcpumap = NULL; + virDomainVcpuInfoPtr vcpuinfo; virDomainObjPtr vm; int ret = -1; @@ -2364,6 +2365,13 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu, if (!pcpumap) goto endjob; + if (!(vcpuinfo = virDomainDefGetVcpu(targetDef, vcpu)) || + !vcpuinfo->online) { + virReportError(VIR_ERR_INVALID_ARG, + _("vcpu '%u' is not active"), vcpu); + goto endjob; + } + if (flags & VIR_DOMAIN_AFFECT_LIVE) { libxl_bitmap map = { .size = maplen, .map = cpumap }; if (libxl_set_vcpuaffinity(cfg->ctx, vm->def->id, vcpu, &map) != 0) { @@ -2374,20 +2382,9 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu, } } - if (!targetDef->cputune.vcpupin) { - if (VIR_ALLOC(targetDef->cputune.vcpupin) < 0) - goto endjob; - targetDef->cputune.nvcpupin = 0; - } - if (virDomainPinAdd(&targetDef->cputune.vcpupin, - &targetDef->cputune.nvcpupin, - cpumap, - maplen, - vcpu) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("failed to update or add vcpupin xml")); - goto endjob; - } + virBitmapFree(vcpuinfo->cpumask); + vcpuinfo->cpumask = pcpumap; + pcpumap = NULL; ret = 0; @@ -2463,15 +2460,14 @@ libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps, memset(cpumaps, 0x00, maplen * ncpumaps); for (vcpu = 0; vcpu < ncpumaps; vcpu++) { - virDomainPinDefPtr pininfo; + virDomainVcpuInfoPtr vcpuinfo = virDomainDefGetVcpu(targetDef, vcpu); virBitmapPtr bitmap = NULL; - pininfo = virDomainPinFind(targetDef->cputune.vcpupin, - targetDef->cputune.nvcpupin, - vcpu); + if (!vcpuinfo->online) + continue; - if (pininfo && pininfo->cpumask) - bitmap = pininfo->cpumask; + if (vcpuinfo->cpumask) + bitmap = vcpuinfo->cpumask; else if (targetDef->cpumask) bitmap = targetDef->cpumask; else diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 12c81d9..3cfc9e3 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1024,7 +1024,7 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) virCgroupPtr cgroup_vcpu = 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; @@ -1082,20 +1082,13 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) virCgroupSetCpusetMems(cgroup_vcpu, mem_mask) < 0) goto cleanup; - /* try to use the default cpu maps */ - if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) + if (vcpu->cpumask) + cpumap = vcpu->cpumask; + else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) cpumap = priv->autoCpuset; else cpumap = vm->def->cpumask; - /* lookup a more specific pinning info */ - for (j = 0; j < def->cputune.nvcpupin; j++) { - if (def->cputune.vcpupin[j]->id == i) { - cpumap = def->cputune.vcpupin[j]->cpumask; - break; - } - } - if (cpumap && qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) < 0) goto cleanup; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3e891e4..da41f13 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4869,10 +4869,8 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, VIR_CGROUP_THREAD_VCPU, vcpu) < 0) goto cleanup; - /* Free vcpupin setting */ - virDomainPinDel(&vm->def->cputune.vcpupin, - &vm->def->cputune.nvcpupin, - vcpu); + virBitmapFree(vcpuinfo->cpumask); + vcpuinfo->cpumask = NULL; ret = 0; @@ -5033,10 +5031,15 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (persistentDef) { /* remove vcpupin entries for vcpus that were unplugged */ if (nvcpus < virDomainDefGetVcpus(persistentDef)) { - for (i = virDomainDefGetVcpus(persistentDef) - 1; i >= nvcpus; i--) - virDomainPinDel(&persistentDef->cputune.vcpupin, - &persistentDef->cputune.nvcpupin, - i); + for (i = virDomainDefGetVcpus(persistentDef) - 1; i >= nvcpus; i--) { + virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(persistentDef, + i); + + if (vcpu) { + virBitmapFree(vcpu->cpumask); + vcpu->cpumask = NULL; + } + } } if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { @@ -5096,9 +5099,11 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, virCgroupPtr cgroup_vcpu = NULL; int ret = -1; qemuDomainObjPrivatePtr priv; - size_t newVcpuPinNum = 0; - virDomainPinDefPtr *newVcpuPin = NULL; virBitmapPtr pcpumap = NULL; + virBitmapPtr pcpumaplive = NULL; + virBitmapPtr pcpumappersist = NULL; + virDomainVcpuInfoPtr vcpuinfolive = NULL; + virDomainVcpuInfoPtr vcpuinfopersist = NULL; virQEMUDriverConfigPtr cfg = NULL; virObjectEventPtr event = NULL; char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = ""; @@ -5126,18 +5131,36 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, priv = vm->privateData; - if (def && vcpu >= virDomainDefGetVcpus(def)) { - virReportError(VIR_ERR_INVALID_ARG, - _("vcpu %d is out of range of live cpu count %d"), - vcpu, virDomainDefGetVcpus(def)); - goto endjob; + if (def) { + if (!(vcpuinfolive = virDomainDefGetVcpu(def, vcpu))) { + virReportError(VIR_ERR_INVALID_ARG, + _("vcpu %d is out of range of live cpu count %d"), + vcpu, virDomainDefGetVcpus(def)); + goto endjob; + } + + if (!vcpuinfolive->online) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("setting cpu pinning for inactive vcpu '%d' is not " + "supported"), vcpu); + goto endjob; + } } - if (persistentDef && vcpu >= virDomainDefGetVcpus(persistentDef)) { - virReportError(VIR_ERR_INVALID_ARG, - _("vcpu %d is out of range of persistent cpu count %d"), - vcpu, virDomainDefGetVcpus(persistentDef)); - goto endjob; + if (persistentDef) { + if (!(vcpuinfopersist = virDomainDefGetVcpu(persistentDef, vcpu))) { + virReportError(VIR_ERR_INVALID_ARG, + _("vcpu %d is out of range of persistent cpu count %d"), + vcpu, virDomainDefGetVcpus(persistentDef)); + goto endjob; + } + + if (!vcpuinfopersist->online) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("setting cpu pinning for inactive vcpu '%d' is not " + "supported"), vcpu); + goto endjob; + } } if (!(pcpumap = virBitmapNewData(cpumap, maplen))) @@ -5149,6 +5172,10 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, goto endjob; } + if ((def && !(pcpumaplive = virBitmapNewCopy(pcpumap))) || + (persistentDef && !(pcpumappersist = virBitmapNewCopy(pcpumap)))) + goto endjob; + if (def) { if (!qemuDomainHasVcpuPids(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -5156,26 +5183,6 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, goto endjob; } - if (def->cputune.vcpupin) { - newVcpuPin = virDomainPinDefCopy(def->cputune.vcpupin, - def->cputune.nvcpupin); - if (!newVcpuPin) - goto endjob; - - newVcpuPinNum = def->cputune.nvcpupin; - } else { - if (VIR_ALLOC(newVcpuPin) < 0) - goto endjob; - newVcpuPinNum = 0; - } - - if (virDomainPinAdd(&newVcpuPin, &newVcpuPinNum, - cpumap, maplen, vcpu) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to update vcpupin")); - goto endjob; - } - /* Configure the corresponding cpuset cgroup before set affinity. */ if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, vcpu, @@ -5197,13 +5204,9 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, } } - if (def->cputune.vcpupin) - virDomainPinDefArrayFree(def->cputune.vcpupin, - def->cputune.nvcpupin); - - def->cputune.vcpupin = newVcpuPin; - def->cputune.nvcpupin = newVcpuPinNum; - newVcpuPin = NULL; + virBitmapFree(vcpuinfolive->cpumask); + vcpuinfolive->cpumask = pcpumaplive; + pcpumaplive = NULL; if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) goto endjob; @@ -5222,21 +5225,9 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, } if (persistentDef) { - if (!persistentDef->cputune.vcpupin) { - if (VIR_ALLOC(persistentDef->cputune.vcpupin) < 0) - goto endjob; - persistentDef->cputune.nvcpupin = 0; - } - if (virDomainPinAdd(&persistentDef->cputune.vcpupin, - &persistentDef->cputune.nvcpupin, - cpumap, - maplen, - vcpu) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to update or add vcpupin xml of " - "a persistent domain")); - goto endjob; - } + virBitmapFree(vcpuinfopersist->cpumask); + vcpuinfopersist->cpumask = pcpumappersist; + pcpumappersist = NULL; ret = virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef); goto endjob; @@ -5248,14 +5239,14 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: - if (newVcpuPin) - virDomainPinDefArrayFree(newVcpuPin, newVcpuPinNum); if (cgroup_vcpu) virCgroupFree(&cgroup_vcpu); virDomainObjEndAPI(&vm); qemuDomainEventQueue(driver, event); VIR_FREE(str); virBitmapFree(pcpumap); + virBitmapFree(pcpumaplive); + virBitmapFree(pcpumappersist); virObjectUnref(cfg); return ret; } @@ -5280,7 +5271,8 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom, virDomainObjPtr vm = NULL; virDomainDefPtr def; int ret = -1; - int hostcpus, vcpu; + int hostcpus; + size_t i; virBitmapPtr allcpumap = NULL; qemuDomainObjPrivatePtr priv = NULL; @@ -5312,16 +5304,15 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom, if (ncpumaps < 1) goto cleanup; - for (vcpu = 0; vcpu < ncpumaps; vcpu++) { - virDomainPinDefPtr pininfo; + for (i = 0; i < ncpumaps; i++) { + virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(def, i); virBitmapPtr bitmap = NULL; - pininfo = virDomainPinFind(def->cputune.vcpupin, - def->cputune.nvcpupin, - vcpu); + if (!vcpu->online) + continue; - if (pininfo && pininfo->cpumask) - bitmap = pininfo->cpumask; + if (vcpu->cpumask) + bitmap = vcpu->cpumask; else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO && priv->autoCpuset) bitmap = priv->autoCpuset; @@ -5330,7 +5321,7 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom, else bitmap = allcpumap; - virBitmapToDataBuf(bitmap, VIR_GET_CPUMAP(cpumaps, maplen, vcpu), maplen); + virBitmapToDataBuf(bitmap, VIR_GET_CPUMAP(cpumaps, maplen, i), maplen); } ret = ncpumaps; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index fa646f2..a1f2729 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2193,23 +2193,23 @@ static int qemuProcessSetVcpuAffinities(virDomainObjPtr vm) { virDomainDefPtr def = vm->def; - virDomainPinDefPtr pininfo; - int n; + virDomainVcpuInfoPtr vcpu; + size_t i; int ret = -1; - VIR_DEBUG("Setting affinity on CPUs nvcpupin=%zu nvcpus=%d hasVcpupids=%d", - def->cputune.nvcpupin, virDomainDefGetVcpus(def), - qemuDomainHasVcpuPids(vm)); - if (!def->cputune.nvcpupin) - return 0; + VIR_DEBUG("Setting affinity on CPUs"); if (!qemuDomainHasVcpuPids(vm)) { /* If any CPU has custom affinity that differs from the * VM default affinity, we must reject it */ - for (n = 0; n < def->cputune.nvcpupin; n++) { - if (def->cputune.vcpupin[n]->cpumask && - !virBitmapEqual(def->cpumask, - def->cputune.vcpupin[n]->cpumask)) { + for (i = 0; i < virDomainDefGetVcpusMax(def); i++) { + vcpu = virDomainDefGetVcpu(def, i); + + if (!vcpu->online) + continue; + + if (vcpu->cpumask && + !virBitmapEqual(def->cpumask, vcpu->cpumask)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cpu affinity is not supported")); return -1; @@ -2218,19 +2218,19 @@ qemuProcessSetVcpuAffinities(virDomainObjPtr vm) return 0; } - for (n = 0; n < virDomainDefGetVcpus(def); n++) { + for (i = 0; i < virDomainDefGetVcpusMax(def); i++) { virBitmapPtr bitmap; - /* set affinity only for existing vcpus */ - if (!(pininfo = virDomainPinFind(def->cputune.vcpupin, - def->cputune.nvcpupin, - n))) + + vcpu = virDomainDefGetVcpu(def, i); + + if (!vcpu->online) continue; - if (!(bitmap = pininfo->cpumask) && + if (!(bitmap = vcpu->cpumask) && !(bitmap = def->cpumask)) continue; - if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, n), bitmap) < 0) + if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, i), bitmap) < 0) goto cleanup; } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 1a7f43b..727382e 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2455,15 +2455,14 @@ static int testDomainGetVcpus(virDomainPtr domain, memset(cpumaps, 0, maxinfo * maplen); for (i = 0; i < maxinfo; i++) { - virDomainPinDefPtr pininfo; + virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(def, i); virBitmapPtr bitmap = NULL; - pininfo = virDomainPinFind(def->cputune.vcpupin, - def->cputune.nvcpupin, - i); + if (!vcpu->online) + continue; - if (pininfo && pininfo->cpumask) - bitmap = pininfo->cpumask; + if (vcpu->cpumask) + bitmap = vcpu->cpumask; else if (def->cpumask) bitmap = def->cpumask; else @@ -2492,6 +2491,7 @@ static int testDomainPinVcpu(virDomainPtr domain, unsigned char *cpumap, int maplen) { + virDomainVcpuInfoPtr vcpuinfo; virDomainObjPtr privdom; virDomainDefPtr def; int ret = -1; @@ -2507,29 +2507,21 @@ static int testDomainPinVcpu(virDomainPtr domain, goto cleanup; } - if (vcpu > virDomainDefGetVcpus(privdom->def)) { + if (!(vcpuinfo = virDomainDefGetVcpu(def, vcpu)) || + !vcpuinfo->online) { virReportError(VIR_ERR_INVALID_ARG, _("requested vcpu '%d' is not present in the domain"), vcpu); goto cleanup; } - if (!def->cputune.vcpupin) { - if (VIR_ALLOC(def->cputune.vcpupin) < 0) - goto cleanup; - def->cputune.nvcpupin = 0; - } - if (virDomainPinAdd(&def->cputune.vcpupin, - &def->cputune.nvcpupin, - cpumap, - maplen, - vcpu) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to update or add vcpupin")); + virBitmapFree(vcpuinfo->cpumask); + + if (!(vcpuinfo->cpumask = virBitmapNewData(cpumap, maplen))) goto cleanup; - } ret = 0; + cleanup: virDomainObjEndAPI(&privdom); return ret; @@ -2566,15 +2558,14 @@ testDomainGetVcpuPinInfo(virDomainPtr dom, ncpumaps = virDomainDefGetVcpus(def); for (vcpu = 0; vcpu < ncpumaps; vcpu++) { - virDomainPinDefPtr pininfo; + virDomainVcpuInfoPtr vcpuinfo = virDomainDefGetVcpu(def, vcpu); virBitmapPtr bitmap = NULL; - pininfo = virDomainPinFind(def->cputune.vcpupin, - def->cputune.nvcpupin, - vcpu); + if (!vcpuinfo->online) + continue; - if (pininfo && pininfo->cpumask) - bitmap = pininfo->cpumask; + if (vcpuinfo->cpumask) + bitmap = vcpuinfo->cpumask; else if (def->cpumask) bitmap = def->cpumask; else diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 7cc24d3..30ea545 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1957,14 +1957,14 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, virDomainDefPtr def) return -1; } - if (def->cputune.vcpupin) { - for (i = 0; i < def->cputune.nvcpupin; i++) { - if (def->cputune.vcpupin[i]->cpumask && - !virBitmapEqual(def->cpumask, def->cputune.vcpupin[i]->cpumask)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("vcpupin cpumask differs from default cpumask")); - return -1; - } + for (i = 0; i < virDomainDefGetVcpusMax(def); i++) { + virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(def, i); + + if (vcpu->cpumask && + !virBitmapEqual(def->cpumask, vcpu->cpumask)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vcpupin cpumask differs from default cpumask")); + return -1; } } -- 2.6.2

virDomainDefFormatInternal is growing rather large. Extract the cputune formatter into a separate function. --- src/conf/domain_conf.c | 230 +++++++++++++++++++++++++++---------------------- 1 file changed, 125 insertions(+), 105 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1ace5a0..a9171df 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21467,6 +21467,129 @@ virDomainDefHasCapabilitiesFeatures(virDomainDefPtr def) return false; } + +static int +virDomainCputuneDefFormat(virBufferPtr buf, + virDomainDefPtr def) +{ + size_t i; + virBuffer childrenBuf = VIR_BUFFER_INITIALIZER; + int ret = -1; + + virBufferAdjustIndent(&childrenBuf, virBufferGetIndent(buf, false) + 2); + + if (def->cputune.sharesSpecified) + virBufferAsprintf(&childrenBuf, "<shares>%lu</shares>\n", + def->cputune.shares); + if (def->cputune.period) + virBufferAsprintf(&childrenBuf, "<period>%llu</period>\n", + def->cputune.period); + if (def->cputune.quota) + virBufferAsprintf(&childrenBuf, "<quota>%lld</quota>\n", + def->cputune.quota); + + if (def->cputune.emulator_period) + virBufferAsprintf(&childrenBuf, "<emulator_period>%llu" + "</emulator_period>\n", + def->cputune.emulator_period); + + if (def->cputune.emulator_quota) + virBufferAsprintf(&childrenBuf, "<emulator_quota>%lld" + "</emulator_quota>\n", + def->cputune.emulator_quota); + + for (i = 0; i < def->maxvcpus; i++) { + char *cpumask; + virDomainVcpuInfoPtr vcpu = def->vcpus + i; + + if (!vcpu->cpumask) + continue; + + if (!(cpumask = virBitmapFormat(vcpu->cpumask))) + goto cleanup; + + virBufferAsprintf(&childrenBuf, + "<vcpupin vcpu='%zu' cpuset='%s'/>\n", i, cpumask); + + VIR_FREE(cpumask); + } + + if (def->cputune.emulatorpin) { + char *cpumask; + virBufferAddLit(&childrenBuf, "<emulatorpin "); + + if (!(cpumask = virBitmapFormat(def->cputune.emulatorpin))) + goto cleanup; + + virBufferAsprintf(&childrenBuf, "cpuset='%s'/>\n", cpumask); + VIR_FREE(cpumask); + } + + for (i = 0; i < def->niothreadids; i++) { + char *cpumask; + + /* Ignore iothreadids with no cpumask */ + if (!def->iothreadids[i]->cpumask) + continue; + + virBufferAsprintf(&childrenBuf, "<iothreadpin iothread='%u' ", + def->iothreadids[i]->iothread_id); + + if (!(cpumask = virBitmapFormat(def->iothreadids[i]->cpumask))) + goto cleanup; + + virBufferAsprintf(&childrenBuf, "cpuset='%s'/>\n", cpumask); + VIR_FREE(cpumask); + } + + for (i = 0; i < def->cputune.nvcpusched; i++) { + virDomainThreadSchedParamPtr sp = &def->cputune.vcpusched[i]; + char *ids = NULL; + + if (!(ids = virBitmapFormat(sp->ids))) + goto cleanup; + + virBufferAsprintf(&childrenBuf, "<vcpusched vcpus='%s' scheduler='%s'", + ids, virProcessSchedPolicyTypeToString(sp->policy)); + VIR_FREE(ids); + + if (sp->policy == VIR_PROC_POLICY_FIFO || + sp->policy == VIR_PROC_POLICY_RR) + virBufferAsprintf(&childrenBuf, " priority='%d'", sp->priority); + virBufferAddLit(&childrenBuf, "/>\n"); + } + + for (i = 0; i < def->cputune.niothreadsched; i++) { + virDomainThreadSchedParamPtr sp = &def->cputune.iothreadsched[i]; + char *ids = NULL; + + if (!(ids = virBitmapFormat(sp->ids))) + goto cleanup; + + virBufferAsprintf(&childrenBuf, "<iothreadsched iothreads='%s' scheduler='%s'", + ids, virProcessSchedPolicyTypeToString(sp->policy)); + VIR_FREE(ids); + + if (sp->policy == VIR_PROC_POLICY_FIFO || + sp->policy == VIR_PROC_POLICY_RR) + virBufferAsprintf(&childrenBuf, " priority='%d'", sp->priority); + virBufferAddLit(&childrenBuf, "/>\n"); + } + + if (virBufferUse(&childrenBuf)) { + virBufferAddLit(buf, "<cputune>\n"); + virBufferAddBuffer(buf, &childrenBuf); + virBufferAddLit(buf, "</cputune>\n"); + } + + ret = 0; + + cleanup: + virBufferFreeAndReset(&childrenBuf); + return ret; +} + + /* This internal version appends to an existing buffer * (possibly with auto-indent), rather than flattening * to string. @@ -21679,111 +21802,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, } } - /* start format cputune */ - indent = virBufferGetIndent(buf, false); - virBufferAdjustIndent(&childrenBuf, indent + 2); - if (def->cputune.sharesSpecified) - virBufferAsprintf(&childrenBuf, "<shares>%lu</shares>\n", - def->cputune.shares); - if (def->cputune.period) - virBufferAsprintf(&childrenBuf, "<period>%llu</period>\n", - def->cputune.period); - if (def->cputune.quota) - virBufferAsprintf(&childrenBuf, "<quota>%lld</quota>\n", - def->cputune.quota); - - if (def->cputune.emulator_period) - virBufferAsprintf(&childrenBuf, "<emulator_period>%llu" - "</emulator_period>\n", - def->cputune.emulator_period); - - if (def->cputune.emulator_quota) - virBufferAsprintf(&childrenBuf, "<emulator_quota>%lld" - "</emulator_quota>\n", - def->cputune.emulator_quota); - - for (i = 0; i < def->maxvcpus; i++) { - char *cpumask; - virDomainVcpuInfoPtr vcpu = def->vcpus + i; - - if (!vcpu->cpumask) - continue; - - if (!(cpumask = virBitmapFormat(vcpu->cpumask))) - goto error; - - virBufferAsprintf(&childrenBuf, - "<vcpupin vcpu='%zu' cpuset='%s'/>\n", i, cpumask); - - VIR_FREE(cpumask); - } - - if (def->cputune.emulatorpin) { - char *cpumask; - virBufferAddLit(&childrenBuf, "<emulatorpin "); - - if (!(cpumask = virBitmapFormat(def->cputune.emulatorpin))) - goto error; - - virBufferAsprintf(&childrenBuf, "cpuset='%s'/>\n", cpumask); - VIR_FREE(cpumask); - } - - for (i = 0; i < def->niothreadids; i++) { - char *cpumask; - - /* Ignore iothreadids with no cpumask */ - if (!def->iothreadids[i]->cpumask) - continue; - - virBufferAsprintf(&childrenBuf, "<iothreadpin iothread='%u' ", - def->iothreadids[i]->iothread_id); - - if (!(cpumask = virBitmapFormat(def->iothreadids[i]->cpumask))) - goto error; - - virBufferAsprintf(&childrenBuf, "cpuset='%s'/>\n", cpumask); - VIR_FREE(cpumask); - } - - for (i = 0; i < def->cputune.nvcpusched; i++) { - virDomainThreadSchedParamPtr sp = &def->cputune.vcpusched[i]; - char *ids = NULL; - - if (!(ids = virBitmapFormat(sp->ids))) - goto error; - virBufferAsprintf(&childrenBuf, "<vcpusched vcpus='%s' scheduler='%s'", - ids, virProcessSchedPolicyTypeToString(sp->policy)); - VIR_FREE(ids); - - if (sp->policy == VIR_PROC_POLICY_FIFO || - sp->policy == VIR_PROC_POLICY_RR) - virBufferAsprintf(&childrenBuf, " priority='%d'", sp->priority); - virBufferAddLit(&childrenBuf, "/>\n"); - } - - for (i = 0; i < def->cputune.niothreadsched; i++) { - virDomainThreadSchedParamPtr sp = &def->cputune.iothreadsched[i]; - char *ids = NULL; - - if (!(ids = virBitmapFormat(sp->ids))) - goto error; - virBufferAsprintf(&childrenBuf, "<iothreadsched iothreads='%s' scheduler='%s'", - ids, virProcessSchedPolicyTypeToString(sp->policy)); - VIR_FREE(ids); - - if (sp->policy == VIR_PROC_POLICY_FIFO || - sp->policy == VIR_PROC_POLICY_RR) - virBufferAsprintf(&childrenBuf, " priority='%d'", sp->priority); - virBufferAddLit(&childrenBuf, "/>\n"); - } - - if (virBufferUse(&childrenBuf)) { - virBufferAddLit(buf, "<cputune>\n"); - virBufferAddBuffer(buf, &childrenBuf); - virBufferAddLit(buf, "</cputune>\n"); - } - virBufferFreeAndReset(&childrenBuf); + if (virDomainCputuneDefFormat(buf, def) < 0) + goto error; if (virDomainNumatuneFormatXML(buf, def->numa) < 0) goto error; -- 2.6.2

Similarly to previous commit change the way how iothread scheduler info is stored and clean up a lot of unnecessary code. --- src/conf/domain_conf.c | 141 +++++++-------------- src/conf/domain_conf.h | 8 +- src/libvirt_private.syms | 1 - src/qemu/qemu_driver.c | 3 - src/qemu/qemu_process.c | 39 +----- .../qemuxml2xmlout-cputune-iothreadsched.xml | 3 +- 6 files changed, 53 insertions(+), 142 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c88af32..e4047f3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2560,10 +2560,6 @@ void virDomainDefFree(virDomainDefPtr def) virBitmapFree(def->cputune.emulatorpin); - for (i = 0; i < def->cputune.niothreadsched; i++) - virBitmapFree(def->cputune.iothreadsched[i].ids); - VIR_FREE(def->cputune.iothreadsched); - virDomainNumaFree(def->numa); virSysinfoDefFree(def->sysinfo); @@ -14648,25 +14644,26 @@ virDomainVcpuThreadSchedParse(xmlNodePtr node, } -static int -virDomainThreadSchedParse(xmlNodePtr node, - unsigned int minid, - unsigned int maxid, - const char *name, - virDomainThreadSchedParamPtr sp) -{ - if (!(sp->ids = virDomainSchedulerParse(node, name, &sp->policy, - &sp->priority))) - return -1; +static virDomainThreadSchedParamPtr +virDomainDefGetIOThreadSched(virDomainDefPtr def, + unsigned int iothread) +{ + virDomainIOThreadIDDefPtr iothrinfo; - if (virBitmapNextSetBit(sp->ids, -1) < minid || - virBitmapLastSetBit(sp->ids) > maxid) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("%sched bitmap is out of range"), name); - return -1; - } + if (!(iothrinfo = virDomainIOThreadIDFind(def, iothread))) + return NULL; - return 0; + return &iothrinfo->sched; +} + + +static int +virDomainIOThreadSchedParse(xmlNodePtr node, + virDomainDefPtr def) +{ + return virDomainThreadSchedParseHelper(node, "iothreads", + virDomainDefGetIOThreadSched, + def); } @@ -15215,46 +15212,10 @@ virDomainDefParseXML(xmlDocPtr xml, _("cannot extract iothreadsched nodes")); goto error; } - if (n) { - if (n > def->niothreadids) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("too many iothreadsched nodes in cputune")); - goto error; - } - if (VIR_ALLOC_N(def->cputune.iothreadsched, n) < 0) + for (i = 0; i < n; i++) { + if (virDomainIOThreadSchedParse(nodes[i], def) < 0) goto error; - def->cputune.niothreadsched = n; - - for (i = 0; i < def->cputune.niothreadsched; i++) { - ssize_t pos = -1; - - if (virDomainThreadSchedParse(nodes[i], - 1, UINT_MAX, - "iothreads", - &def->cputune.iothreadsched[i]) < 0) - goto error; - - while ((pos = virBitmapNextSetBit(def->cputune.iothreadsched[i].ids, - pos)) > -1) { - if (!virDomainIOThreadIDFind(def, pos)) { - virReportError(VIR_ERR_XML_DETAIL, "%s", - _("iothreadsched attribute 'iothreads' " - "uses undefined iothread ids")); - goto error; - } - } - - for (j = 0; j < i; j++) { - if (virBitmapOverlaps(def->cputune.iothreadsched[i].ids, - def->cputune.iothreadsched[j].ids)) { - virReportError(VIR_ERR_XML_DETAIL, "%s", - _("iothreadsched attributes 'iothreads' " - "must not overlap")); - goto error; - } - } - } } VIR_FREE(nodes); @@ -18406,29 +18367,6 @@ virDomainIOThreadIDDel(virDomainDefPtr def, } } -void -virDomainIOThreadSchedDelId(virDomainDefPtr def, - unsigned int iothreadid) -{ - size_t i; - - if (!def->cputune.iothreadsched || !def->cputune.niothreadsched) - return; - - for (i = 0; i < def->cputune.niothreadsched; i++) { - if (virBitmapIsBitSet(def->cputune.iothreadsched[i].ids, iothreadid)) { - ignore_value(virBitmapClearBit(def->cputune.iothreadsched[i].ids, - iothreadid)); - if (virBitmapIsAllClear(def->cputune.iothreadsched[i].ids)) { - virBitmapFree(def->cputune.iothreadsched[i].ids); - VIR_DELETE_ELEMENT(def->cputune.iothreadsched, i, - def->cputune.niothreadsched); - } - return; - } - } -} - static int virDomainEventActionDefFormat(virBufferPtr buf, @@ -21645,6 +21583,27 @@ virDomainFormatVcpuSchedDef(virDomainDefPtr def, static int +virDomainFormatIOThreadSchedDef(virDomainDefPtr def, + virBufferPtr buf) +{ + virBitmapPtr allthreadmap; + int ret; + + if (def->niothreadids == 0) + return 0; + + if (!(allthreadmap = virDomainIOThreadIDMap(def))) + return -1; + + ret = virDomainFormatSchedDef(def, buf, "iothreads", + virDomainDefGetIOThreadSched, allthreadmap); + + virBitmapFree(allthreadmap); + return ret; +} + + +static int virDomainCputuneDefFormat(virBufferPtr buf, virDomainDefPtr def) { @@ -21721,22 +21680,8 @@ virDomainCputuneDefFormat(virBufferPtr buf, if (virDomainFormatVcpuSchedDef(def, &childrenBuf) < 0) goto cleanup; - for (i = 0; i < def->cputune.niothreadsched; i++) { - virDomainThreadSchedParamPtr sp = &def->cputune.iothreadsched[i]; - char *ids = NULL; - - if (!(ids = virBitmapFormat(sp->ids))) - goto cleanup; - - virBufferAsprintf(&childrenBuf, "<iothreadsched iothreads='%s' scheduler='%s'", - ids, virProcessSchedPolicyTypeToString(sp->policy)); - VIR_FREE(ids); - - if (sp->policy == VIR_PROC_POLICY_FIFO || - sp->policy == VIR_PROC_POLICY_RR) - virBufferAsprintf(&childrenBuf, " priority='%d'", sp->priority); - virBufferAddLit(&childrenBuf, "/>\n"); - } + if (virDomainFormatIOThreadSchedDef(def, &childrenBuf) < 0) + goto cleanup; if (virBufferUse(&childrenBuf)) { virBufferAddLit(buf, "<cputune>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3868c13..ba87fea 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1898,7 +1898,6 @@ typedef enum { typedef struct _virDomainThreadSchedParam virDomainThreadSchedParam; typedef virDomainThreadSchedParam *virDomainThreadSchedParamPtr; struct _virDomainThreadSchedParam { - virBitmapPtr ids; virProcessSchedPolicy policy; int priority; }; @@ -2095,6 +2094,8 @@ struct _virDomainIOThreadIDDef { unsigned int iothread_id; int thread_id; virBitmapPtr cpumask; + + virDomainThreadSchedParam sched; }; void virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def); @@ -2111,9 +2112,6 @@ struct _virDomainCputune { unsigned long long emulator_period; long long emulator_quota; virBitmapPtr emulatorpin; - - size_t niothreadsched; - virDomainThreadSchedParamPtr iothreadsched; }; @@ -2124,7 +2122,6 @@ struct _virDomainVcpuInfo { bool online; virBitmapPtr cpumask; - /* note: the sched.ids bitmap is unused so it doesn't have to be cleared */ virDomainThreadSchedParam sched; }; @@ -2705,7 +2702,6 @@ virDomainIOThreadIDDefPtr virDomainIOThreadIDAdd(virDomainDefPtr def, virBitmapPtr virDomainIOThreadIDMap(virDomainDefPtr def) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; void virDomainIOThreadIDDel(virDomainDefPtr def, unsigned int iothread_id); -void virDomainIOThreadSchedDelId(virDomainDefPtr def, unsigned int iothread_id); unsigned int virDomainDefFormatConvertXMLFlags(unsigned int flags); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 12cc887..b15da38 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -346,7 +346,6 @@ virDomainIOThreadIDDefFree; virDomainIOThreadIDDel; virDomainIOThreadIDFind; virDomainIOThreadIDMap; -virDomainIOThreadSchedDelId; virDomainKeyWrapCipherNameTypeFromString; virDomainKeyWrapCipherNameTypeToString; virDomainLeaseDefFree; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 21a050d..aaeeedb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6143,8 +6143,6 @@ qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver, virDomainIOThreadIDDel(vm->def, iothread_id); - virDomainIOThreadSchedDelId(vm->def, iothread_id); - if (qemuDomainDelCgroupForThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, iothread_id) < 0) @@ -6234,7 +6232,6 @@ qemuDomainChgIOThread(virQEMUDriverPtr driver, } virDomainIOThreadIDDel(persistentDef, iothread_id); - virDomainIOThreadSchedDelId(persistentDef, iothread_id); persistentDef->iothreads--; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 311a69c..69e618d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2281,34 +2281,6 @@ qemuProcessSetIOThreadsAffinity(virDomainObjPtr vm) return ret; } -/* Set Scheduler parameters for vCPU or I/O threads. */ -int -qemuProcessSetSchedParams(int id, - pid_t pid, - size_t nsp, - virDomainThreadSchedParamPtr sp) -{ - bool val = false; - size_t i = 0; - virDomainThreadSchedParamPtr s = NULL; - - for (i = 0; i < nsp; i++) { - if (virBitmapGetBit(sp[i].ids, id, &val) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot get bit from bitmap")); - } - if (val) { - s = &sp[i]; - break; - } - } - - if (!s) - return 0; - - return virProcessSetScheduler(pid, s->policy, s->priority); -} - static int qemuProcessSetSchedulers(virDomainObjPtr vm) { @@ -2327,10 +2299,13 @@ qemuProcessSetSchedulers(virDomainObjPtr vm) } 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) + virDomainIOThreadIDDefPtr info = vm->def->iothreadids[i]; + + if (info->sched.policy == VIR_PROC_POLICY_NONE) + continue; + + if (virProcessSetScheduler(info->thread_id, info->sched.policy, + info->sched.priority) < 0) return -1; } diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-iothreadsched.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-iothreadsched.xml index 5177114..4d665e9 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-iothreadsched.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-iothreadsched.xml @@ -13,8 +13,7 @@ <vcpupin vcpu='1' cpuset='1'/> <emulatorpin cpuset='1'/> <vcpusched vcpus='0-1' scheduler='fifo' priority='1'/> - <iothreadsched iothreads='1,3' scheduler='batch'/> - <iothreadsched iothreads='2' scheduler='batch'/> + <iothreadsched iothreads='1-3' scheduler='batch'/> </cputune> <os> <type arch='i686' machine='pc'>hvm</type> -- 2.6.2

Rather than iterating 3 times for various settings this function aggregates all the code into single place. One of the other advantages is that it can then be reused for properly setting IOThread info on hotplug. --- src/qemu/qemu_cgroup.c | 93 --------------------------- src/qemu/qemu_cgroup.h | 1 - src/qemu/qemu_process.c | 165 +++++++++++++++++++++++++++++++++--------------- src/qemu/qemu_process.h | 2 + 4 files changed, 116 insertions(+), 145 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index f3a9b5c..4b5cf36 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1084,99 +1084,6 @@ qemuSetupCgroupForEmulator(virDomainObjPtr vm) return -1; } -int -qemuSetupCgroupForIOThreads(virDomainObjPtr vm) -{ - virCgroupPtr cgroup_iothread = NULL; - qemuDomainObjPrivatePtr priv = vm->privateData; - virDomainDefPtr def = vm->def; - size_t i; - unsigned long long period = vm->def->cputune.period; - long long quota = vm->def->cputune.quota; - char *mem_mask = NULL; - virDomainNumatuneMemMode mem_mode; - - if (def->niothreadids == 0) - return 0; - - if ((period || quota) && - !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("cgroup cpu is required for scheduler tuning")); - return -1; - } - - /* - * If CPU cgroup controller is not initialized here, then we need - * neither period nor quota settings. And if CPUSET controller is - * not initialized either, then there's nothing to do anyway. - */ - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) && - !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) - return 0; - - if (virDomainNumatuneGetMode(vm->def->numa, -1, &mem_mode) == 0 && - mem_mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT && - virDomainNumatuneMaybeFormatNodeset(vm->def->numa, - priv->autoNodeset, - &mem_mask, -1) < 0) - goto cleanup; - - 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, - def->iothreadids[i]->iothread_id, - true, &cgroup_iothread) < 0) - goto cleanup; - - if (period || quota) { - if (qemuSetupCgroupVcpuBW(cgroup_iothread, period, quota) < 0) - goto cleanup; - } - - /* Set iothreadpin in cgroup if iothreadpin xml is provided */ - if (virCgroupHasController(priv->cgroup, - VIR_CGROUP_CONTROLLER_CPUSET)) { - virBitmapPtr cpumask = NULL; - - if (mem_mask && - virCgroupSetCpusetMems(cgroup_iothread, mem_mask) < 0) - goto cleanup; - - 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; - - if (cpumask && - qemuSetupCgroupCpusetCpus(cgroup_iothread, cpumask) < 0) - goto cleanup; - } - - /* move the thread for iothread to sub dir */ - if (virCgroupAddTask(cgroup_iothread, - def->iothreadids[i]->thread_id) < 0) - goto cleanup; - - virCgroupFree(&cgroup_iothread); - } - VIR_FREE(mem_mask); - - return 0; - - cleanup: - if (cgroup_iothread) { - virCgroupRemove(cgroup_iothread); - virCgroupFree(&cgroup_iothread); - } - VIR_FREE(mem_mask); - - return -1; -} int qemuRemoveCgroup(virDomainObjPtr vm) diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index 69d1202..021bfe6 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -53,7 +53,6 @@ int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, unsigned long long period, long long quota); int qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup, virBitmapPtr cpumask); -int qemuSetupCgroupForIOThreads(virDomainObjPtr vm); int qemuSetupCgroupForEmulator(virDomainObjPtr vm); int qemuRemoveCgroup(virDomainObjPtr vm); int qemuAddToCgroup(virDomainObjPtr vm); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f23a3c6..6de6d5a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2208,47 +2208,6 @@ qemuProcessSetEmulatorAffinity(virDomainObjPtr vm) return ret; } -/* Set CPU affinities for IOThreads threads. */ -static int -qemuProcessSetIOThreadsAffinity(virDomainObjPtr vm) -{ - virDomainDefPtr def = vm->def; - size_t i; - int ret = -1; - - for (i = 0; i < def->niothreadids; i++) { - /* set affinity only for existing iothreads */ - if (!def->iothreadids[i]->cpumask) - continue; - - if (virProcessSetAffinity(def->iothreadids[i]->thread_id, - def->iothreadids[i]->cpumask) < 0) - goto cleanup; - } - ret = 0; - - cleanup: - return ret; -} - -static int -qemuProcessSetSchedulers(virDomainObjPtr vm) -{ - size_t i = 0; - - for (i = 0; i < vm->def->niothreadids; i++) { - virDomainIOThreadIDDefPtr info = vm->def->iothreadids[i]; - - if (info->sched.policy == VIR_PROC_POLICY_NONE) - continue; - - if (virProcessSetScheduler(info->thread_id, info->sched.policy, - info->sched.priority) < 0) - return -1; - } - - return 0; -} static int qemuProcessInitPasswords(virConnectPtr conn, @@ -4548,6 +4507,118 @@ qemuProcessSetupVcpus(virDomainObjPtr vm) /** + * qemuProcessSetupIOThread: + * @vm: domain object + * @iothread: iothread data structure to set the data for + * + * This function sets resource properities (affinity, cgroups, scheduler) for a + * IOThread. This function expects that the IOThread is online and the IOThread + * pids were correctly detected at the point when it's called. + * + * Returns 0 on success, -1 on error. + */ +int +qemuProcessSetupIOThread(virDomainObjPtr vm, + virDomainIOThreadIDDefPtr iothread) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + unsigned long long period = vm->def->cputune.period; + long long quota = vm->def->cputune.quota; + virDomainNumatuneMemMode mem_mode; + char *mem_mask = NULL; + virCgroupPtr cgroup_iothread = NULL; + virBitmapPtr cpumask = NULL; + int ret = -1; + + if ((period || quota) && + !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cgroup cpu is required for scheduler tuning")); + return -1; + } + + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) || + virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { + if (virDomainNumatuneGetMode(vm->def->numa, -1, &mem_mode) == 0 && + mem_mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT && + virDomainNumatuneMaybeFormatNodeset(vm->def->numa, + priv->autoNodeset, + &mem_mask, -1) < 0) + goto cleanup; + + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, + iothread->iothread_id, + true, &cgroup_iothread) < 0) + goto cleanup; + } + + if (iothread->cpumask) + cpumask = iothread->cpumask; + else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) + cpumask = priv->autoCpuset; + else + cpumask = vm->def->cpumask; + + if (period || quota) { + if (qemuSetupCgroupVcpuBW(cgroup_iothread, period, quota) < 0) + goto cleanup; + } + + if (cpumask) { + if (virProcessSetAffinity(iothread->thread_id, cpumask) < 0) + goto cleanup; + } + + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { + if (mem_mask && + virCgroupSetCpusetMems(cgroup_iothread, mem_mask) < 0) + goto cleanup; + + if (cpumask && + qemuSetupCgroupCpusetCpus(cgroup_iothread, cpumask) < 0) + goto cleanup; + } + + if (cgroup_iothread && + virCgroupAddTask(cgroup_iothread, iothread->thread_id) < 0) + goto cleanup; + + if (iothread->sched.policy != VIR_PROC_POLICY_NONE && + virProcessSetScheduler(iothread->thread_id, iothread->sched.policy, + iothread->sched.priority) < 0) + goto cleanup; + + ret = 0; + + cleanup: + if (cgroup_iothread) { + if (ret < 0) + virCgroupRemove(cgroup_iothread); + virCgroupFree(&cgroup_iothread); + } + + VIR_FREE(mem_mask); + return ret; +} + + +static int +qemuProcessSetupIOThreads(virDomainObjPtr vm) +{ + size_t i; + + for (i = 0; i < vm->def->niothreadids; i++) { + virDomainIOThreadIDDefPtr info = vm->def->iothreadids[i]; + + if (qemuProcessSetupIOThread(vm, info) < 0) + return -1; + } + + return 0; +} + + +/** * qemuProcessLaunch: * * Launch a new QEMU process with stopped virtual CPUs. @@ -5022,16 +5093,8 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuProcessSetupVcpus(vm) < 0) goto cleanup; - VIR_DEBUG("Setting cgroup for each IOThread (if required)"); - if (qemuSetupCgroupForIOThreads(vm) < 0) - goto cleanup; - - VIR_DEBUG("Setting affinity of IOThread threads"); - if (qemuProcessSetIOThreadsAffinity(vm) < 0) - goto cleanup; - - VIR_DEBUG("Setting scheduler parameters"); - if (qemuProcessSetSchedulers(vm) < 0) + VIR_DEBUG("Setting IOThread tuning/settings"); + if (qemuProcessSetupIOThreads(vm) < 0) goto cleanup; VIR_DEBUG("Setting any required VM passwords"); diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index a3fdfdd..03953b0 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -156,5 +156,7 @@ int qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm); int qemuProcessSetupVcpu(virDomainObjPtr vm, unsigned int vcpuid); +int qemuProcessSetupIOThread(virDomainObjPtr vm, + virDomainIOThreadIDDefPtr iothread); #endif /* __QEMU_PROCESS_H__ */ -- 2.6.2

Since majority of the steps is shared, the function can be reused to simplify code. Similarly to previous path doing this same for vCPUs this also fixes the a similar bug (which is not tracked). --- src/qemu/qemu_driver.c | 101 +------------------------------------------------ 1 file changed, 2 insertions(+), 99 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9420fed..f595bc9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4649,70 +4649,6 @@ static void qemuProcessEventHandler(void *data, void *opaque) VIR_FREE(processEvent); } -static virCgroupPtr -qemuDomainAddCgroupForThread(virCgroupPtr cgroup, - virCgroupThreadName nameval, - int idx, - char *mem_mask, - pid_t pid) -{ - virCgroupPtr new_cgroup = NULL; - int rv = -1; - - /* Create cgroup */ - if (virCgroupNewThread(cgroup, nameval, idx, true, &new_cgroup) < 0) - return NULL; - - if (mem_mask && - virCgroupHasController(cgroup, VIR_CGROUP_CONTROLLER_CPUSET) && - virCgroupSetCpusetMems(new_cgroup, mem_mask) < 0) - goto error; - - /* Add pid/thread to the cgroup */ - rv = virCgroupAddTask(new_cgroup, pid); - if (rv < 0) { - virCgroupRemove(new_cgroup); - goto error; - } - - return new_cgroup; - - error: - virCgroupFree(&new_cgroup); - return NULL; -} - - -static int -qemuDomainHotplugPinThread(virBitmapPtr cpumask, - int idx, - pid_t pid, - virCgroupPtr cgroup) -{ - int ret = -1; - - if (cgroup && - virCgroupHasController(cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { - if (qemuSetupCgroupCpusetCpus(cgroup, cpumask) < 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("failed to set cpuset.cpus in cgroup for id %d"), - idx); - goto cleanup; - } - } else { - if (virProcessSetAffinity(pid, cpumask) < 0) { - virReportError(VIR_ERR_SYSTEM_ERROR, - _("failed to set cpu affinity for id %d"), - idx); - goto cleanup; - } - } - - ret = 0; - - cleanup: - return ret; -} static int qemuDomainDelCgroupForThread(virCgroupPtr cgroup, @@ -5936,11 +5872,7 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver, unsigned int exp_niothreads = vm->def->niothreadids; int new_niothreads = 0; qemuMonitorIOThreadInfoPtr *new_iothreads = NULL; - virCgroupPtr cgroup_iothread = NULL; - char *mem_mask = NULL; - virDomainNumatuneMemMode mode; virDomainIOThreadIDDefPtr iothrid; - virBitmapPtr cpumask; if (virDomainIOThreadIDFind(vm->def, iothread_id)) { virReportError(VIR_ERR_INVALID_ARG, @@ -5980,14 +5912,6 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver, } vm->def->iothreads = exp_niothreads; - if (virDomainNumatuneGetMode(vm->def->numa, -1, &mode) == 0 && - mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT && - virDomainNumatuneMaybeFormatNodeset(vm->def->numa, - priv->autoNodeset, - &mem_mask, -1) < 0) - goto cleanup; - - /* * If we've successfully added an IOThread, find out where we added it * in the QEMU IOThread list, so we can add it to our iothreadids list @@ -6009,27 +5933,8 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver, iothrid->thread_id = new_iothreads[idx]->thread_id; - /* Add IOThread to cgroup if present */ - if (priv->cgroup) { - cgroup_iothread = - qemuDomainAddCgroupForThread(priv->cgroup, - VIR_CGROUP_THREAD_IOTHREAD, - iothread_id, mem_mask, - iothrid->thread_id); - if (!cgroup_iothread) - goto cleanup; - } - - if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) - cpumask = priv->autoCpuset; - else - cpumask = vm->def->cpumask; - - if (cpumask) { - if (qemuDomainHotplugPinThread(cpumask, iothread_id, - iothrid->thread_id, cgroup_iothread) < 0) - goto cleanup; - } + if (qemuProcessSetupIOThread(vm, iothrid) < 0) + goto cleanup; ret = 0; @@ -6039,10 +5944,8 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver, VIR_FREE(new_iothreads[idx]); VIR_FREE(new_iothreads); } - VIR_FREE(mem_mask); virDomainAuditIOThread(vm, orig_niothreads, new_niothreads, "update", rc == 0); - virCgroupFree(&cgroup_iothread); VIR_FREE(alias); return ret; -- 2.6.2