10:34 a.m.
On 09/04/2014 03:52 AM, Wang Rui wrote:
From: Yue Wenyuan <yuewenyuan(a)huawei.com>
This patch implements libvirt_lxc process pin with emulatorpin
specified in xml.
Signed-off-by: Wang Rui <moon.wangrui(a)huawei.com>
Signed-off-by: Yue Wenyuan <yuewenyuan(a)huawei.com>
---
src/lxc/lxc_cgroup.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
src/lxc/lxc_cgroup.h | 4 +++
src/lxc/lxc_controller.c | 4 +++
3 files changed, 76 insertions(+)
I'm not an LXC expert, but I'll give this a go since it's been sitting
around a while.
I am curious why only the CPUSET (eg, <vcpuset ... cpuset="1-4,^3,6"> or
<emulatorpin cpuset="1-3"/>) were handled and the <emulator_period>
and
<emulator_quota> were not?
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index f9af31c..f696bf8 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -530,3 +530,71 @@ int virLXCCgroupSetup(virDomainDefPtr def,
cleanup:
return ret;
}
+
+static int virLXCCgroupSetupCpusetTuneForEmulator(virDomainDefPtr def,
Change name virLXCCgroupSetupCpusetTuneForEmulator to
virLXCSetupCgroupCpusetTunesForEmulator [1]
note the (s) on Tune as well... Could have also gone with
"CpusetCpusMems", but that really seemed too long
+ virCgroupPtr cgroup,
+ virBitmapPtr nodemask)
Although lxc_cgroup doesn't do it for the majority of the functions use:
static int
virLXCSetupCgroupEmulatorCpusetCpusMems(...)
make sure to align arguments properly too.
+{
+ int ret = -1;
+ char *mask = NULL;
+
+ if (def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) {
+ if (def->cputune.emulatorpin) {
+ if (!(mask = virBitmapFormat(def->cputune.emulatorpin->cpumask)))
+ return ret;
+ } else if (def->cpumask) {
+ if (!(mask = virBitmapFormat(def->cpumask)))
+ return ret;
+ }
At this point mask could still be NULL, thus you need a "if (mask && "
prior to call...
+ if (virCgroupSetCpusetCpus(cgroup, mask) < 0)
+ goto cleanup;
+ }
And if MODE_AUTO, then does virLXCControllerSetupCpuAffinity called
prior to this function satisfy the doc requirement to query numad? If
so, then perhaps noting that as a comment prior to this hunk of code
would be beneficial. If not, then that needs to be handled. It doesn't
seem that way since I don't see comparable code in LXC to the
qemuPrepareCpumap code that handles the host.numaCell.
Also since you're reusing 'mask' below you'll need a VIR_FREE(mask);
prior to the next set of calls; otherwise, you'll leak it.
+
+ if (virDomainNumatuneMaybeFormatNodeset(def->numatune, nodemask,
+ &mask, -1) < 0)
+ goto cleanup;
+
+ if (mask && virCgroupSetCpusetMems(cgroup, mask) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(mask);
+ return ret;
+}
+
+int virLXCCgroupSetupForEmulator(virDomainDefPtr def,
+ virCgroupPtr cgroup,
+ virBitmapPtr nodemask)
The qemu function is qemuSetupCgroupForEmulator, thus this should be
virLXCSetupCgroupForEmulator. [2]
The function arguments need to be aligned and like above:
int
virLXCSetupCgroupForEmulator(...)
+{
+ virCgroupPtr cgroup_emulator = NULL;
+
+ if (!virCgroupHasController(cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
+ return 0;
+
+ if (cgroup == NULL)
+ return 0; /* Not supported, so claim success */
This seems to be superfluous as the virCgroupHasController() will return
false if (!cgroup), although I do see that it's a cut-n-paste of the
similar qemu code.
+
+ if (virCgroupNewEmulator(cgroup, true, &cgroup_emulator) < 0)
+ goto error;
+
+ if (virCgroupMoveTask(cgroup, cgroup_emulator) < 0)
+ goto error;
+
+ if (virCgroupHasController(cgroup_emulator, VIR_CGROUP_CONTROLLER_CPUSET) &&
^^^^^^^^^^^^^^^
qemu code uses priv->cgroup, so should this code use 'cgroup' instead?
or is the qemu code incorrect and should be fixed?
+ virLXCCgroupSetupCpusetTuneForEmulator(def, cgroup_emulator,
nodemask) < 0)
[1] Use new name...
+ goto error;
+
+ virCgroupFree(&cgroup_emulator);
+ return 0;
+
+ error:
+
+ if (cgroup_emulator) {
+ virCgroupRemove(cgroup_emulator);
+ virCgroupFree(&cgroup_emulator);
+ }
+
+ return -1;
+}
diff --git a/src/lxc/lxc_cgroup.h b/src/lxc/lxc_cgroup.h
index 0e78126..32086c5 100644
--- a/src/lxc/lxc_cgroup.h
+++ b/src/lxc/lxc_cgroup.h
@@ -33,6 +33,10 @@ int virLXCCgroupSetup(virDomainDefPtr def,
virCgroupPtr cgroup,
virBitmapPtr nodemask);
+int virLXCCgroupSetupForEmulator(virDomainDefPtr def,
+ virCgroupPtr cgroup,
+ virBitmapPtr nodemask);
+
[2] See note above about name change s/CgroupSetup/SetupCgroup
Also the argument alignment is off - yes I see the previous function
prototype is too, but lets at least do this one right and fix the other
one later in either a follow up or separate patch prior to this patch.
int virLXCCgroupGetMeminfo(virLXCMeminfoPtr meminfo);
int
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 1861dd6..1a62e20 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -698,6 +698,10 @@ static int virLXCControllerSetupResourceLimits(virLXCControllerPtr
ctrl)
if (virLXCCgroupSetup(ctrl->def, ctrl->cgroup, nodemask) < 0)
goto cleanup;
+ VIR_DEBUG("Setting cgroup for lxc emulator");
+ if (virLXCCgroupSetupForEmulator(ctrl->def, ctrl->cgroup, nodemask) < 0)
[2] See note above about name change s/CgroupSetup/SetupCgroup
+ goto cleanup;
+
ret = 0;
cleanup:
virBitmapFree(nodemask);
You need to update docs/formatdomain.html.in as well to add the LXC
Since 1.2.x (whenever this gets in) to various bits and parts of the
<cputune> and <vcpus> descriptions.
John