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

Ján Tomko jtomko at redhat.com
Fri Jun 24 10:51:40 UTC 2016


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 at redhat.com>
>---
> 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.

Jan




More information about the libvir-list mailing list