Refactor cgroups to allow a group per driver to be managed directly

Allow the driver level cgroup to be managed explicitly by the
hypervisor drivers, in order to detect whether to enable or
disable cgroup support for domains. Provides better error
reporting of failures. Also allow for creation of cgroups for
unprivileged drivers if controller is accessible by the user.

* src/cgroup.c, src/cgroup.h: Add an API to obtain a driver cgroup
* src/lxc_conf.h, src/lxc_controller.c, src/lxc_driver.c:
  Obtain a driver cgroup at startup and use that instead of
  re-creating everytime.
* src/util.c, src/util.h, src/libvirt_private.syms: Add a
  virGetUserName() helper

src/cgroup.c

@@ -484,36 +484,6 @@ static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group)
 }
 
 
-static int virCgroupRoot(virCgroupPtr *group);
-/**
- * virCgroupHaveSupport:
- *
- * Returns 0 if support is present, negative if not
- */
-int virCgroupHaveSupport(void)
-{
-    virCgroupPtr root;
-    int i;
-    int rc;
-    int any = 0;
-
-    rc = virCgroupRoot(&root);
-    if (rc < 0)
-        return rc;
-
-    for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) {
-        if (root->controllers[i].mountPoint != NULL)
-            any = 1;
-    }
-
-    virCgroupFree(&root);
-
-    if (any)
-        return 0;
-
-    return -ENOENT;
-}
-
 static int virCgroupNew(const char *path,
                         virCgroupPtr *group)
 {
@@ -547,47 +517,47 @@ err:
     return rc;
 }
 
-static int virCgroupRoot(virCgroupPtr *group)
-{
-    return virCgroupNew("/", group);
-}
-
-static int virCgroupCreate(virCgroupPtr parent,
-                           const char *name,
-                           virCgroupPtr *group)
+static int virCgroupAppRoot(int privileged,
+                            virCgroupPtr *group)
 {
-    int rc = 0;
-    char *path;
+    virCgroupPtr rootgrp = NULL;
+    int rc;
 
-    VIR_DEBUG("Creating %s under %s", name, parent->path);
+    rc = virCgroupNew("/", &rootgrp);
+    if (rc != 0)
+        return rc;
 
-    rc = virAsprintf(&path, "%s/%s",
-                     STREQ(parent->path, "/") ?
-                     "" : parent->path,
-                     name);
-    if (rc < 0) {
-        rc = -ENOMEM;
-        goto err;
-    }
+    if (privileged) {
+        rc = virCgroupNew("/libvirt", group);
+    } else {
+        char *rootname;
+        char *username;
+        username = virGetUserName(NULL, getuid());
+        if (!username) {
+            rc = -ENOMEM;
+            goto cleanup;
+        }
+        rc = virAsprintf(&rootname, "/libvirt-%s", username);
+        VIR_FREE(username);
+        if (rc < 0) {
+            rc = -ENOMEM;
+            goto cleanup;
+        }
 
-    rc = virCgroupNew(path, group);
-    if (rc != 0) {
-        DEBUG0("Unable to allocate new virCgroup structure");
-        goto err;
+        rc = virCgroupNew(rootname, group);
+        VIR_FREE(rootname);
     }
-
-    rc = virCgroupMakeGroup(parent, *group);
     if (rc != 0)
-        goto err;
+        goto cleanup;
 
-    return rc;
-err:
-    virCgroupFree(group);
-    *group = NULL;
+    rc = virCgroupMakeGroup(rootgrp, *group);
 
+cleanup:
+    virCgroupFree(&rootgrp);
     return rc;
 }
 
+
 /**
  * virCgroupRemove:
  *
@@ -648,45 +618,83 @@ int virCgroupAddTask(virCgroupPtr group, pid_t pid)
     return rc;
 }
 
+
 /**
- * virCgroupForDomain:
+ * virCgroupForDriver:
  *
- * @def: Domain definition to create cgroup for
- * @driverName: Classification of this domain type (e.g., xen, qemu, lxc)
+ * @name: name of this driver (e.g., xen, qemu, lxc)
  * @group: Pointer to returned virCgroupPtr
  *
  * Returns 0 on success
  */
-int virCgroupForDomain(virDomainDefPtr def,
-                       const char *driverName,
-                       virCgroupPtr *group)
+int virCgroupForDriver(const char *name,
+                       virCgroupPtr *group,
+                       int privileged,
+                       int create)
 {
     int rc;
+    char *path = NULL;
     virCgroupPtr rootgrp = NULL;
-    virCgroupPtr daemongrp = NULL;
-    virCgroupPtr typegrp = NULL;
 
-    rc = virCgroupRoot(&rootgrp);
+    rc = virCgroupAppRoot(privileged, &rootgrp);
     if (rc != 0)
         goto out;
 
-    rc = virCgroupCreate(rootgrp, "libvirt", &daemongrp);
-    if (rc != 0)
+    if (virAsprintf(&path, "%s/%s", rootgrp->path, name) < 0) {
+        rc = -ENOMEM;
         goto out;
+    }
 
-    rc = virCgroupCreate(daemongrp, driverName, &typegrp);
-    if (rc != 0)
-        goto out;
+    rc = virCgroupNew(path, group);
+    VIR_FREE(path);
+
+    if (rc == 0 &&
+        create) {
+        rc = virCgroupMakeGroup(rootgrp, *group);
+        if (rc != 0)
+            virCgroupFree(group);
+    }
 
-    rc = virCgroupCreate(typegrp, def->name, group);
 out:
-    virCgroupFree(&typegrp);
-    virCgroupFree(&daemongrp);
     virCgroupFree(&rootgrp);
 
     return rc;
 }
 
+
+/**
+ * virCgroupForDomain:
+ *
+ * @driver: group for driver owning the domain
+ * @name: name of the domain
+ * @group: Pointer to returned virCgroupPtr
+ *
+ * Returns 0 on success
+ */
+int virCgroupForDomain(virCgroupPtr driver,
+                       const char *name,
+                       virCgroupPtr *group,
+                       int create)
+{
+    int rc;
+    char *path;
+
+    if (virAsprintf(&path, "%s/%s", driver->path, name) < 0)
+        return -ENOMEM;
+
+    rc = virCgroupNew(path, group);
+    VIR_FREE(path);
+
+    if (rc == 0 &&
+        create) {
+        rc = virCgroupMakeGroup(driver, *group);
+        if (rc != 0)
+            virCgroupFree(group);
+    }
+
+    return rc;
+}
+
 /**
  * virCgroupSetMemory:
  *

src/cgroup.h

@@ -12,18 +12,18 @@
 #ifndef CGROUP_H
 #define CGROUP_H
 
-#include <stdint.h>
-
 struct virCgroup;
 typedef struct virCgroup *virCgroupPtr;
 
-#include "domain_conf.h"
-
-int virCgroupHaveSupport(void);
+int virCgroupForDriver(const char *name,
+                       virCgroupPtr *group,
+                       int privileged,
+                       int create);
 
-int virCgroupForDomain(virDomainDefPtr def,
-                       const char *driverName,
-                       virCgroupPtr *group);
+int virCgroupForDomain(virCgroupPtr driver,
+                       const char *name,
+                       virCgroupPtr *group,
+                       int create);
 
 int virCgroupAddTask(virCgroupPtr group, pid_t pid);
 

src/libvirt_private.syms

@@ -382,6 +382,7 @@ virRun;
 virSkipSpaces;
 virKillProcess;
 virGetUserDirectory;
+virGetUserName;
 virGetUserID;
 virGetGroupID;
 

src/lxc_conf.h

@@ -31,6 +31,7 @@
 #include "domain_event.h"
 #include "capabilities.h"
 #include "threads.h"
+#include "cgroup.h"
 
 #define LXC_CONFIG_DIR SYSCONF_DIR "/libvirt/lxc"
 #define LXC_STATE_DIR LOCAL_STATE_DIR "/run/libvirt/lxc"
@@ -42,6 +43,7 @@ struct __lxc_driver {
 
     virCapsPtr caps;
 
+    virCgroupPtr cgroup;
     virDomainObjList domains;
     char *configDir;
     char *autostartDir;

src/lxc_controller.c

@@ -48,7 +48,6 @@
 #include "veth.h"
 #include "memory.h"
 #include "util.h"
-#include "cgroup.h"
 
 #define VIR_FROM_THIS VIR_FROM_LXC
 
@@ -69,6 +68,7 @@ struct cgroup_device_policy {
  */
 static int lxcSetContainerResources(virDomainDefPtr def)
 {
+    virCgroupPtr driver;
     virCgroupPtr cgroup;
     int rc = -1;
     int i;
@@ -82,14 +82,18 @@ static int lxcSetContainerResources(virDomainDefPtr def)
         {'c', LXC_DEV_MAJ_TTY, LXC_DEV_MIN_PTMX},
         {0,   0, 0}};
 
-    if (virCgroupHaveSupport() != 0)
-        return 0; /* Not supported, so claim success */
+    rc = virCgroupForDriver("lxc", &driver, 1, 0);
+    if (rc != 0) {
+        lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s",
+                 _("Unable to get cgroup for driver"));
+        return rc;
+    }
 
-    rc = virCgroupForDomain(def, "lxc", &cgroup);
+    rc = virCgroupForDomain(driver, def->name, &cgroup, 1);
     if (rc != 0) {
         lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                 _("Unable to create cgroup for %s\n"), def->name);
-        return rc;
+                 _("Unable to create cgroup for domain %s"), def->name);
+        goto cleanup;
     }
 
     rc = virCgroupSetMemory(cgroup, def->maxmem);
@@ -119,9 +123,10 @@ out:
     if (rc != 0) {
         virReportSystemError(NULL, -rc, "%s",
                              _("Failed to set lxc resources"));
-        virCgroupRemove(cgroup);
     }
 
+cleanup:
+    virCgroupFree(&driver);
     virCgroupFree(&cgroup);
 
     return rc;

src/lxc_driver.c

@@ -46,7 +46,6 @@
 #include "bridge.h"
 #include "veth.h"
 #include "event.h"
-#include "cgroup.h"
 #include "nodeinfo.h"
 #include "uuid.h"
 
@@ -421,10 +420,10 @@ static int lxcDomainGetInfo(virDomainPtr dom,
 
     info->state = vm->state;
 
-    if (!virDomainIsActive(vm) || virCgroupHaveSupport() != 0) {
+    if (!virDomainIsActive(vm) || driver->cgroup == NULL) {
         info->cpuTime = 0;
     } else {
-        if (virCgroupForDomain(vm->def, "lxc", &cgroup) != 0) {
+        if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) {
             lxcError(dom->conn, dom, VIR_ERR_INTERNAL_ERROR,
                      _("Unable to get cgroup for %s\n"), vm->def->name);
             goto cleanup;
@@ -555,7 +554,8 @@ static int lxcVMCleanup(virConnectPtr conn,
         vethDelete(vm->def->nets[i]->ifname);
     }
 
-    if (virCgroupForDomain(vm->def, "lxc", &cgroup) == 0) {
+    if (driver->cgroup &&
+        virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) == 0) {
         virCgroupRemove(cgroup);
         virCgroupFree(&cgroup);
     }
@@ -1348,6 +1348,7 @@ static int lxcStartup(int privileged)
 {
     unsigned int i;
     char *ld;
+    int rc;
 
     /* Valgrind gets very annoyed when we clone containers, so
      * disable LXC when under valgrind
@@ -1386,6 +1387,13 @@ static int lxcStartup(int privileged)
 
     lxc_driver->have_netns = lxcCheckNetNsSupport();
 
+    rc = virCgroupForDriver("lxc", &lxc_driver->cgroup, privileged, 1);
+    if (rc < 0) {
+        char buf[1024];
+        VIR_WARN("Unable to create cgroup for driver: %s",
+                 virStrerror(-rc, buf, sizeof(buf)));
+    }
+
     /* Call function to load lxc driver configuration information */
     if (lxcLoadDriverConfig(lxc_driver) < 0)
         goto cleanup;
@@ -1405,7 +1413,6 @@ static int lxcStartup(int privileged)
         virDomainObjPtr vm = lxc_driver->domains.objs[i];
         char *config = NULL;
         virDomainDefPtr tmp;
-        int rc;
 
         virDomainObjLock(vm);
         if ((vm->monitor = lxcMonitorClient(NULL, lxc_driver, vm)) < 0) {
@@ -1587,7 +1594,7 @@ static int lxcSetSchedulerParameters(virDomainPtr domain,
     virDomainObjPtr vm = NULL;
     int ret = -1;
 
-    if (virCgroupHaveSupport() != 0)
+    if (driver->cgroup == NULL)
         return -1;
 
     lxcDriverLock(driver);
@@ -1600,7 +1607,7 @@ static int lxcSetSchedulerParameters(virDomainPtr domain,
         goto cleanup;
     }
 
-    if (virCgroupForDomain(vm->def, "lxc", &group) != 0)
+    if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0)
         goto cleanup;
 
     for (i = 0; i < nparams; i++) {
@@ -1634,7 +1641,7 @@ static int lxcGetSchedulerParameters(virDomainPtr domain,
     unsigned long val;
     int ret = -1;
 
-    if (virCgroupHaveSupport() != 0)
+    if (driver->cgroup == NULL)
         return -1;
 
     if ((*nparams) != 1) {
@@ -1653,7 +1660,7 @@ static int lxcGetSchedulerParameters(virDomainPtr domain,
         goto cleanup;
     }
 
-    if (virCgroupForDomain(vm->def, "lxc", &group) != 0)
+    if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0)
         goto cleanup;
 
     if (virCgroupGetCpuShares(group, &val) != 0)

src/util.c

@@ -1826,8 +1826,14 @@ int virRandom(int max)
 
 
 #ifdef HAVE_GETPWUID_R
-char *virGetUserDirectory(virConnectPtr conn,
-                          uid_t uid)
+enum {
+    VIR_USER_ENT_DIRECTORY,
+    VIR_USER_ENT_NAME,
+};
+
+static char *virGetUserEnt(virConnectPtr conn,
+                           uid_t uid,
+                           int field)
 {
     char *strbuf;
     char *ret;
@@ -1855,7 +1861,10 @@ char *virGetUserDirectory(virConnectPtr conn,
         return NULL;
     }
 
-    ret = strdup(pw->pw_dir);
+    if (field == VIR_USER_ENT_DIRECTORY)
+        ret = strdup(pw->pw_dir);
+    else
+        ret = strdup(pw->pw_name);
 
     VIR_FREE(strbuf);
     if (!ret)
@@ -1864,6 +1873,18 @@ char *virGetUserDirectory(virConnectPtr conn,
     return ret;
 }
 
+char *virGetUserDirectory(virConnectPtr conn,
+                          uid_t uid)
+{
+    return virGetUserEnt(conn, uid, VIR_USER_ENT_DIRECTORY);
+}
+
+char *virGetUserName(virConnectPtr conn,
+                     uid_t uid)
+{
+    return virGetUserEnt(conn, uid, VIR_USER_ENT_NAME);
+}
+
 
 int virGetUserID(virConnectPtr conn,
                  const char *name,

src/util.h

@@ -217,6 +217,8 @@ int virKillProcess(pid_t pid, int sig);
 #ifdef HAVE_GETPWUID_R
 char *virGetUserDirectory(virConnectPtr conn,
                           uid_t uid);
+char *virGetUserName(virConnectPtr conn,
+                     uid_t uid);
 int virGetUserID(virConnectPtr conn,
                  const char *name,
                  uid_t *uid);