Re: [libvirt] [PATCH 2/3] qemu: do not put a task into machine cgroup
On 11/13/2015 11:57 AM, Henning Schild wrote:
The machine cgroup is a superset, a parent to the emulator and vcpuX
cgroups. The parent cgroup should never have any tasks directly in it.
In fact the parent cpuset might contain way more cpus than the sum of
emulatorpin and vcpupins. So putting tasks in the superset will allow
them to run outside of <cputune>.
Signed-off-by: Henning Schild <henning.schild(a)siemens.com>
---
src/qemu/qemu_cgroup.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 28d2ca2..2c74a22 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -769,12 +769,6 @@ qemuInitCgroup(virQEMUDriverPtr driver,
goto cleanup;
}
- if (virCgroupAddTask(priv->cgroup, vm->pid) < 0) {
- virCgroupRemove(priv->cgroup);
- virCgroupFree(&priv->cgroup);
- goto cleanup;
- }
-
Moving this to later would also seem to imply that the code after the
qemuSetupCgroup (which calls qemuInitCgroup) from qemuProcessLaunch
would need some movement too, e.g.:
/* This must be done after cgroup placement to avoid resetting CPU
* affinity */
if (!vm->def->cputune.emulatorpin &&
qemuProcessInitCpuAffinity(vm) < 0)
goto cleanup;
Theoretically that would then need to be between the following:
VIR_DEBUG("Setting cgroup for emulator (if required)");
if (qemuSetupCgroupForEmulator(vm) < 0)
goto cleanup;
<<<... right here, I believe ...>>>
VIR_DEBUG("Setting affinity of emulator threads");
if (qemuProcessSetEmulatorAffinity(vm) < 0)
goto cleanup;
Again, weak ACK - hopefully Peter/Martin can take a look. In any case a
v2 probably should be done.
John
done:
ret = 0;
cleanup:
@@ -1145,6 +1139,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;