[libvirt] [PATCH 1/2] qemu: Add qemuProcessSetupPid()

Setting up cgroups and other things for all kinds of threads (the emulator thread, vCPU threads, I/O threads) was copy-pasted every time new thing was added. Over time each one of those functions changed a bit differently. So create one function that does all that setup and start using it. That will shave some duplicated code and maybe fix some bugs as well. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/qemu/qemu_process.c | 278 +++++++++++++++--------------------------------- 1 file changed, 87 insertions(+), 191 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 215fe5f2f210..d1247d2fd0f9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2306,70 +2306,108 @@ qemuProcessSetLinkStates(virQEMUDriverPtr driver, } +/** + * qemuProcessSetupPid: + * + * This function sets resource properities (affinity, cgroups, + * scheduler) for any PID associated with a domain. It should be used + * to set up emulator PIDs as well as vCPU and I/O thread pids to + * ensure they are all handled the same way. + * + * Returns 0 on success, -1 on error. + */ static int -qemuProcessSetupEmulator(virDomainObjPtr vm) +qemuProcessSetupPid(virDomainObjPtr vm, + pid_t pid, + virCgroupThreadName nameval, + int id, + virBitmapPtr cpumask, + int sched_policy, + int sched_priority) { - virBitmapPtr cpumask = NULL; - virCgroupPtr cgroup_emulator = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; - unsigned long long period = vm->def->cputune.emulator_period; - long long quota = vm->def->cputune.emulator_quota; + unsigned long long period = vm->def->cputune.period; + long long quota = vm->def->cputune.quota; + virDomainNumatuneMemMode mem_mode; + virCgroupPtr cgroup = NULL; + virBitmapPtr use_cpumask; + char *mem_mask = 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; + goto cleanup; } - if (vm->def->cputune.emulatorpin) - cpumask = vm->def->cputune.emulatorpin; - else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO && - priv->autoCpuset) - cpumask = priv->autoCpuset; + /* Infer which cpumask shall be used. */ + if (cpumask) + use_cpumask = cpumask; + else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) + use_cpumask = priv->autoCpuset; else - cpumask = vm->def->cpumask; + use_cpumask = vm->def->cpumask; - /* If CPU cgroup controller is not initialized here, then we need + /* + * 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. */ + * 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)) { - if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, - true, &cgroup_emulator) < 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; - if (virCgroupAddTask(cgroup_emulator, vm->pid) < 0) + if (virCgroupNewThread(priv->cgroup, nameval, id, true, &cgroup) < 0) goto cleanup; + /* move the thread for vcpu to sub dir */ + if (virCgroupAddTask(cgroup, pid) < 0) + goto cleanup; - if (cpumask) { - if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET) && - qemuSetupCgroupCpusetCpus(cgroup_emulator, cpumask) < 0) + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { + if (use_cpumask && + qemuSetupCgroupCpusetCpus(cgroup, use_cpumask) < 0) goto cleanup; - } - if (period || quota) { - if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) && - qemuSetupCgroupVcpuBW(cgroup_emulator, period, - quota) < 0) + /* + * Don't setup cpuset.mems for the emulator, they need to + * be set up after initialization in order for kvm + * allocations to succeed. + */ + if (nameval != VIR_CGROUP_THREAD_EMULATOR && + mem_mask && virCgroupSetCpusetMems(cgroup, mem_mask) < 0) goto cleanup; + } + + if ((period || quota) && + qemuSetupCgroupVcpuBW(cgroup, period, quota) < 0) + goto cleanup; } - if (cpumask && - virProcessSetAffinity(vm->pid, cpumask) < 0) + /* Setup legacy affinity. */ + if (use_cpumask && virProcessSetAffinity(pid, use_cpumask) < 0) goto cleanup; - ret = 0; + /* Set scheduler type and priority. */ + if (sched_policy != VIR_PROC_POLICY_NONE && + virProcessSetScheduler(pid, sched_policy, sched_priority) < 0) + goto cleanup; + ret = 0; cleanup: - if (cgroup_emulator) { + VIR_FREE(mem_mask); + if (cgroup) { if (ret < 0) - virCgroupRemove(cgroup_emulator); - virCgroupFree(&cgroup_emulator); + virCgroupRemove(cgroup); + virCgroupFree(&cgroup); } return ret; @@ -2377,6 +2415,15 @@ qemuProcessSetupEmulator(virDomainObjPtr vm) static int +qemuProcessSetupEmulator(virDomainObjPtr vm) +{ + return qemuProcessSetupPid(vm, vm->pid, VIR_CGROUP_THREAD_EMULATOR, + 0, vm->def->cputune.emulatorpin, + VIR_PROC_POLICY_NONE, 0); +} + + +static int qemuProcessInitPasswords(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4574,80 +4621,10 @@ qemuProcessSetupVcpu(virDomainObjPtr vm, { 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 (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; + return qemuProcessSetupPid(vm, vcpupid, VIR_CGROUP_THREAD_VCPU, + vcpuid, vcpu->cpumask, + vcpu->sched.policy, vcpu->sched.priority); } @@ -4700,98 +4677,17 @@ 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, + return qemuProcessSetupPid(vm, iothread->thread_id, + 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; + iothread->cpumask, + iothread->sched.policy, + iothread->sched.priority); } @@ -5292,7 +5188,7 @@ qemuProcessLaunch(virConnectPtr conn, if (!qemuProcessVerifyGuestCPU(driver, vm, asyncJob)) goto cleanup; - VIR_DEBUG("Setting up post-init cgroup restrictions"); + VIR_DEBUG("Setting Post-init cgroup resctrictions"); if (qemuSetupCpusetMems(vm) < 0) goto cleanup; -- 2.9.0