Re: [PATCH 1/2] qemu: Add qemuProcessSetupPid()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]