Re: [libvirt] [PATCH 1/2] qemu: process: Move emulator thread setting code into one function

Similarly to the refactors to iothreads and vcpus, move the code that initializes the emulator thread settings into single function. --- src/qemu/qemu_cgroup.c | 66 ----------------------------------------- src/qemu/qemu_cgroup.h | 1 - src/qemu/qemu_process.c | 78 +++++++++++++++++++++++++++++++++++++++---------- 3 files changed, 62 insertions(+), 83 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index f3c5fbb..a294bb0 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1062,72 +1062,6 @@ qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup, int -qemuSetupCgroupForEmulator(virDomainObjPtr vm) -{ - virBitmapPtr cpumask = NULL; - virCgroupPtr cgroup_emulator = NULL; - virDomainDefPtr def = vm->def; - qemuDomainObjPrivatePtr priv = vm->privateData; - unsigned long long period = vm->def->cputune.emulator_period; - long long quota = vm->def->cputune.emulator_quota; - - 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 (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, - true, &cgroup_emulator) < 0) - goto cleanup; - - if (virCgroupMoveTask(priv->cgroup, cgroup_emulator) < 0) - goto cleanup; - - if (def->cputune.emulatorpin) - cpumask = def->cputune.emulatorpin; - else if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) - cpumask = priv->autoCpuset; - else if (def->cpumask) - cpumask = def->cpumask; - - if (cpumask) { - if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET) && - qemuSetupCgroupCpusetCpus(cgroup_emulator, cpumask) < 0) - goto cleanup; - } - - if (period || quota) { - if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) && - qemuSetupCgroupVcpuBW(cgroup_emulator, period, - quota) < 0) - goto cleanup; - } - - virCgroupFree(&cgroup_emulator); - return 0; - - cleanup: - if (cgroup_emulator) { - virCgroupRemove(cgroup_emulator); - virCgroupFree(&cgroup_emulator); - } - - return -1; -} - - -int qemuRemoveCgroup(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index a8b8e1b..7a9d10a 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -54,7 +54,6 @@ int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, unsigned long long period, long long quota); int qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup, virBitmapPtr cpumask); -int qemuSetupCgroupForEmulator(virDomainObjPtr vm); int qemuRemoveCgroup(virDomainObjPtr vm); #endif /* __QEMU_CGROUP_H__ */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e760182..07b7d63 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2185,22 +2185,72 @@ qemuProcessSetLinkStates(virQEMUDriverPtr driver, } -/* Set CPU affinities for emulator threads. */ static int -qemuProcessSetEmulatorAffinity(virDomainObjPtr vm) +qemuProcessSetupEmulator(virDomainObjPtr vm) { - virBitmapPtr cpumask; - virDomainDefPtr def = vm->def; + 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; int ret = -1; - if (def->cputune.emulatorpin) - cpumask = def->cputune.emulatorpin; - else if (def->cpumask) - cpumask = def->cpumask; + 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 (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; else - return 0; + cpumask = vm->def->cpumask; + + /* 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)) { + + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, + true, &cgroup_emulator) < 0) + goto cleanup; + + if (virCgroupMoveTask(priv->cgroup, cgroup_emulator) < 0) + goto cleanup; + + + if (cpumask) { + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET) && + qemuSetupCgroupCpusetCpus(cgroup_emulator, cpumask) < 0) + goto cleanup; + } + + if (period || quota) { + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) && + qemuSetupCgroupVcpuBW(cgroup_emulator, period, + quota) < 0) + goto cleanup; + } + } + + if (cpumask && + virProcessSetAffinity(vm->pid, cpumask) < 0) + goto cleanup; + + ret = 0; + + cleanup: + if (cgroup_emulator) { + if (ret < 0) + virCgroupRemove(cgroup_emulator); + virCgroupFree(&cgroup_emulator); + } - ret = virProcessSetAffinity(vm->pid, cpumask); return ret; } @@ -5135,12 +5185,8 @@ qemuProcessLaunch(virConnectPtr conn, if (rv == -1) /* The VM failed to start */ goto cleanup; - VIR_DEBUG("Setting cgroup for emulator (if required)"); - if (qemuSetupCgroupForEmulator(vm) < 0) - goto cleanup; - - VIR_DEBUG("Setting affinity of emulator threads"); - if (qemuProcessSetEmulatorAffinity(vm) < 0) + VIR_DEBUG("Setting emulator tuning/settings"); + if (qemuProcessSetupEmulator(vm) < 0)

goto cleanup; VIR_DEBUG("Waiting for monitor to show up");