On 01/11/2016 06:38 AM, Henning Schild wrote:
On Fri, 8 Jan 2016 11:05:59 -0500
John Ferlan <jferlan(a)redhat.com> wrote:
>
>>>
>>> I'm leaning towards something in the test. I'll check if reverting
>>> these changes alters the results. I don't imagine it will.
>>
>> The real question is which thread it fails on and at what point in
>> time. My patches only changed the order of operations where threads
>> enter the cpuset cgroups at a slightly different time. And the qemu
>> main thread never enters the parent group, it becomes an
>> emulator-thread. Maybe you can point to exactly the assertion that
>> fails. Including a link to the test code. And yes if you can
>> confirm that the patches are to blame that would be a good first
>> step ;).
>>
>> Thanks,
>> Henning
>>
>
> Update:
>
> I have found that if I revert patch 2...
>
> Then modify qemuInitCgroup() to modify the virCgroupNewMachine check
> to also ensure "|| !priv->cgroup)
I see the check for the parent cgroup should probably go back into
virCgroupNewMachine, including the cleanup stuff in case of failure.
Forgot to CC you (and Jan) on the 4 patch series I sent:
http://www.redhat.com/archives/libvir-list/2016-January/msg00236.html
Patches 2, 3, & 4 are related to above while patch 1 is for below.
John
> Then modify qemuSetupCgroupForEmulator() to make the
> virCgroupAddTask() call like was in patch 2
>
> Then modify patch 3 (qemuSetupCgroupForVcpu) to change the call:
>
>
> if (!cpumap)
> continue;
>
> if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) < 0)
> goto cleanup;
>
> to
>
> if (cpumap &&
> qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) < 0)
> goto cleanup;
>
Well that is not a syntactical change, maybe easier to read and in line
with the other places where qemuSetupCgroupCpusetCpus is called.
> Then retest and the test passes again.
>
> Note that taking this route, I found that when I start the guest, I
> have the following in 'tasks':
>
> # cat /sys/fs/cgroup/memory/machine.slice/tasks
> # cat /sys/fs/cgroup/memory/machine.slice/*/tasks
> 15007
> 15008
> 15010
> 15011
> 15013
> #
>
> Where '15007' is the virt-tests-vm1 process (eg, /proc/$pid/cgroup).
> If I read the intentions you had, this follows that...
>
> I'll post a couple of patches in a bit...
>
> John