On Fri, Jun 24, 2016 at 12:51:40PM +0200, Ján Tomko wrote:
On Wed, Jun 22, 2016 at 06:37:21PM +0200, 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(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 215fe5f2f210..d1247d2fd0f9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2306,70 +2306,108 @@ qemuProcessSetLinkStates(virQEMUDriverPtr driver, } +/** + * qemuProcessSetupPid: + * + * This function sets resource properities (affinity, cgroups, + * scheduler) for any PID associated with a domain. It should be used + * to set up emulator PIDs as well as vCPU and I/O thread pids to + * ensure they are all handled the same way. + * + * Returns 0 on success, -1 on error. + */ static int -qemuProcessSetupEmulator(virDomainObjPtr vm) +qemuProcessSetupPid(virDomainObjPtr vm, + pid_t pid, + virCgroupThreadName nameval, + int id, + virBitmapPtr cpumask,+ int sched_policy, + int sched_priority)You can just pass a pointer to the virDomainThreadSchedParam structure.{ - virBitmapPtr cpumask = NULL; - virCgroupPtr cgroup_emulator = NULL; qemuDomainObjPrivatePtr priv = vm->privateData;- unsigned long long period = vm->def->cputune.emulator_period; - long long quota = vm->def->cputune.emulator_quota; + unsigned long long period = vm->def->cputune.period; + long long quota = vm->def->cputune.quota;These two are not equivalent, we even have separate XML elements for them: <cputune> <period>1000000</period> <quota>-1</quota> <emulator_period>1000000</emulator_period> <emulator_quota>-1</emulator_quota> </cputune> Also, renaming the variables and reordering the code in the respective function first would make this fragile code easier to review.
After the fiasco with _some_ other patch, I'll rather send a v2...
Jan
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list