[libvirt] [PATCHv2 2/3] qemu_cgroup: put qemu right into emulator sub-cgroup

[Henning Schild]

On Tue, 1 Mar 2016 12:34:44 +0100
Peter Krempa <pkrempa at redhat.com> wrote:

> On Tue, Mar 01, 2016 at 11:20:18 +0000, Daniel Berrange wrote:
> > On Fri, Feb 26, 2016 at 04:34:23PM +0100, Henning Schild wrote:  
> > > Move qemuProcessSetupEmulator up under qemuSetupCgroup. That way
> > > we move the one main thread right into the emulator cgroup,
> > > instead of moving multiple threads later on. And we do not
> > > actually want any threads running in the parent cgroups (cpu
> > > cpuacct cpuset).
> > > 
> > > Signed-off-by: Henning Schild <henning.schild at siemens.com>
> > > ---
> > >  src/qemu/qemu_process.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> > > index 0c43183..7725a5f 100644
> > > --- a/src/qemu/qemu_process.c
> > > +++ b/src/qemu/qemu_process.c
> > > @@ -5087,6 +5087,10 @@ qemuProcessLaunch(virConnectPtr conn,
> > >          qemuProcessInitCpuAffinity(vm) < 0)
> > >          goto cleanup;
> > >  
> > > +    VIR_DEBUG("Setting emulator tuning/settings");
> > > +    if (qemuProcessSetupEmulator(vm) < 0)
> > > +        goto cleanup;
> > > +
> > >      VIR_DEBUG("Setting domain security labels");
> > >      if (virSecurityManagerSetAllLabel(driver->securityManager,
> > >                                        vm->def,
> > > @@ -5129,10 +5133,6 @@ qemuProcessLaunch(virConnectPtr conn,
> > >      if (rv == -1) /* The VM failed to start */
> > >          goto cleanup;
> > >  
> > > -    VIR_DEBUG("Setting emulator tuning/settings");
> > > -    if (qemuProcessSetupEmulator(vm) < 0)
> > > -        goto cleanup;
> > > -
> > >      VIR_DEBUG("Waiting for monitor to show up");
> > >      if (qemuProcessWaitForMonitor(driver, vm, asyncJob,
> > > priv->qemuCaps, logCtxt) < 0) goto cleanup;  
> > 
> > Do you have some other local patches applied to your git ?  I just
> > went to apply this and realized that qemuProcessSetupEmulator()
> > does not actually exist. It git master the function is
> > qemuSetupCgroupForEmulator and there is another function call
> > qemuProcessSetEmulatorAffinity just after it too.  So I can't apply
> > this patch  
> 
> This was based on top of my refactor that creates
> qemuProcessSetupEmulator. I didn't realize Henning based that on top
> of my patch. I wanted to wait until this gets sorted and then re-do my
> patch, but I can push it so that you can apply that.

Peter could you please look into the affinity setting code of qemu
after my patch 2?
In the cgroups affinity setting code the main threads (qemuProcess) is
considered an emulator-thread right away. The manual affinty setting
code should apply the same scheme.
I think qemuProcessInitCpuAffinity is obsolte. If cornercases remain
they should become part of qemuProcessSetupEmulator.
Depending on how qemuProcessSetupEmulator has to change we will see
about the patch ordering later.

> Peter