On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Systemd uses a named cgroup mount for tracking processes. Add it as another type of controller, albeit one which we have to special case in a number of places. In particular we must never create/delete directories there, nor add tasks. Essentially the systemd mount is to be considered read-only for libvirt.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/vircgroup.c | 68 +++++++++++++++++++++++++++++++++++++++------------ src/util/vircgroup.h | 1 + tests/vircgrouptest.c | 9 +++++++ 3 files changed, 63 insertions(+), 15 deletions(-)
@@ -524,13 +541,16 @@ static int virCgroupDetect(virCgroupPtr group, return -1; }
- if (parent || path[0] == '/') { - if (virCgroupCopyPlacement(group, path, parent) < 0) - return -1; - } else { - if (virCgroupDetectPlacement(group, pid, path) < 0) - return -1;
This previously called only one of the two functions...
- } + /* In some cases we can copy part of the placement info + * based on the parent cgroup... + */ + if ((parent || path[0] == '/') && + virCgroupCopyPlacement(group, path, parent) < 0) + return -1; + + /* ... but use /proc/cgroups to fill in the rest */ + if (virCgroupDetectPlacement(group, pid, path) < 0)
...now, if virCgroupCopyPlacement returns 0, it calls both functions. Is that intentional?
@@ -822,6 +842,12 @@ static int virCgroupMakeGroup(virCgroupPtr parent, for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { char *path = NULL;
+ /* We must never mkdir() in systemd's hierachy */
s/hierachy/hierarchy/
+ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) { + VIR_DEBUG("Not creating systemd controller group"); + continue; + } + /* Skip over controllers that aren't mounted */ if (!group->controllers[i].mountPoint) { VIR_DEBUG("Skipping unmounted controller %s", @@ -1026,6 +1052,10 @@ int virCgroupRemove(virCgroupPtr group) if (!group->controllers[i].mountPoint) continue;
+ /* We must never rmdir() in systemd's hiearchy */
s/hiearchy/hierarchy/ (hmm, you copied-and-pasted, but ended up with a different typo between the two comments?)
+ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) + continue; + /* Don't delete the root group, if we accidentally ended up in it for some reason */ if (STREQ(group->controllers[i].placement, "/")) @@ -1065,6 +1095,10 @@ int virCgroupAddTask(virCgroupPtr group, pid_t pid) if (!group->controllers[i].mountPoint) continue;
+ /* We must never add tasks in systemd's hiearchy */
s/hiearchy/hierarchy/
+ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) + continue; + if (virCgroupSetValueU64(group, i, "tasks", (unsigned long long)pid) < 0) goto cleanup; } @@ -1166,6 +1200,10 @@ int virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr dest_group) !dest_group->controllers[i].mountPoint) continue;
+ /* We must never move tasks in systemd's hiearchy */
s/hiearchy/hierarchy/ Conditional ACK if you can address my question and fix the typos. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org