[libvirt] [PATCH 0/4] Adjustment to recent cgroup/cpuset changes
The problem being resolved by this series was seen as a failure in the virt-test 'memtune' test which attempts to look at the on disk change for the cgroup by using the 'memory' controller entry in /proc/$pid/cgroup. For reasons still not quite 100% crystal clear, without doing the revert in patch#3 of this series, the /proc/$pid/cgroup file gets updated with the controller path. On the somewhat older f20 test system, this resulted in an adjustment to other controllers (memory, devices, and blkio). On a different test environment (f23), the file wasn't erroneously updated. Although systemd may ultimately be at fault - it just wasn't obvious what the failure was - so reverting the patch and making the one change (in patch #4) that appeared to at least follow the spirit of the reverted patch with respect to using virCgroupAddTask for the qemuSetupCgroupForEmulator similar to how qemuSetupCgroupForVcpu does it for the various vpcu threads. John Ferlan (4): cgroup: Fix possible bug as a result of code motion for vcpu cgroup setup qemu: Add check for NULL cgroup return from virCgroupNewMachine Revert "qemu: do not put a task into machine cgroup" qemu: Put the emulator cgroup pid into the right task file src/qemu/qemu_cgroup.c | 18 +++++++++++++----- src/qemu/qemu_process.c | 12 ++++++------ 2 files changed, 19 insertions(+), 11 deletions(-) -- 2.5.0
Commit id '90b721e43' moved where the virCgroupAddTask was made until after the check for the vcpupin checks. However, in doing so it missed an option where if the cpumap didn't exist, then the code would continue back to the top of the current vcpu loop. The results was that the virCgroupAddTask wouldn't be called. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_cgroup.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 1c406ce..91b3328 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1079,10 +1079,7 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) } } - if (!cpumap) - continue; - - if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) < 0) + if (cpumap && qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) < 0) goto cleanup; } -- 2.5.0
Commit id '71ce4759' altered the cgroup processing with respect to the call to virCgroupAddTask being moved out from lower layers into the calling layers especially for qemu processing of emulator and vcpu threads. What was missed in the processing was that it is possible for a code path to return a NULL cgroup *and* a 0 return status via virCgroupNewPartition failure when virCgroupNewIgnoreError succeeded when virCgroupNewMachineManual returns. This was initially seen for lxc cgroup processing and patched by comit id 'ae09988eb'. We'll make the same check here as a subsquent patch will revert commit id 'a41c00b47' which would leave the possibility that priv->cgroup is NULL. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_cgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 91b3328..515d76a 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -782,7 +782,7 @@ qemuInitCgroup(virQEMUDriverPtr driver, nnicindexes, nicindexes, vm->def->resource->partition, cfg->cgroupControllers, - &priv->cgroup) < 0) { + &priv->cgroup) < 0 || !priv->cgroup) { if (virCgroupNewIgnoreError()) goto done; -- 2.5.0
This reverts commit a41c00b472efaa192d2deae51ab732e65903238f. The patch was causing erroneous updates to the /proc/$pid/cgroup file. This resulted in some unexpected behavoirs. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_cgroup.c | 15 +++++++++++---- src/qemu/qemu_process.c | 12 ++++++------ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 515d76a..16c6492 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -789,6 +789,17 @@ qemuInitCgroup(virQEMUDriverPtr driver, goto cleanup; } + if (virCgroupAddTask(priv->cgroup, vm->pid) < 0) { + virErrorPtr saved = virSaveLastError(); + virCgroupRemove(priv->cgroup); + virCgroupFree(&priv->cgroup); + if (saved) { + virSetError(saved); + virFreeError(saved); + } + goto cleanup; + } + done: ret = 0; cleanup: @@ -1157,10 +1168,6 @@ qemuSetupCgroupForEmulator(virDomainObjPtr vm) goto cleanup; } - /* consider the first thread an emulator-thread */ - if (virCgroupAddTask(cgroup_emulator, vm->pid) < 0) - goto cleanup; - virCgroupFree(&cgroup_emulator); return 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 819ad05..8849d15 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4894,6 +4894,12 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuSetupCgroup(driver, vm, nnicindexes, nicindexes) < 0) goto cleanup; + /* This must be done after cgroup placement to avoid resetting CPU + * affinity */ + if (!vm->def->cputune.emulatorpin && + qemuProcessInitCpuAffinity(vm) < 0) + goto cleanup; + VIR_DEBUG("Setting domain security labels"); if (virSecurityManagerSetAllLabel(driver->securityManager, vm->def, @@ -4940,12 +4946,6 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuSetupCgroupForEmulator(vm) < 0) goto cleanup; - /* This must be done after cgroup placement to avoid resetting CPU - * affinity */ - if (!vm->def->cputune.emulatorpin && - qemuProcessInitCpuAffinity(vm) < 0) - goto cleanup; - VIR_DEBUG("Setting affinity of emulator threads"); if (qemuProcessSetEmulatorAffinity(vm) < 0) goto cleanup; -- 2.5.0
Recently reverted commit id 'a41c00b4' was designed to move the setting of the task file into the right place in the cgroup hierarchy. This patch applies the portion of the reverted patch which writes the pid to the right task file. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_cgroup.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 16c6492..a0ad03f 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1168,6 +1168,10 @@ qemuSetupCgroupForEmulator(virDomainObjPtr vm) goto cleanup; } + /* consider the first thread an emulator-thread */ + if (virCgroupAddTask(cgroup_emulator, vm->pid) < 0) + goto cleanup; + virCgroupFree(&cgroup_emulator); return 0; -- 2.5.0
participants (1)
-
John Ferlan