Re: [libvirt] [PATCH 1/2] qemu: Add qemuProcessSetupPid()
On 06/22/2016 12:37 PM, Martin Kletzander wrote:
Setting up cgroups and other things for all kinds of threads (the
emulator thread, vCPU threads, I/O threads) was copy-pasted every time
new thing was added. Over time each one of those functions changed a
bit differently. So create one function that does all that setup and
start using it. That will shave some duplicated code and maybe fix some
bugs as well.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/qemu/qemu_process.c | 278 +++++++++++++++---------------------------------
1 file changed, 87 insertions(+), 191 deletions(-)
Would have been so much easier one at a time... The scroll wheel on my
mouse needs a break.
I think this looks so much more logical. The only other comment I have
is regarding the virCgroupAddTask call ordering...
If I'm reading right, prior to this set of changes the emulator code
would call that right after virCgroupNewThread and then make the
qemuSetupCgroupCpusetCpus and virCgroupHasController calls while vcpus
and iothreads would call virCgroupAddTask after all cgroup related
calls. I mention this mainly because I have a feint (or faint)
recollection of there being issues when regarding ordering of calls and
what gets copied when (details I really tried to forget).
Since emulator is the pid and vcpu/iothread are tid's I'm just being
overly cautious that there's a difference in ordering that isn't readily
apparent.
This looks reasonable to me, but hopefully someone else will chime in
regarding the order of virCgroupNewThread, virCgroupAddTask,
qemuSetupCgroupCpusetCpus, virCgroupSetCpusetMems, and
qemuSetupCgroupVcpuBW prior to calling virProcessSetAffinity
Consider it a fragile ACK -
John