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&gt; > --- > 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; > >