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

diff --git a/src/cgroup.c b/src/cgroup.c
index d9c31413f514aa3e3e57988d2b843bead9b9afaa..d3d45d2e7ec4cc91c306502b4d2b5356f40531a6 100644 (file)
--- a/src/cgroup.c
+++ b/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:
  *

diff --git a/src/cgroup.h b/src/cgroup.h
index 11c44f98e87816c50c35fca932c0f21687d56ece..dbb444bef0af592a1cafb8741c31d78750e60d96 100644 (file)
--- a/src/cgroup.h
+++ b/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);
 

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7a62b67017d8e9f029738e3df36ad40639836cc7..0e9c9f72a38d59d05a1b9da16b1ed57dec6abf4c 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -382,6 +382,7 @@ virRun;
 virSkipSpaces;
 virKillProcess;
 virGetUserDirectory;
+virGetUserName;
 virGetUserID;
 virGetGroupID;
 

diff --git a/src/lxc_conf.h b/src/lxc_conf.h
index 9690cc82824be02d973fba7adf357ea0c47426d6..15402d8d11cf15812b5781a77812bbf2bd377501 100644 (file)
--- a/src/lxc_conf.h
+++ b/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;

diff --git a/src/lxc_controller.c b/src/lxc_controller.c
index 08fbbe43fb818d2d31fe03ec0fe2765cb9ce6606..9ad48a7d13e56a6d74fdf4e6975c6f0ff9ee024d 100644 (file)
--- a/src/lxc_controller.c
+++ b/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;

diff --git a/src/lxc_driver.c b/src/lxc_driver.c
index 4ce8e40a52b1db2fc327836b6bfffef87f07386b..e670fb60e900444e2fc7d2556b2cd18c49a9c27b 100644 (file)
--- a/src/lxc_driver.c
+++ b/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)

diff --git a/src/util.c b/src/util.c
index 2420f8bcb4e33c6391c495e0ff5c94a036b6e8a7..5261714498f67a403b81e7c5d42ce7268c2071ce 100644 (file)
--- a/src/util.c
+++ b/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,

diff --git a/src/util.h b/src/util.h
index 43a5bbc219857d6c8be89d710295e21f9f0593aa..3102014e75626394adf6b24ade93e5bb3fc1ea6b 100644 (file)
--- a/src/util.h
+++ b/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);