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

Martin Kletzander mkletzan at redhat.com
Wed Jun 22 16:37:21 UTC 2016


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)
 {
-    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;
+    virDomainNumatuneMemMode mem_mode;
+    virCgroupPtr cgroup = NULL;
+    virBitmapPtr use_cpumask;
+    char *mem_mask = NULL;
     int ret = -1;

     if ((period || quota) &&
         !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                        _("cgroup cpu is required for scheduler tuning"));
-        return -1;
+        goto cleanup;
     }

-    if (vm->def->cputune.emulatorpin)
-        cpumask = vm->def->cputune.emulatorpin;
-    else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO &&
-             priv->autoCpuset)
-        cpumask = priv->autoCpuset;
+    /* Infer which cpumask shall be used. */
+    if (cpumask)
+        use_cpumask = cpumask;
+    else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)
+        use_cpumask = priv->autoCpuset;
     else
-        cpumask = vm->def->cpumask;
+        use_cpumask = vm->def->cpumask;

-    /* If CPU cgroup controller is not initialized here, then we need
+    /*
+     * If CPU cgroup controller is not initialized here, then we need
      * neither period nor quota settings.  And if CPUSET controller is
-     * not initialized either, then there's nothing to do anyway. */
+     * not initialized either, then there's nothing to do anyway.
+     */
     if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) ||
         virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) {

-        if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
-                               true, &cgroup_emulator) < 0)
+        if (virDomainNumatuneGetMode(vm->def->numa, -1, &mem_mode) == 0 &&
+            mem_mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
+            virDomainNumatuneMaybeFormatNodeset(vm->def->numa,
+                                                priv->autoNodeset,
+                                                &mem_mask, -1) < 0)
             goto cleanup;

-        if (virCgroupAddTask(cgroup_emulator, vm->pid) < 0)
+        if (virCgroupNewThread(priv->cgroup, nameval, id, true, &cgroup) < 0)
             goto cleanup;

+        /* move the thread for vcpu to sub dir */
+        if (virCgroupAddTask(cgroup, pid) < 0)
+            goto cleanup;

-        if (cpumask) {
-            if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET) &&
-                qemuSetupCgroupCpusetCpus(cgroup_emulator, cpumask) < 0)
+        if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) {
+            if (use_cpumask &&
+                qemuSetupCgroupCpusetCpus(cgroup, use_cpumask) < 0)
                 goto cleanup;
-        }

-        if (period || quota) {
-            if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) &&
-                qemuSetupCgroupVcpuBW(cgroup_emulator, period,
-                                      quota) < 0)
+            /*
+             * Don't setup cpuset.mems for the emulator, they need to
+             * be set up after initialization in order for kvm
+             * allocations to succeed.
+             */
+            if (nameval != VIR_CGROUP_THREAD_EMULATOR &&
+                mem_mask && virCgroupSetCpusetMems(cgroup, mem_mask) < 0)
                 goto cleanup;
+
         }
+
+        if ((period || quota) &&
+            qemuSetupCgroupVcpuBW(cgroup, period, quota) < 0)
+            goto cleanup;
     }

-    if (cpumask &&
-        virProcessSetAffinity(vm->pid, cpumask) < 0)
+    /* Setup legacy affinity. */
+    if (use_cpumask && virProcessSetAffinity(pid, use_cpumask) < 0)
         goto cleanup;

-    ret = 0;
+    /* Set scheduler type and priority. */
+    if (sched_policy != VIR_PROC_POLICY_NONE &&
+        virProcessSetScheduler(pid, sched_policy, sched_priority) < 0)
+        goto cleanup;

+    ret = 0;
  cleanup:
-    if (cgroup_emulator) {
+    VIR_FREE(mem_mask);
+    if (cgroup) {
         if (ret < 0)
-            virCgroupRemove(cgroup_emulator);
-        virCgroupFree(&cgroup_emulator);
+            virCgroupRemove(cgroup);
+        virCgroupFree(&cgroup);
     }

     return ret;
@@ -2377,6 +2415,15 @@ qemuProcessSetupEmulator(virDomainObjPtr vm)


 static int
+qemuProcessSetupEmulator(virDomainObjPtr vm)
+{
+    return qemuProcessSetupPid(vm, vm->pid, VIR_CGROUP_THREAD_EMULATOR,
+                               0, vm->def->cputune.emulatorpin,
+                               VIR_PROC_POLICY_NONE, 0);
+}
+
+
+static int
 qemuProcessInitPasswords(virConnectPtr conn,
                          virQEMUDriverPtr driver,
                          virDomainObjPtr vm,
@@ -4574,80 +4621,10 @@ qemuProcessSetupVcpu(virDomainObjPtr vm,
 {
     pid_t vcpupid = qemuDomainGetVcpuPid(vm, vcpuid);
     virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(vm->def, vcpuid);
-    qemuDomainObjPrivatePtr priv = vm->privateData;
-    char *mem_mask = NULL;
-    virDomainNumatuneMemMode mem_mode;
-    unsigned long long period = vm->def->cputune.period;
-    long long quota = vm->def->cputune.quota;
-    virCgroupPtr cgroup_vcpu = NULL;
-    virBitmapPtr cpumask;
-    int ret = -1;
-
-    if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) ||
-        virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) {
-
-        if (virDomainNumatuneGetMode(vm->def->numa, -1, &mem_mode) == 0 &&
-            mem_mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
-            virDomainNumatuneMaybeFormatNodeset(vm->def->numa,
-                                                priv->autoNodeset,
-                                                &mem_mask, -1) < 0)
-            goto cleanup;
-
-        if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, vcpuid,
-                               true, &cgroup_vcpu) < 0)
-            goto cleanup;
-
-        if (period || quota) {
-            if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0)
-                goto cleanup;
-        }
-    }
-
-    /* infer which cpumask shall be used */
-    if (vcpu->cpumask)
-        cpumask = vcpu->cpumask;
-    else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)
-        cpumask = priv->autoCpuset;
-    else
-        cpumask = vm->def->cpumask;
-
-    /* setup cgroups */
-    if (cgroup_vcpu) {
-        if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) {
-            if (mem_mask && virCgroupSetCpusetMems(cgroup_vcpu, mem_mask) < 0)
-                goto cleanup;
-
-            if (cpumask && qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumask) < 0)
-                goto cleanup;
-        }
-
-        /* move the thread for vcpu to sub dir */
-        if (virCgroupAddTask(cgroup_vcpu, vcpupid) < 0)
-            goto cleanup;
-    }
-
-    /* setup legacy affinty */
-    if (cpumask && virProcessSetAffinity(vcpupid, cpumask) < 0)
-        goto cleanup;
-
-    /* set scheduler type and priority */
-    if (vcpu->sched.policy != VIR_PROC_POLICY_NONE) {
-        if (virProcessSetScheduler(vcpupid, vcpu->sched.policy,
-                                   vcpu->sched.priority) < 0)
-            goto cleanup;
-    }
-
-    ret = 0;
-
- cleanup:
-    VIR_FREE(mem_mask);
-    if (cgroup_vcpu) {
-        if (ret < 0)
-            virCgroupRemove(cgroup_vcpu);
-        virCgroupFree(&cgroup_vcpu);
-    }

-    return ret;
+    return qemuProcessSetupPid(vm, vcpupid, VIR_CGROUP_THREAD_VCPU,
+                               vcpuid, vcpu->cpumask,
+                               vcpu->sched.policy, vcpu->sched.priority);
 }


@@ -4700,98 +4677,17 @@ qemuProcessSetupVcpus(virDomainObjPtr vm)
 }


-/**
- * qemuProcessSetupIOThread:
- * @vm: domain object
- * @iothread: iothread data structure to set the data for
- *
- * This function sets resource properities (affinity, cgroups, scheduler) for a
- * IOThread. This function expects that the IOThread is online and the IOThread
- * pids were correctly detected at the point when it's called.
- *
- * Returns 0 on success, -1 on error.
- */
 int
 qemuProcessSetupIOThread(virDomainObjPtr vm,
                          virDomainIOThreadIDDefPtr iothread)
 {
-    qemuDomainObjPrivatePtr priv = vm->privateData;
-    unsigned long long period = vm->def->cputune.period;
-    long long quota = vm->def->cputune.quota;
-    virDomainNumatuneMemMode mem_mode;
-    char *mem_mask = NULL;
-    virCgroupPtr cgroup_iothread = NULL;
-    virBitmapPtr cpumask = NULL;
-    int ret = -1;
-
-    if ((period || quota) &&
-        !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("cgroup cpu is required for scheduler tuning"));
-        return -1;
-    }
-
-    if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) ||
-        virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) {
-        if (virDomainNumatuneGetMode(vm->def->numa, -1, &mem_mode) == 0 &&
-            mem_mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
-            virDomainNumatuneMaybeFormatNodeset(vm->def->numa,
-                                                priv->autoNodeset,
-                                                &mem_mask, -1) < 0)
-            goto cleanup;

-        if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD,
+    return qemuProcessSetupPid(vm, iothread->thread_id,
+                               VIR_CGROUP_THREAD_IOTHREAD,
                                iothread->iothread_id,
-                               true, &cgroup_iothread) < 0)
-            goto cleanup;
-    }
-
-    if (iothread->cpumask)
-        cpumask = iothread->cpumask;
-    else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)
-        cpumask = priv->autoCpuset;
-    else
-        cpumask = vm->def->cpumask;
-
-    if (period || quota) {
-        if (qemuSetupCgroupVcpuBW(cgroup_iothread, period, quota) < 0)
-            goto cleanup;
-    }
-
-    if (cgroup_iothread) {
-        if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) {
-            if (mem_mask &&
-                virCgroupSetCpusetMems(cgroup_iothread, mem_mask) < 0)
-                goto cleanup;
-
-            if (cpumask &&
-                qemuSetupCgroupCpusetCpus(cgroup_iothread, cpumask) < 0)
-                goto cleanup;
-        }
-
-        if (virCgroupAddTask(cgroup_iothread, iothread->thread_id) < 0)
-            goto cleanup;
-    }
-
-    if (cpumask && virProcessSetAffinity(iothread->thread_id, cpumask) < 0)
-        goto cleanup;
-
-    if (iothread->sched.policy != VIR_PROC_POLICY_NONE &&
-        virProcessSetScheduler(iothread->thread_id, iothread->sched.policy,
-                               iothread->sched.priority) < 0)
-        goto cleanup;
-
-    ret = 0;
-
- cleanup:
-    if (cgroup_iothread) {
-        if (ret < 0)
-            virCgroupRemove(cgroup_iothread);
-        virCgroupFree(&cgroup_iothread);
-    }
-
-    VIR_FREE(mem_mask);
-    return ret;
+                               iothread->cpumask,
+                               iothread->sched.policy,
+                               iothread->sched.priority);
 }


@@ -5292,7 +5188,7 @@ qemuProcessLaunch(virConnectPtr conn,
     if (!qemuProcessVerifyGuestCPU(driver, vm, asyncJob))
         goto cleanup;

-    VIR_DEBUG("Setting up post-init cgroup restrictions");
+    VIR_DEBUG("Setting Post-init cgroup resctrictions");
     if (qemuSetupCpusetMems(vm) < 0)
         goto cleanup;

-- 
2.9.0




More information about the libvir-list mailing list