[libvirt] [PATCH 2/4] Add support for systemd cgroup mount

[Eric Blake]

On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at 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 at 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130730/75303cdf/attachment-0001.sig>