[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 4/8] 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

Signed-off-by: Daniel P. Berrange <berrange redhat com>
---
 src/cgroup.c             |  162 ++++++++++++++++++++++++----------------------
 src/cgroup.h             |   16 ++--
 src/libvirt_private.syms |    1 +
 src/lxc_conf.h           |    2 +
 src/lxc_controller.c     |   19 ++++--
 src/lxc_driver.c         |   25 +++++---
 src/util.c               |   27 +++++++-
 src/util.h               |    2 +
 8 files changed, 150 insertions(+), 104 deletions(-)

diff --git a/src/cgroup.c b/src/cgroup.c
index d9c3141..d3d45d2 100644
--- 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 11c44f9..dbb444b 100644
--- 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 7a62b67..0e9c9f7 100644
--- 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 30a51e1..24aadf7 100644
--- a/src/lxc_conf.h
+++ b/src/lxc_conf.h
@@ -30,6 +30,7 @@
 #include "domain_conf.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"
@@ -41,6 +42,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 08fbbe4..9ad48a7 100644
--- 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 3503573..ff35845 100644
--- 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"
 
 
@@ -393,10 +392,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;
@@ -527,7 +526,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);
     }
@@ -1145,6 +1145,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
@@ -1174,6 +1175,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;
@@ -1193,7 +1201,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) {
@@ -1330,7 +1337,7 @@ static int lxcSetSchedulerParameters(virDomainPtr domain,
     virDomainObjPtr vm = NULL;
     int ret = -1;
 
-    if (virCgroupHaveSupport() != 0)
+    if (driver->cgroup == NULL)
         return -1;
 
     lxcDriverLock(driver);
@@ -1343,7 +1350,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++) {
@@ -1377,7 +1384,7 @@ static int lxcGetSchedulerParameters(virDomainPtr domain,
     unsigned long val;
     int ret = -1;
 
-    if (virCgroupHaveSupport() != 0)
+    if (driver->cgroup == NULL)
         return -1;
 
     if ((*nparams) != 1) {
@@ -1396,7 +1403,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 2420f8b..5261714 100644
--- 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 1a7286c..e905c38 100644
--- 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);
-- 
1.6.2.5


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]