[libvirt] [PATCH v4 0/4] vcpu info storage refactors - part 2
Yet another version since I've changed a few places other than requested in the review. Peter Krempa (4): qemu: vcpu: Aggregate code to set vCPU tuning qemu: vcpu: Reuse qemuProcessSetupVcpu in vcpu hotplug qemu: iothread: Aggregate code to set IOThread tuning qemu: iothread: Reuse qemuProcessSetupIOThread in iothread hotplug src/qemu/qemu_cgroup.c | 188 ------------------------ src/qemu/qemu_cgroup.h | 2 - src/qemu/qemu_driver.c | 136 +----------------- src/qemu/qemu_process.c | 376 +++++++++++++++++++++++++++++++++--------------- src/qemu/qemu_process.h | 6 + 5 files changed, 266 insertions(+), 442 deletions(-) -- 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 vCPU info on hotplug. With this approach autoCpuset is also used when setting the process affinity rather than just via cgroups. --- src/qemu/qemu_cgroup.c | 95 ---------------------- src/qemu/qemu_cgroup.h | 1 - src/qemu/qemu_process.c | 212 ++++++++++++++++++++++++++++++++---------------- src/qemu/qemu_process.h | 4 + 4 files changed, 148 insertions(+), 164 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 3cfc9e3..f3a9b5c 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1018,101 +1018,6 @@ qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup, return ret; } -int -qemuSetupCgroupForVcpu(virDomainObjPtr vm) -{ - virCgroupPtr cgroup_vcpu = 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 ((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. CPU pinning - * will be set via virProcessSetAffinity. - */ - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) && - !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) - return 0; - - /* If vCPU<->pid mapping is missing we can't do vCPU pinning */ - if (!qemuDomainHasVcpuPids(vm)) - 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 < virDomainDefGetVcpusMax(def); i++) { - virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(def, i); - - if (!vcpu->online) - continue; - - virCgroupFree(&cgroup_vcpu); - if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, i, - true, &cgroup_vcpu) < 0) - goto cleanup; - - if (period || quota) { - if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0) - goto cleanup; - } - - /* Set vcpupin in cgroup if vcpupin xml is provided */ - if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { - virBitmapPtr cpumap = NULL; - - if (mem_mask && - virCgroupSetCpusetMems(cgroup_vcpu, mem_mask) < 0) - goto cleanup; - - 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; - - if (cpumap && qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) < 0) - goto cleanup; - } - - /* move the thread for vcpu to sub dir */ - if (virCgroupAddTask(cgroup_vcpu, - qemuDomainGetVcpuPid(vm, i)) < 0) - goto cleanup; - - } - virCgroupFree(&cgroup_vcpu); - VIR_FREE(mem_mask); - - return 0; - - cleanup: - if (cgroup_vcpu) { - virCgroupRemove(cgroup_vcpu); - virCgroupFree(&cgroup_vcpu); - } - VIR_FREE(mem_mask); - - return -1; -} int qemuSetupCgroupForEmulator(virDomainObjPtr vm) diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index 347d126..69d1202 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 qemuSetupCgroupForVcpu(virDomainObjPtr vm); int qemuSetupCgroupForIOThreads(virDomainObjPtr vm); int qemuSetupCgroupForEmulator(virDomainObjPtr vm); int qemuRemoveCgroup(virDomainObjPtr vm); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3838dc8..fc87d07 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2188,56 +2188,6 @@ qemuProcessSetLinkStates(virQEMUDriverPtr driver, return ret; } -/* Set CPU affinities for vcpus if vcpupin xml provided. */ -static int -qemuProcessSetVcpuAffinities(virDomainObjPtr vm) -{ - virDomainDefPtr def = vm->def; - virDomainVcpuInfoPtr vcpu; - size_t i; - int ret = -1; - 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 (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; - } - } - return 0; - } - - for (i = 0; i < virDomainDefGetVcpusMax(def); i++) { - virBitmapPtr bitmap; - - vcpu = virDomainDefGetVcpu(def, i); - - if (!vcpu->online) - continue; - - if (!(bitmap = vcpu->cpumask) && - !(bitmap = def->cpumask)) - continue; - - if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, i), bitmap) < 0) - goto cleanup; - } - - ret = 0; - cleanup: - return ret; -} /* Set CPU affinities for emulator threads. */ static int @@ -2286,18 +2236,6 @@ qemuProcessSetSchedulers(virDomainObjPtr vm) { size_t i = 0; - for (i = 0; i < virDomainDefGetVcpusMax(vm->def); i++) { - virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(vm->def, i); - - if (!vcpu->online || - vcpu->sched.policy == VIR_PROC_POLICY_NONE) - continue; - - if (virProcessSetScheduler(qemuDomainGetVcpuPid(vm, i), - vcpu->sched.policy, vcpu->sched.priority) < 0) - return -1; - } - for (i = 0; i < vm->def->niothreadids; i++) { virDomainIOThreadIDDefPtr info = vm->def->iothreadids[i]; @@ -4495,6 +4433,148 @@ qemuProcessInit(virQEMUDriverPtr driver, /** + * qemuProcessSetupVcpu: + * @vm: domain object + * @vcpuid: id of VCPU to set defaults + * + * This function sets resource properities (affinity, cgroups, scheduler) for a + * vCPU. This function expects that the vCPU is online and the vCPU pids were + * correctly detected at the point when it's called. + * + * Returns 0 on success, -1 on error. + */ +int +qemuProcessSetupVcpu(virDomainObjPtr vm, + unsigned int vcpuid) +{ + pid_t vcpupid = qemuDomainGetVcpuPid(vm, vcpuid); + virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(vm->def, vcpuid); + qemuDomainObjPrivatePtr priv = vm->privateData; + char *mem_mask = NULL; + virDomainNumatuneMemMode mem_mode; + unsigned long long period = vm->def->cputune.period; + long long quota = vm->def->cputune.quota; + virCgroupPtr cgroup_vcpu = NULL; + virBitmapPtr cpumask; + 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")); + goto cleanup; + } + + 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_VCPU, vcpuid, + true, &cgroup_vcpu) < 0) + goto cleanup; + + if (period || quota) { + if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0) + goto cleanup; + } + } + + /* infer which cpumask shall be used */ + if (vcpu->cpumask) + cpumask = vcpu->cpumask; + else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) + cpumask = priv->autoCpuset; + else + cpumask = vm->def->cpumask; + + /* setup cgroups */ + if (cgroup_vcpu) { + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { + if (mem_mask && virCgroupSetCpusetMems(cgroup_vcpu, mem_mask) < 0) + goto cleanup; + + if (cpumask && qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumask) < 0) + goto cleanup; + } + + /* move the thread for vcpu to sub dir */ + if (virCgroupAddTask(cgroup_vcpu, vcpupid) < 0) + goto cleanup; + } + + /* setup legacy affinty */ + if (cpumask && virProcessSetAffinity(vcpupid, cpumask) < 0) + goto cleanup; + + /* set scheduler type and priority */ + if (vcpu->sched.policy != VIR_PROC_POLICY_NONE) { + if (virProcessSetScheduler(vcpupid, vcpu->sched.policy, + vcpu->sched.priority) < 0) + goto cleanup; + } + + ret = 0; + + cleanup: + VIR_FREE(mem_mask); + if (cgroup_vcpu) { + if (ret < 0) + virCgroupRemove(cgroup_vcpu); + virCgroupFree(&cgroup_vcpu); + } + + return ret; +} + + +static int +qemuProcessSetupVcpus(virDomainObjPtr vm) +{ + virDomainVcpuInfoPtr vcpu; + unsigned int maxvcpus = virDomainDefGetVcpusMax(vm->def); + size_t i; + + if (!qemuDomainHasVcpuPids(vm)) { + /* If any CPU has custom affinity that differs from the + * VM default affinity, we must reject it */ + for (i = 0; i < maxvcpus; i++) { + vcpu = virDomainDefGetVcpu(vm->def, i); + + if (!vcpu->online) + continue; + + if (vcpu->cpumask && + !virBitmapEqual(vm->def->cpumask, vcpu->cpumask)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cpu affinity is not supported")); + return -1; + } + } + + return 0; + } + + for (i = 0; i < maxvcpus; i++) { + vcpu = virDomainDefGetVcpu(vm->def, i); + + if (!vcpu->online) + continue; + + if (qemuProcessSetupVcpu(vm, i) < 0) + return -1; + } + + return 0; +} + + +/** * qemuProcessLaunch: * * Launch a new QEMU process with stopped virtual CPUs. @@ -4958,18 +5038,14 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuProcessDetectIOThreadPIDs(driver, vm, asyncJob) < 0) goto cleanup; - VIR_DEBUG("Setting cgroup for each VCPU (if required)"); - if (qemuSetupCgroupForVcpu(vm) < 0) + VIR_DEBUG("Setting vCPU tuning/settings"); + if (qemuProcessSetupVcpus(vm) < 0) goto cleanup; VIR_DEBUG("Setting cgroup for each IOThread (if required)"); if (qemuSetupCgroupForIOThreads(vm) < 0) goto cleanup; - VIR_DEBUG("Setting VCPU affinities"); - if (qemuProcessSetVcpuAffinities(vm) < 0) - goto cleanup; - VIR_DEBUG("Setting affinity of IOThread threads"); if (qemuProcessSetIOThreadsAffinity(vm) < 0) goto cleanup; diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 907a58d..7de9b89 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -160,4 +160,8 @@ virDomainDiskDefPtr qemuProcessFindDomainDiskByAlias(virDomainObjPtr vm, int qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm); + +int qemuProcessSetupVcpu(virDomainObjPtr vm, + unsigned int vcpuid); + #endif /* __QEMU_PROCESS_H__ */ -- 2.6.2
On Mon, Feb 08, 2016 at 12:26:30PM +0100, Peter Krempa wrote:
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 vCPU info on hotplug.
With this approach autoCpuset is also used when setting the process affinity rather than just via cgroups. --- src/qemu/qemu_cgroup.c | 95 ---------------------- src/qemu/qemu_cgroup.h | 1 - src/qemu/qemu_process.c | 212 ++++++++++++++++++++++++++++++++---------------- src/qemu/qemu_process.h | 4 + 4 files changed, 148 insertions(+), 164 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 3cfc9e3..f3a9b5c 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1018,101 +1018,6 @@ qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup, return ret; }
-int -qemuSetupCgroupForVcpu(virDomainObjPtr vm) -{ - virCgroupPtr cgroup_vcpu = 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 ((period || quota) && - !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("cgroup cpu is required for scheduler tuning")); - return -1; - } -
Before, this error was reported...
- /* - * 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. CPU pinning - * will be set via virProcessSetAffinity. - */ - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) && - !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) - return 0; - - /* If vCPU<->pid mapping is missing we can't do vCPU pinning */ - if (!qemuDomainHasVcpuPids(vm)) - return 0;
... before the VCPU pid check here.
- - 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)
/** + * qemuProcessSetupVcpu: + * @vm: domain object + * @vcpuid: id of VCPU to set defaults + * + * This function sets resource properities (affinity, cgroups, scheduler) for a
*properties. Also, cgroups, affinity, scheduler is the order.
+ * vCPU. This function expects that the vCPU is online and the vCPU pids were + * correctly detected at the point when it's called. + * + * Returns 0 on success, -1 on error. + */ +int +qemuProcessSetupVcpu(virDomainObjPtr vm, + unsigned int vcpuid) +{ + pid_t vcpupid = qemuDomainGetVcpuPid(vm, vcpuid); + virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(vm->def, vcpuid); + qemuDomainObjPrivatePtr priv = vm->privateData; + char *mem_mask = NULL; + virDomainNumatuneMemMode mem_mode; + unsigned long long period = vm->def->cputune.period; + long long quota = vm->def->cputune.quota; + virCgroupPtr cgroup_vcpu = NULL; + virBitmapPtr cpumask; + 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")); + goto cleanup; + } +
This error won't be displayed if the domain has no VCPU pids. ACK if you move it to the caller. Jan
Since majority of the steps is shared, the function can be reused to simplify code. Additionally this resolves https://bugzilla.redhat.com/show_bug.cgi?id=1244128 since the cpu bandwidth limiting with cgroups would not be set on the hotplug path. Additionally the code now sets the thread affinity and honors autoCpuset as in the regular startup code path. --- src/qemu/qemu_driver.c | 35 +---------------------------------- 1 file changed, 1 insertion(+), 34 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6f76316..52ed0c9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4743,10 +4743,6 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, int ret = -1; int rc; int oldvcpus = virDomainDefGetVcpus(vm->def); - virCgroupPtr cgroup_vcpu = NULL; - char *mem_mask = NULL; - virDomainNumatuneMemMode mem_mode; - pid_t vcpupid; if (!(vcpuinfo = virDomainDefGetVcpu(vm->def, vcpu))) return -1; @@ -4779,41 +4775,12 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, goto cleanup; } - vcpupid = qemuDomainGetVcpuPid(vm, vcpu); - - 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 (priv->cgroup) { - cgroup_vcpu = qemuDomainAddCgroupForThread(priv->cgroup, - VIR_CGROUP_THREAD_VCPU, - vcpu, mem_mask, vcpupid); - if (!cgroup_vcpu) - goto cleanup; - } - - /* Inherit def->cpuset */ - if (vm->def->cpumask) { - if (qemuDomainHotplugPinThread(vm->def->cpumask, vcpu, vcpupid, - cgroup_vcpu) < 0) { - goto cleanup; - } - } - - if (vcpuinfo->sched.policy != VIR_PROC_POLICY_NONE && - virProcessSetScheduler(vcpupid, vcpuinfo->sched.policy, - vcpuinfo->sched.priority) < 0) + if (qemuProcessSetupVcpu(vm, vcpu) < 0) goto cleanup; ret = 0; cleanup: - VIR_FREE(mem_mask); - virCgroupFree(&cgroup_vcpu); return ret; } -- 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 | 164 +++++++++++++++++++++++++++++++++--------------- src/qemu/qemu_process.h | 2 + 4 files changed, 115 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 fc87d07..999c74b 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, @@ -4575,6 +4534,117 @@ 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 (cgroup_iothread) { + 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 (virCgroupAddTask(cgroup_iothread, iothread->thread_id) < 0) + goto cleanup; + } + + if (cpumask && virProcessSetAffinity(iothread->thread_id, cpumask) < 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. @@ -5042,16 +5112,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 7de9b89..ad80410 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -163,5 +163,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 52ed0c9..fe2f527 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
On Mon, Feb 08, 2016 at 12:26:29PM +0100, Peter Krempa wrote:
Yet another version since I've changed a few places other than requested in the review.
Peter Krempa (4): qemu: vcpu: Aggregate code to set vCPU tuning qemu: vcpu: Reuse qemuProcessSetupVcpu in vcpu hotplug qemu: iothread: Aggregate code to set IOThread tuning qemu: iothread: Reuse qemuProcessSetupIOThread in iothread hotplug
src/qemu/qemu_cgroup.c | 188 ------------------------ src/qemu/qemu_cgroup.h | 2 - src/qemu/qemu_driver.c | 136 +----------------- src/qemu/qemu_process.c | 376 +++++++++++++++++++++++++++++++++--------------- src/qemu/qemu_process.h | 6 + 5 files changed, 266 insertions(+), 442 deletions(-)
ACK series Jan
participants (2)
-
Ján Tomko -
Peter Krempa