10:37 a.m.
On Fri, Oct 03, 2008 at 08:40:22AM -0700, Dan Smith wrote:
Revision 3 of the cgroups patch, with the round 2 comments addressed.
Fine for me +1
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
10:40 a.m.
New subject: [libvirt] [PATCH 1 of 2] Add internal cgroup manipulation functions
This patch adds src/cgroup.{c,h} with support for creating and manipulating
cgroups.
All groups created with the internal API are forced under $mount/libvirt/
to keep everything together. The first time a group is created, the libvirt
directory is also created, and the settings from the root are inherited.
The code creates groups in all mounts requires to get memory and devices
functionality. When setting a value, the appropriate mount is determined
and the value is set there. I have tested this with all controllers mounted
in a single location, as well as all of them mounted separately.
Changes:
- Handle multiple mount points
- Add more abstract internal API, per recent discussion
- Consider absence of memory or devices controllers as "no cgroup support"
diff -r 444e2614d0a2 -r 5590af0c8c3e src/cgroup.c
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/src/cgroup.c Fri Oct 03 08:06:52 2008 -0700
@@ -0,0 +1,762 @@
+/*
+ * cgroup.c: Tools for managing cgroups
+ *
+ * Copyright IBM Corp. 2008
+ *
+ * See COPYING.LIB for the License of this software
+ *
+ * Authors:
+ * Dan Smith <danms(a)us.ibm.com>
+ */
+#include <config.h>
+
+#include <stdio.h>
+#include <stdint.h>
+#include <inttypes.h>
+#include <mntent.h>
+#include <fcntl.h>
+#include <string.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <libgen.h>
+
+#include "internal.h"
+#include "util.h"
+#include "memory.h"
+#include "cgroup.h"
+
+#define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt, __VA_ARGS__)
+#define DEBUG0(msg) VIR_DEBUG(__FILE__, "%s", msg)
+
+#define CGROUP_MAX_VAL 512
+
+struct virCgroup {
+ char *path;
+};
+
+const char *supported_controllers[] = {
+ "memory",
+ "devices",
+ NULL
+};
+
+/**
+ * virCgroupFree:
+ *
+ * @group: The group structure to free
+ */
+void virCgroupFree(virCgroupPtr *group)
+{
+ if (*group != NULL) {
+ VIR_FREE((*group)->path);
+ VIR_FREE(*group);
+ *group = NULL;
+ }
+}
+
+static virCgroupPtr virCgroupGetMount(const char *controller)
+{
+ FILE *mounts;
+ struct mntent entry;
+ char buf[CGROUP_MAX_VAL];
+ virCgroupPtr root = NULL;
+
+ if (VIR_ALLOC(root) != 0)
+ return NULL;
+
+ mounts = fopen("/proc/mounts", "r");
+ if (mounts == NULL) {
+ DEBUG0("Unable to open /proc/mounts: %m");
+ goto err;
+ }
+
+ while (getmntent_r(mounts, &entry, buf, sizeof(buf)) != NULL) {
+ if (STREQ(entry.mnt_type, "cgroup") &&
+ (strstr(entry.mnt_opts, controller))) {
+ root->path = strdup(entry.mnt_dir);
+ break;
+ }
+ }
+
+ DEBUG("Mount for %s is %s\n", controller, root->path);
+
+ if (root->path == NULL) {
+ DEBUG0("Did not find cgroup mount");
+ goto err;
+ }
+
+ fclose(mounts);
+
+ return root;
+err:
+ virCgroupFree(&root);
+
+ return NULL;
+}
+
+/**
+ * virCgroupHaveSupport:
+ *
+ * Returns 0 if support is present, negative if not
+ */
+int virCgroupHaveSupport(void)
+{
+ virCgroupPtr root;
+ int i;
+
+ for (i = 0; supported_controllers[i] != NULL; i++) {
+ root = virCgroupGetMount(supported_controllers[i]);
+ if (root == NULL)
+ return -1;
+ virCgroupFree(&root);
+ }
+
+ return 0;
+}
+
+static int virCgroupPathOfGroup(const char *group,
+ const char *controller,
+ char **path)
+{
+ virCgroupPtr root = NULL;
+ int rc = 0;
+
+ root = virCgroupGetMount(controller);
+ if (root == NULL) {
+ rc = -ENOTDIR;
+ goto out;
+ }
+
+ if (asprintf(path, "%s/%s", root->path, group) == -1)
+ rc = -ENOMEM;
+out:
+ virCgroupFree(&root);
+
+ return rc;
+}
+
+static int virCgroupPathOf(const char *grppath,
+ const char *key,
+ char **path)
+{
+ virCgroupPtr root;
+ int rc = 0;
+ char *controller = NULL;
+
+ if (strchr(key, '.') == NULL)
+ return -EINVAL;
+
+ if (sscanf(key, "%a[^.]", &controller) != 1)
+ return -EINVAL;
+
+ root = virCgroupGetMount(controller);
+ if (root == NULL) {
+ rc = -ENOTDIR;
+ goto out;
+ }
+
+ if (asprintf(path, "%s/%s/%s", root->path, grppath, key) == -1)
+ rc = -ENOMEM;
+out:
+ virCgroupFree(&root);
+ VIR_FREE(controller);
+
+ return rc;
+}
+
+static int virCgroupSetValueStr(virCgroupPtr group,
+ const char *key,
+ const char *value)
+{
+ int fd = -1;
+ int rc = 0;
+ char *keypath = NULL;
+
+ rc = virCgroupPathOf(group->path, key, &keypath);
+ if (rc != 0)
+ return rc;
+
+ fd = open(keypath, O_WRONLY);
+ if (fd < 0) {
+ DEBUG("Unable to open %s: %m", keypath);
+ rc = -ENOENT;
+ goto out;
+ }
+
+ DEBUG("Writing '%s' to '%s'", value, keypath);
+
+ rc = safewrite(fd, value, strlen(value));
+ if (rc < 0) {
+ DEBUG("Failed to write value '%s': %m", value);
+ rc = -errno;
+ goto out;
+ } else if (rc != strlen(value)) {
+ DEBUG("Short write of value '%s'", value);
+ rc = -ENOSPC;
+ goto out;
+ }
+
+ rc = 0;
+out:
+ VIR_FREE(keypath);
+ close(fd);
+
+ return rc;
+}
+
+static int virCgroupSetValueU64(virCgroupPtr group,
+ const char *key,
+ uint64_t value)
+{
+ char *strval = NULL;
+ int rc;
+
+ if (asprintf(&strval, "%" PRIu64, value) == -1)
+ return -ENOMEM;
+
+ rc = virCgroupSetValueStr(group, key, strval);
+
+ VIR_FREE(strval);
+
+ return rc;
+}
+
+#if 0
+/* This is included for completeness, but not yet used */
+
+static int virCgroupSetValueI64(virCgroupPtr group,
+ const char *key,
+ int64_t value)
+{
+ char *strval = NULL;
+ int rc;
+
+ if (asprintf(&strval, "%" PRIi64, value) == -1)
+ return -ENOMEM;
+
+ rc = virCgroupSetValueStr(group, key, strval);
+
+ VIR_FREE(strval);
+
+ return rc;
+}
+
+static int virCgroupGetValueStr(virCgroupPtr group,
+ const char *key,
+ char **value)
+{
+ int fd = -1;
+ int rc;
+ char *keypath = NULL;
+ char buf[CGROUP_MAX_VAL];
+
+ memset(buf, 0, sizeof(buf));
+
+ rc = virCgroupPathOf(group->path, key, &keypath);
+ if (rc != 0) {
+ DEBUG("No path of %s, %s", group->path, key);
+ return rc;
+ }
+
+ fd = open(keypath, O_RDONLY);
+ if (fd < 0) {
+ DEBUG("Unable to open %s: %m", keypath);
+ rc = -ENOENT;
+ goto out;
+ }
+
+ rc = saferead(fd, buf, sizeof(buf));
+ if (rc < 0) {
+ DEBUG("Failed to read %s: %m\n", keypath);
+ rc = -errno;
+ goto out;
+ } else if (rc == 0) {
+ DEBUG("Short read of %s\n", keypath);
+ rc = -EIO;
+ goto out;
+ }
+
+ *value = strdup(buf);
+ if (*value == NULL) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ rc = 0;
+out:
+ VIR_FREE(keypath);
+ close(fd);
+
+ return rc;
+}
+
+static int virCgroupGetValueU64(virCgroupPtr group,
+ const char *key,
+ uint64_t *value)
+{
+ char *strval = NULL;
+ int rc = 0;
+
+ rc = virCgroupGetValueStr(group, key, &strval);
+ if (rc != 0)
+ goto out;
+
+ if (sscanf(strval, "%" SCNu64, value) != 1)
+ rc = -EINVAL;
+out:
+ VIR_FREE(strval);
+
+ return rc;
+}
+
+static int virCgroupGetValueI64(virCgroupPtr group,
+ const char *key,
+ int64_t *value)
+{
+ char *strval = NULL;
+ int rc = 0;
+
+ rc = virCgroupGetValueStr(group, key, &strval);
+ if (rc != 0)
+ goto out;
+
+ if (sscanf(strval, "%" SCNi64, value) != 1)
+ rc = -EINVAL;
+out:
+ VIR_FREE(strval);
+
+ return rc;
+}
+#endif
+
+static int _virCgroupInherit(const char *path,
+ const char *key)
+{
+ int rc = 0;
+ int fd = -1;
+ char buf[CGROUP_MAX_VAL];
+ char *keypath = NULL;
+ char *pkeypath = NULL;
+
+ memset(buf, 0, sizeof(buf));
+
+ if (asprintf(&keypath, "%s/%s", path, key) == -1) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ if (access(keypath, F_OK) != 0) {
+ DEBUG("Group %s has no key %s\n", path, key);
+ goto out;
+ }
+
+ if (asprintf(&pkeypath, "%s/../%s", path, key) == -1) {
+ rc = -ENOMEM;
+ VIR_FREE(keypath);
+ goto out;
+ }
+
+ fd = open(pkeypath, O_RDONLY);
+ if (fd < 0) {
+ rc = -errno;
+ goto out;
+ }
+
+ if (saferead(fd, buf, sizeof(buf)) <= 0) {
+ rc = -errno;
+ goto out;
+ }
+
+ close(fd);
+
+ fd = open(keypath, O_WRONLY);
+ if (fd < 0) {
+ rc = -errno;
+ goto out;
+ }
+
+ if (safewrite(fd, buf, strlen(buf)) != strlen(buf)) {
+ rc = -errno;
+ goto out;
+ }
+
+out:
+ VIR_FREE(keypath);
+ VIR_FREE(pkeypath);
+ close(fd);
+
+ return rc;
+}
+
+static int virCgroupInherit(const char *grppath)
+{
+ int i;
+ int rc = 0;
+ const char *inherit_values[] = {
+ "cpuset.cpus",
+ "cpuset.mems",
+ NULL
+ };
+
+ for (i = 0; inherit_values[i] != NULL; i++) {
+ const char *key = inherit_values[i];
+
+ rc = _virCgroupInherit(grppath, key);
+ if (rc != 0) {
+ DEBUG("inherit of %s failed\n", key);
+ break;
+ }
+ }
+
+ return rc;
+}
+
+static int virCgroupMakeGroup(const char *name)
+{
+ int i;
+ int rc = 0;
+
+ for (i = 0; supported_controllers[i] != NULL; i++) {
+ char *path = NULL;
+ virCgroupPtr root;
+
+ root = virCgroupGetMount(supported_controllers[i]);
+ if (root == NULL)
+ continue;
+
+ rc = virCgroupPathOfGroup(name, supported_controllers[i], &path);
+ if (rc != 0) {
+ virCgroupFree(&root);
+ break;
+ }
+
+ virCgroupFree(&root);
+
+ if (access(path, F_OK) != 0) {
+ if (mkdir(path, 0655) < 0) {
+ rc = -errno;
+ VIR_FREE(path);
+ break;
+ }
+ virCgroupInherit(path);
+ }
+
+ VIR_FREE(path);
+ }
+
+ return rc;
+}
+
+static int virCgroupRoot(virCgroupPtr *root)
+{
+ int rc = 0;
+ char *grppath = NULL;
+
+ if (VIR_ALLOC((*root)) != 0) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ (*root)->path = strdup("libvirt");
+ if ((*root)->path == NULL) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ rc = virCgroupMakeGroup((*root)->path);
+out:
+ if (rc != 0)
+ virCgroupFree(root);
+ VIR_FREE(grppath);
+
+ return rc;
+}
+
+static int virCgroupNew(virCgroupPtr *parent,
+ const char *group,
+ virCgroupPtr *newgroup)
+{
+ int rc = 0;
+ char *typpath = NULL;
+
+ *newgroup = NULL;
+
+ if (*parent == NULL) {
+ rc = virCgroupRoot(parent);
+ if (rc != 0)
+ goto err;
+ }
+
+ if (VIR_ALLOC((*newgroup)) != 0) {
+ rc = -ENOMEM;
+ goto err;
+ }
+
+ rc = asprintf(&((*newgroup)->path),
+ "%s/%s",
+ (*parent)->path,
+ group);
+ if (rc == -1) {
+ rc = -ENOMEM;
+ goto err;
+ }
+
+ rc = 0;
+
+ return rc;
+err:
+ virCgroupFree(newgroup);
+ *newgroup = NULL;
+
+ VIR_FREE(typpath);
+
+ return rc;
+}
+
+static int virCgroupOpen(virCgroupPtr parent,
+ const char *group,
+ virCgroupPtr *newgroup)
+{
+ int rc = 0;
+ char *grppath = NULL;
+ bool free_parent = (parent == NULL);
+
+ rc = virCgroupNew(&parent, group, newgroup);
+ if (rc != 0)
+ goto err;
+
+ if (free_parent)
+ virCgroupFree(&parent);
+
+ rc = virCgroupPathOfGroup((*newgroup)->path,
+ supported_controllers[0],
+ &grppath);
+ if (rc != 0)
+ goto err;
+
+ if (access(grppath, F_OK) != 0) {
+ rc = -ENOENT;
+ goto err;
+ }
+
+ return rc;
+err:
+ virCgroupFree(newgroup);
+ *newgroup = NULL;
+
+ return rc;
+}
+
+static int virCgroupCreate(virCgroupPtr parent,
+ const char *group,
+ virCgroupPtr *newgroup)
+{
+ int rc = 0;
+ bool free_parent = (parent == NULL);
+
+ rc = virCgroupNew(&parent, group, newgroup);
+ if (rc != 0) {
+ DEBUG0("Unable to allocate new virCgroup structure");
+ goto err;
+ }
+
+ rc = virCgroupMakeGroup((*newgroup)->path);
+ if (rc != 0)
+ goto err;
+
+ if (free_parent)
+ virCgroupFree(&parent);
+
+ return rc;
+err:
+ virCgroupFree(newgroup);
+ *newgroup = NULL;
+
+ if (free_parent)
+ virCgroupFree(&parent);
+
+ return rc;
+}
+
+/**
+ * virCgroupRemove:
+ *
+ * @group: The group to be removed
+ *
+ * Returns: 0 on success
+ */
+int virCgroupRemove(virCgroupPtr group)
+{
+ int rc = 0;
+ int i;
+ char *grppath = NULL;
+
+ for (i = 0; supported_controllers[i] != NULL; i++) {
+ if (virCgroupPathOfGroup(group->path,
+ supported_controllers[i],
+ &grppath) != 0)
+ continue;
+
+ if (rmdir(grppath) != 0)
+ rc = -errno;
+
+ VIR_FREE(grppath);
+ }
+
+ return rc;
+}
+
+/**
+ * virCgroupAddTask:
+ *
+ * @group: The cgroup to add a task to
+ * @pid: The pid of the task to add
+ *
+ * Returns: 0 on success
+ */
+int virCgroupAddTask(virCgroupPtr group, pid_t pid)
+{
+ int rc = 0;
+ int fd = -1;
+ int i;
+ char *grppath = NULL;
+ char *taskpath = NULL;
+ char *pidstr = NULL;
+
+ for (i = 0; supported_controllers[i] != NULL; i++) {
+ rc = virCgroupPathOfGroup(group->path,
+ supported_controllers[i],
+ &grppath);
+ if (rc != 0)
+ goto done;
+
+ if (asprintf(&taskpath, "%s/tasks", grppath) == -1) {
+ rc = -ENOMEM;
+ goto done;
+ }
+
+ fd = open(taskpath, O_WRONLY);
+ if (fd < 0) {
+ rc = -errno;
+ goto done;
+ }
+
+ if (asprintf(&pidstr, "%lu", (unsigned long)pid) == -1) {
+ rc = -ENOMEM;
+ goto done;
+ }
+
+ if (write(fd, pidstr, strlen(pidstr)) <= 0) {
+ rc = -errno;
+ goto done;
+ }
+
+ done:
+ VIR_FREE(grppath);
+ VIR_FREE(taskpath);
+ VIR_FREE(pidstr);
+ close(fd);
+
+ if (rc != 0)
+ break;
+ }
+
+ return rc;
+}
+
+/**
+ * virCgroupForDomain:
+ *
+ * @def: Domain definition to create cgroup for
+ * @driverName: Classification of this domain type (e.g., xen, qemu, lxc)
+ * @group: Pointer to returned virCgroupPtr
+ *
+ * Returns 0 on success
+ */
+int virCgroupForDomain(virDomainDefPtr def,
+ const char *driverName,
+ virCgroupPtr *group)
+{
+ int rc;
+ virCgroupPtr typegrp = NULL;
+
+ rc = virCgroupOpen(NULL, driverName, &typegrp);
+ if (rc == -ENOENT) {
+ rc = virCgroupCreate(NULL, driverName, &typegrp);
+ if (rc != 0)
+ goto out;
+ } else if (rc != 0)
+ goto out;
+
+ rc = virCgroupOpen(typegrp, def->name, group);
+ if (rc == -ENOENT)
+ rc = virCgroupCreate(typegrp, def->name, group);
+out:
+ virCgroupFree(&typegrp);
+
+ return rc;
+}
+
+/**
+ * virCgroupSetMemory:
+ *
+ * @group: The cgroup to change memory for
+ * @kb: The memory amount in kilobytes
+ *
+ * Returns: 0 on success
+ */
+int virCgroupSetMemory(virCgroupPtr group, unsigned long kb)
+{
+ return virCgroupSetValueU64(group,
+ "memory.limit_in_bytes",
+ kb << 10);
+}
+
+/**
+ * virCgroupDenyAllDevices:
+ *
+ * @group: The cgroup to deny devices for
+ *
+ * Returns: 0 on success
+ */
+int virCgroupDenyAllDevices(virCgroupPtr group)
+{
+ return virCgroupSetValueStr(group,
+ "devices.deny",
+ "a");
+}
+
+/**
+ * virCgroupAllowDevice:
+ *
+ * @group: The cgroup to allow a device for
+ * @type: The device type (i.e., 'c' or 'b')
+ * @major: The major number of the device
+ * @minor: The minor number of the device
+ *
+ * Returns: 0 on success
+ */
+int virCgroupAllowDevice(virCgroupPtr group,
+ char type,
+ int major,
+ int minor)
+{
+ int rc;
+ char *devstr = NULL;
+
+ if (asprintf(&devstr, "%c %i:%i rwm", type, major, minor) == -1) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ rc = virCgroupSetValueStr(group,
+ "devices.allow",
+ devstr);
+out:
+ VIR_FREE(devstr);
+
+ return rc;
+}
diff -r 444e2614d0a2 -r 5590af0c8c3e src/cgroup.h
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/src/cgroup.h Fri Oct 03 08:06:52 2008 -0700
@@ -0,0 +1,53 @@
+/*
+ * cgroup.h: Interface to tools for managing cgroups
+ *
+ * Copyright IBM Corp. 2008
+ *
+ * See COPYING.LIB for the License of this software
+ *
+ * Authors:
+ * Dan Smith <danms(a)us.ibm.com>
+ */
+
+#ifndef CGROUP_H
+#define CGROUP_H
+
+#include <stdint.h>
+
+struct virCgroup;
+typedef struct virCgroup *virCgroupPtr;
+
+#include "domain_conf.h"
+
+#define VIR_CG_DEV_MAJ_MEMORY 1
+#define VIR_CG_DEV_MIN_NULL 3
+#define VIR_CG_DEV_MIN_ZERO 5
+#define VIR_CG_DEV_MIN_FULL 7
+#define VIR_CG_DEV_MIN_RANDOM 8
+#define VIR_CG_DEV_MIN_URANDOM 9
+
+#define VIR_CG_DEV_MAJ_TTY 5
+#define VIR_CG_DEV_MIN_CONSOLE 1
+
+int virCgroupHaveSupport(void);
+
+int virCgroupForDomain(virDomainDefPtr def,
+ const char *driverName,
+ virCgroupPtr *group);
+
+int virCgroupAddTask(virCgroupPtr group, pid_t pid);
+
+int virCgroupSetMemory(virCgroupPtr group, unsigned long kb);
+
+int virCgroupDenyAllDevices(virCgroupPtr group);
+
+int virCgroupAllowDevice(virCgroupPtr group,
+ char type,
+ int major,
+ int minor);
+
+int virCgroupRemove(virCgroupPtr group);
+
+void virCgroupFree(virCgroupPtr *group);
+
+#endif /* CGROUP_H */
10:40 a.m.
New subject: [libvirt] [PATCH 2 of 2] Use cgroup functions to set resource limits on LXC domains
This patch adds code to the controller to set up a cgroup named after the
domain name, set the memory limit, and restrict devices. It also
adds bits to lxc_driver to properly clean up the cgroup on domain death.
If virCgroupHaveSupport() says that no support is available, then we just
allow the domain creation to proceed as it did before without resource
controls in place.
Changes:
- Use device number constants from cgroup.h
- Be sure to remove the cgroup on the failure path
- Use the more abstract API
diff -r 5590af0c8c3e -r e827f165c23b src/Makefile.am
--- a/src/Makefile.am Fri Oct 03 08:06:52 2008 -0700
+++ b/src/Makefile.am Fri Oct 03 08:38:57 2008 -0700
@@ -90,13 +90,15 @@
lxc_conf.c lxc_conf.h \
lxc_container.c lxc_container.h \
lxc_driver.c lxc_driver.h \
- veth.c veth.h
+ veth.c veth.h \
+ cgroup.c cgroup.h
LXC_CONTROLLER_SOURCES = \
lxc_conf.c lxc_conf.h \
lxc_container.c lxc_container.h \
lxc_controller.c \
- veth.c veth.h
+ veth.c veth.h \
+ cgroup.c cgroup.h
OPENVZ_DRIVER_SOURCES = \
openvz_conf.c openvz_conf.h \
diff -r 5590af0c8c3e -r e827f165c23b src/lxc_controller.c
--- a/src/lxc_controller.c Fri Oct 03 08:06:52 2008 -0700
+++ b/src/lxc_controller.c Fri Oct 03 08:38:57 2008 -0700
@@ -42,12 +42,82 @@
#include "veth.h"
#include "memory.h"
#include "util.h"
-
+#include "cgroup.h"
#define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt, __VA_ARGS__)
#define DEBUG0(msg) VIR_DEBUG(__FILE__, "%s", msg)
int debugFlag = 0;
+
+struct cgroup_device_policy {
+ char type;
+ int major;
+ int minor;
+};
+
+/**
+ * lxcSetContainerResources
+ * @def: pointer to virtual machine structure
+ *
+ * Creates a cgroup for the container, moves the task inside,
+ * and sets resource limits
+ *
+ * Returns 0 on success or -1 in case of error
+ */
+static int lxcSetContainerResources(virDomainDefPtr def)
+{
+ virCgroupPtr cgroup;
+ int rc = -1;
+ int i;
+ struct cgroup_device_policy devices[] = {
+ {'c', VIR_CG_DEV_MAJ_MEMORY, VIR_CG_DEV_MIN_NULL},
+ {'c', VIR_CG_DEV_MAJ_MEMORY, VIR_CG_DEV_MIN_ZERO},
+ {'c', VIR_CG_DEV_MAJ_MEMORY, VIR_CG_DEV_MIN_FULL},
+ {'c', VIR_CG_DEV_MAJ_MEMORY, VIR_CG_DEV_MIN_RANDOM},
+ {'c', VIR_CG_DEV_MAJ_MEMORY, VIR_CG_DEV_MIN_URANDOM},
+ {'c', VIR_CG_DEV_MAJ_TTY, VIR_CG_DEV_MIN_CONSOLE},
+ {0, 0, 0}};
+
+ if (virCgroupHaveSupport() != 0)
+ return 0; /* Not supported, so claim success */
+
+ rc = virCgroupForDomain(def, "lxc", &cgroup);
+ if (rc != 0) {
+ lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("Unable to create cgroup for %s\n"), def->name);
+ return rc;
+ }
+
+ rc = virCgroupSetMemory(cgroup, def->maxmem);
+ if (rc != 0)
+ goto out;
+
+ rc = virCgroupDenyAllDevices(cgroup);
+ if (rc != 0)
+ goto out;
+
+ for (i = 0; devices[i].type != 0; i++) {
+ struct cgroup_device_policy *dev = &devices[i];
+ rc = virCgroupAllowDevice(cgroup,
+ dev->type,
+ dev->major,
+ dev->minor);
+ if (rc != 0)
+ goto out;
+ }
+
+ rc = virCgroupAddTask(cgroup, getpid());
+out:
+ if (rc != 0) {
+ lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("Failed to set lxc resources: %s\n"), strerror(-rc));
+ virCgroupRemove(cgroup);
+ }
+
+ virCgroupFree(&cgroup);
+
+ return rc;
+}
static char*lxcMonitorPath(virDomainDefPtr def)
{
@@ -394,6 +464,9 @@
if (lxcControllerMoveInterfaces(nveths, veths, container) < 0)
goto cleanup;
+ if (lxcSetContainerResources(def) < 0)
+ goto cleanup;
+
if (lxcContainerSendContinue(control[0]) < 0)
goto cleanup;
diff -r 5590af0c8c3e -r e827f165c23b src/lxc_driver.c
--- a/src/lxc_driver.c Fri Oct 03 08:06:52 2008 -0700
+++ b/src/lxc_driver.c Fri Oct 03 08:38:57 2008 -0700
@@ -43,6 +43,7 @@
#include "bridge.h"
#include "veth.h"
#include "event.h"
+#include "cgroup.h"
/* debug macros */
@@ -376,6 +377,7 @@
int waitRc;
int childStatus = -1;
virDomainNetDefPtr net;
+ virCgroupPtr cgroup;
while (((waitRc = waitpid(vm->pid, &childStatus, 0)) == -1) &&
errno == EINTR)
@@ -408,6 +410,11 @@
for (net = vm->def->nets; net; net = net->next) {
vethInterfaceUpOrDown(net->ifname, 0);
vethDelete(net->ifname);
+ }
+
+ if (virCgroupForDomain(vm->def, "lxc", &cgroup) == 0) {
+ virCgroupRemove(cgroup);
+ virCgroupFree(&cgroup);
}
return rc;
10:40 a.m.
New subject: [libvirt] [PATCH 2 of 2] Use cgroup functions to set resource limits on LXC domains
On Fri, Oct 03, 2008 at 08:40:24AM -0700, Dan Smith wrote:
This patch adds code to the controller to set up a cgroup named after
the
domain name, set the memory limit, and restrict devices. It also
adds bits to lxc_driver to properly clean up the cgroup on domain death.
If virCgroupHaveSupport() says that no support is available, then we just
allow the domain creation to proceed as it did before without resource
controls in place.
+ struct cgroup_device_policy devices[] = {
+ {'c', VIR_CG_DEV_MAJ_MEMORY, VIR_CG_DEV_MIN_NULL},
+ {'c', VIR_CG_DEV_MAJ_MEMORY, VIR_CG_DEV_MIN_ZERO},
+ {'c', VIR_CG_DEV_MAJ_MEMORY, VIR_CG_DEV_MIN_FULL},
+ {'c', VIR_CG_DEV_MAJ_MEMORY, VIR_CG_DEV_MIN_RANDOM},
+ {'c', VIR_CG_DEV_MAJ_MEMORY, VIR_CG_DEV_MIN_URANDOM},
+ {'c', VIR_CG_DEV_MAJ_TTY, VIR_CG_DEV_MIN_CONSOLE},
+ {0, 0, 0}};
You're going to hate me for suggesting more changes, but....
This list of devices is currently duplicated in two places - once
here where we set permissions, and again when we actually create
the container and populate its /dev/ in lxc_container.c. Could do
with a master list of device nodes used by both.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
10:45 a.m.
New subject: [libvirt] [PATCH 2 of 2] Use cgroup functions to set resource limits on LXC domains
DB> This list of devices is currently duplicated in two places - once
DB> here where we set permissions, and again when we actually create
DB> the container and populate its /dev/ in lxc_container.c. Could do
DB> with a master list of device nodes used by both.
So does that mean they should be somewhere other than cgroup.h?
If not, I think it might be appropriate to make that change in a
separate patch, since populating /dev isn't really cgroup-related.
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
10:55 a.m.
New subject: [libvirt] [PATCH 2 of 2] Use cgroup functions to set resource limits on LXC domains
On Fri, Oct 03, 2008 at 08:45:00AM -0700, Dan Smith wrote:
DB> This list of devices is currently duplicated in two places -
once
DB> here where we set permissions, and again when we actually create
DB> the container and populate its /dev/ in lxc_container.c. Could do
DB> with a master list of device nodes used by both.
So does that mean they should be somewhere other than cgroup.h?
I'd think lxc_config.h or lcx_container.h would be a more natural
place for the constants, since both the .c files which would use
them as lxc_XXXX driver files.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
11:06 a.m.
New subject: [libvirt] [PATCH 2 of 2] Use cgroup functions to set resource limits on LXC domains
DB> I'd think lxc_config.h or lcx_container.h would be a more natural
DB> place for the constants, since both the .c files which would use
DB> them as lxc_XXXX driver files.
Okay, I moved them to lxc_container.h and updated the device numbers
in lxc_container.c to match.
Shall I spam the list with another set? :)
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
11:23 a.m.
New subject: [libvirt] [PATCH 2 of 2] Use cgroup functions to set resource limits on LXC domains
On Fri, Oct 03, 2008 at 09:06:49AM -0700, Dan Smith wrote:
DB> I'd think lxc_config.h or lcx_container.h would be a more
natural
DB> place for the constants, since both the .c files which would use
DB> them as lxc_XXXX driver files.
Okay, I moved them to lxc_container.h and updated the device numbers
in lxc_container.c to match.
Shall I spam the list with another set? :)
IMHO, no need, pushing to CVS and fixing things from there will be
simpler,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
11:46 a.m.
New subject: [libvirt] [PATCH 2 of 2] Use cgroup functions to set resource limits on LXC domains
DV> IMHO, no need, pushing to CVS and fixing things from there will be
DV> simpler,
Done, thanks!
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
8:07 a.m.
New subject: [libvirt] [PATCH 2 of 2] Use cgroup functions to set resource limits on LXC domains
On Fri, Oct 03, 2008 at 08:40:24AM -0700, Dan Smith wrote:
This patch adds code to the controller to set up a cgroup named after
the
domain name, set the memory limit, and restrict devices. It also
adds bits to lxc_driver to properly clean up the cgroup on domain death.
The device whitelisting is all very nice, but we completely forgot / ignored
the fact that there's nothing stopping a container mounting the cgroups
device controller and giving itself the device access we just took away :-)
The kernel code says
/*
* Modify the whitelist using allow/deny rules.
* CAP_SYS_ADMIN is needed for this. It's at least separate from CAP_MKNOD
* so we can give a container CAP_MKNOD to let it create devices but not
* modify the whitelist.
* It seems likely we'll want to add a CAP_CONTAINER capability to allow
* us to also grant CAP_SYS_ADMIN to containers without giving away the
* device whitelist controls, but for now we'll stick with CAP_SYS_ADMIN
*
* Taking rules away is always allowed (given CAP_SYS_ADMIN). Granting
* new access is only allowed if you're in the top-level cgroup, or your
* parent cgroup has the access you're asking for.
*/
So, looks like we need to explicitly set the capabilities of containers
to either mask out CAP_SYS_ADMIN from libvirtd's set, or construct an
explicit capability whitelist
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
10:16 a.m.
New subject: [libvirt] [PATCH 2 of 2] Use cgroup functions to set resource limits on LXC domains
DB> The device whitelisting is all very nice, but we completely forgot
DB> / ignored the fact that there's nothing stopping a container
DB> mounting the cgroups device controller and giving itself the
DB> device access we just took away :-)
Ah, interesting.
DB> So, looks like we need to explicitly set the capabilities of
DB> containers to either mask out CAP_SYS_ADMIN from libvirtd's set,
DB> or construct an explicit capability whitelist
Yeah, I guess so. I'll start looking into this :)
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
10:51 a.m.
New subject: [libvirt] [PATCH 2 of 2] Use cgroup functions to set resource limits on LXC domains
On Thu, Oct 16, 2008 at 02:07:20PM +0100, Daniel P. Berrange wrote:
On Fri, Oct 03, 2008 at 08:40:24AM -0700, Dan Smith wrote:
> This patch adds code to the controller to set up a cgroup named after the
> domain name, set the memory limit, and restrict devices. It also
> adds bits to lxc_driver to properly clean up the cgroup on domain death.
The device whitelisting is all very nice, but we completely forgot / ignored
the fact that there's nothing stopping a container mounting the cgroups
device controller and giving itself the device access we just took away :-)
The kernel code says
/*
* Modify the whitelist using allow/deny rules.
* CAP_SYS_ADMIN is needed for this. It's at least separate from CAP_MKNOD
* so we can give a container CAP_MKNOD to let it create devices but not
* modify the whitelist.
* It seems likely we'll want to add a CAP_CONTAINER capability to allow
* us to also grant CAP_SYS_ADMIN to containers without giving away the
* device whitelist controls, but for now we'll stick with CAP_SYS_ADMIN
*
* Taking rules away is always allowed (given CAP_SYS_ADMIN). Granting
* new access is only allowed if you're in the top-level cgroup, or your
* parent cgroup has the access you're asking for.
*/
That last paragraph actually suggest another possible aproach which won't
require messing with capabilities.
Consider a hierarchy of cgroups
$ROOT-CGROUP
|
...
|
+- libvirtd
|
+- lxc
|
+- $VM-NAME
+- $VM-NAME
+- $VM-NAME
libvirtd itself can be in any cgroup, beit the root one, or a child of
the root. If libvirtd is in the root it can grant whatever it likes to
cgroups for guests. If libvirt is not in root, then assuming its parent
has access to the device it can also grant devices as needed.
To prevent a LXC containre from giving itself device acess we need to
make sure either, it doesn't have CAP_SYS_ADMIN, or make sure its
parent cgroup doesn't have access. Fortunately we already have a cgroup
in the heirarchy between libvirtd's cgroup & the one the VM sits.
ie the cgroup named after the driver - 'lxc' in this case. No process
ever lives in this group - we're just using it for filesystem namespace
uniqueness.
So if that source code comment is correct, all we need todo is set a
deny-all rule in that intermediate 'lxc' cgroup, and then containers
will not be able to get access back, even if they have CAP_SYS_ADMIN
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
4 p.m.
New subject: [libvirt] [PATCH 2 of 2] Use cgroup functions to set resource limits on LXC domains
DB> So if that source code comment is correct, all we need todo is set
DB> a deny-all rule in that intermediate 'lxc' cgroup, and then
DB> containers will not be able to get access back, even if they have
DB> CAP_SYS_ADMIN
Even if I make the per-driver group have a deny-all policy, I can
still add arbitrary items to devices.allow and gain access from a
subgroup. So, I think we're going to need to restrict CAP_SYS_ADMIN
if we really want isolation, but I'm not sure what else that is likely
to break.
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
5881
days inactive
5894
days old
13 comments
3 participants
participants (3)
-
Dan Smith -
Daniel P. Berrange -
Daniel Veillard