[libvirt] [PATCH 2/3] qemu: do not put a task into machine cgroup

[Henning Schild]

On Tue, 8 Dec 2015 12:23:19 -0500
John Ferlan <jferlan at redhat.com> wrote:

> 
> 
> 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 at 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  ...>>>

Good catch! That code is confusing. I will try and merge
qemuProcessInitCpuAffinity with qemuProcessSetEmulatorAffinity.

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