On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list