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

[John Ferlan]

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

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