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@xxxxxxxxxx> > --- > 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list