10:48 a.m.
The problem being resolved by this series was seen as a failure in the
virt-test 'memtune' test which attempts to look at the on disk change
for the cgroup by using the 'memory' controller entry in /proc/$pid/cgroup.
For reasons still not quite 100% crystal clear, without doing the revert
in patch#3 of this series, the /proc/$pid/cgroup file gets updated with
the controller path. On the somewhat older f20 test system, this resulted
in an adjustment to other controllers (memory, devices, and blkio). On
a different test environment (f23), the file wasn't erroneously updated.
Although systemd may ultimately be at fault - it just wasn't obvious
what the failure was - so reverting the patch and making the one change
(in patch #4) that appeared to at least follow the spirit of the
reverted patch with respect to using virCgroupAddTask for the
qemuSetupCgroupForEmulator similar to how qemuSetupCgroupForVcpu
does it for the various vpcu threads.
John Ferlan (4):
cgroup: Fix possible bug as a result of code motion for vcpu cgroup
setup
qemu: Add check for NULL cgroup return from virCgroupNewMachine
Revert "qemu: do not put a task into machine cgroup"
qemu: Put the emulator cgroup pid into the right task file
src/qemu/qemu_cgroup.c | 18 +++++++++++++-----
src/qemu/qemu_process.c | 12 ++++++------
2 files changed, 19 insertions(+), 11 deletions(-)
--
2.5.0
10:48 a.m.
New subject: [libvirt] [PATCH 1/4] cgroup: Fix possible bug as a result of code motion for vcpu cgroup setup
Commit id '90b721e43' moved where the virCgroupAddTask was made until
after the check for the vcpupin checks. However, in doing so it missed
an option where if the cpumap didn't exist, then the code would continue
back to the top of the current vcpu loop. The results was that the
virCgroupAddTask wouldn't be called.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_cgroup.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 1c406ce..91b3328 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -1079,10 +1079,7 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm)
}
}
- if (!cpumap)
- continue;
-
- if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) < 0)
+ if (cpumap && qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) < 0)
goto cleanup;
}
--
2.5.0
10:48 a.m.
New subject: [libvirt] [PATCH 2/4] qemu: Add check for NULL cgroup return from virCgroupNewMachine
Commit id '71ce4759' altered the cgroup processing with respect to the
call to virCgroupAddTask being moved out from lower layers into the calling
layers especially for qemu processing of emulator and vcpu threads.
What was missed in the processing was that it is possible for a code path to
return a NULL cgroup *and* a 0 return status via virCgroupNewPartition
failure when virCgroupNewIgnoreError succeeded when virCgroupNewMachineManual
returns. This was initially seen for lxc cgroup processing and patched
by comit id 'ae09988eb'. We'll make the same check here as a subsquent
patch will revert commit id 'a41c00b47' which would leave the possibility
that priv->cgroup is NULL.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_cgroup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 91b3328..515d76a 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -782,7 +782,7 @@ qemuInitCgroup(virQEMUDriverPtr driver,
nnicindexes, nicindexes,
vm->def->resource->partition,
cfg->cgroupControllers,
- &priv->cgroup) < 0) {
+ &priv->cgroup) < 0 || !priv->cgroup) {
if (virCgroupNewIgnoreError())
goto done;
--
2.5.0
10:48 a.m.
New subject: [libvirt] [PATCH 3/4] Revert "qemu: do not put a task into machine cgroup"
This reverts commit a41c00b472efaa192d2deae51ab732e65903238f.
The patch was causing erroneous updates to the /proc/$pid/cgroup file.
This resulted in some unexpected behavoirs.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_cgroup.c | 15 +++++++++++----
src/qemu/qemu_process.c | 12 ++++++------
2 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 515d76a..16c6492 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -789,6 +789,17 @@ qemuInitCgroup(virQEMUDriverPtr driver,
goto cleanup;
}
+ if (virCgroupAddTask(priv->cgroup, vm->pid) < 0) {
+ virErrorPtr saved = virSaveLastError();
+ virCgroupRemove(priv->cgroup);
+ virCgroupFree(&priv->cgroup);
+ if (saved) {
+ virSetError(saved);
+ virFreeError(saved);
+ }
+ goto cleanup;
+ }
+
done:
ret = 0;
cleanup:
@@ -1157,10 +1168,6 @@ qemuSetupCgroupForEmulator(virDomainObjPtr vm)
goto cleanup;
}
- /* consider the first thread an emulator-thread */
- if (virCgroupAddTask(cgroup_emulator, vm->pid) < 0)
- goto cleanup;
-
virCgroupFree(&cgroup_emulator);
return 0;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 819ad05..8849d15 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4894,6 +4894,12 @@ qemuProcessLaunch(virConnectPtr conn,
if (qemuSetupCgroup(driver, vm, nnicindexes, nicindexes) < 0)
goto cleanup;
+ /* This must be done after cgroup placement to avoid resetting CPU
+ * affinity */
+ if (!vm->def->cputune.emulatorpin &&
+ qemuProcessInitCpuAffinity(vm) < 0)
+ goto cleanup;
+
VIR_DEBUG("Setting domain security labels");
if (virSecurityManagerSetAllLabel(driver->securityManager,
vm->def,
@@ -4940,12 +4946,6 @@ qemuProcessLaunch(virConnectPtr conn,
if (qemuSetupCgroupForEmulator(vm) < 0)
goto cleanup;
- /* This must be done after cgroup placement to avoid resetting CPU
- * affinity */
- if (!vm->def->cputune.emulatorpin &&
- qemuProcessInitCpuAffinity(vm) < 0)
- goto cleanup;
-
VIR_DEBUG("Setting affinity of emulator threads");
if (qemuProcessSetEmulatorAffinity(vm) < 0)
goto cleanup;
--
2.5.0
10:48 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: Put the emulator cgroup pid into the right task file
Recently reverted commit id 'a41c00b4' was designed to move the setting
of the task file into the right place in the cgroup hierarchy. This patch
applies the portion of the reverted patch which writes the pid to the
right task file.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_cgroup.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 16c6492..a0ad03f 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -1168,6 +1168,10 @@ qemuSetupCgroupForEmulator(virDomainObjPtr vm)
goto cleanup;
}
+ /* consider the first thread an emulator-thread */
+ if (virCgroupAddTask(cgroup_emulator, vm->pid) < 0)
+ goto cleanup;
+
virCgroupFree(&cgroup_emulator);
return 0;
--
2.5.0
3243
days inactive
3243
days old
4 comments
1 participants
participants (1)
-
John Ferlan