Make QEMU cgroups use configurable

* qemud/libvirtd_qemu.aug, qemud/test_libvirtd_qemu.aug,
   src/qemu.conf: Add 'cgroups_controllers' and 'cgroups_device_acl'
   parameters
 * src/qemu_conf.h, src/qemu_conf.c: Load & parse configuration params
   for cgroups
 * src/qemu_driver.c: Only use cgroups controllers that are activated,
   and use configured device whitelist instead of default, if set.

qemud/libvirtd_qemu.aug

@@ -31,6 +31,8 @@ module Libvirtd_qemu =
                  | str_entry "vnc_sasl_dir"
                  | str_entry "user"
                  | str_entry "group"
+                 | str_array_entry "cgroup_controllers"
+                 | str_array_entry "cgroup_device_acl"
 
    (* Each enty in the config is one of the following three ... *)
    let entry = vnc_entry

qemud/test_libvirtd_qemu.aug

@@ -83,6 +83,10 @@ vnc_sasl_dir = \"/some/directory/sasl2\"
 user = \"root\"
 
 group = \"root\"
+
+cgroup_controllers = [ \"cpu\", \"devices\" ]
+
+cgroup_device_acl = [ \"/dev/null\", \"/dev/full\", \"/dev/zero\" ]
 "
 
    test Libvirtd_qemu.lns get conf =
@@ -165,7 +169,18 @@ group = \"root\"
 { "#comment" = "point to the directory, and create a qemu.conf in that location" }
 { "#comment" = "" }
 { "vnc_sasl_dir" = "/some/directory/sasl2" }
-{ "#comment" = "" }
+{ "#empty" }
 { "user"= "root" }
-{ "#comment" = "" }
+{ "#empty" }
-{ "group" = "root" }
+{ "group" = "root" }
+{ "#empty" }
+{ "cgroup_controllers"
+    { "1" = "cpu" }
+    { "2" = "devices" }
+}
+{ "#empty" }
+{ "cgroup_device_acl"
+    { "1" = "/dev/null" }
+    { "2" = "/dev/full" }
+    { "3" = "/dev/zero" }
+}

src/Makefile.am

@@ -142,7 +142,8 @@ VBOX_DRIVER_EXTRA_DIST = vbox/vbox_tmpl.c vbox/README
 
 QEMU_DRIVER_SOURCES =						\
 		qemu_conf.c qemu_conf.h				\
-		qemu_driver.c qemu_driver.h
+		qemu_driver.c qemu_driver.h			\
+		cgroup.c cgroup.h
 
 UML_DRIVER_SOURCES =						\
 		uml_conf.c uml_conf.h				\

src/cgroup.c

@@ -31,17 +31,6 @@
 
 #define CGROUP_MAX_VAL 512
 
-enum {
-    VIR_CGROUP_CONTROLLER_CPU,
-    VIR_CGROUP_CONTROLLER_CPUACCT,
-    VIR_CGROUP_CONTROLLER_CPUSET,
-    VIR_CGROUP_CONTROLLER_MEMORY,
-    VIR_CGROUP_CONTROLLER_DEVICES,
-
-    VIR_CGROUP_CONTROLLER_LAST
-};
-
-VIR_ENUM_DECL(virCgroupController);
 VIR_ENUM_IMPL(virCgroupController, VIR_CGROUP_CONTROLLER_LAST,
               "cpu", "cpuacct", "cpuset", "memory", "devices");
 

src/cgroup.h

@@ -15,6 +15,18 @@
 struct virCgroup;
 typedef struct virCgroup *virCgroupPtr;
 
+enum {
+    VIR_CGROUP_CONTROLLER_CPU,
+    VIR_CGROUP_CONTROLLER_CPUACCT,
+    VIR_CGROUP_CONTROLLER_CPUSET,
+    VIR_CGROUP_CONTROLLER_MEMORY,
+    VIR_CGROUP_CONTROLLER_DEVICES,
+
+    VIR_CGROUP_CONTROLLER_LAST
+};
+
+VIR_ENUM_DECL(virCgroupController);
+
 int virCgroupForDriver(const char *name,
                        virCgroupPtr *group,
                        int privileged,

src/qemu.conf

@@ -95,3 +95,37 @@
 
 # The group ID for QEMU processes run by the system instance
 #group = "root"
+
+
+# What cgroup controllers to make use of with QEMU guests
+#
+#  - 'cpu' - use for schedular tunables
+#  - 'devices' - use for device whitelisting
+#
+# NB, even if configured here, they won't be used unless
+# the adminsitrator has mounted cgroups. eg
+#
+#  mkdir /dev/cgroup
+#  mount -t cgroup -o devices,cpu none /dev/cgroup
+#
+# They can be mounted anywhere, and different controlers
+# can be mounted in different locations. libvirt will detect
+# where they are located.
+#
+# cgroup_controllers = [ "cpu", "devices" ]
+
+# This is the basic set of devices allowed / required by
+# all virtual machines.
+#
+# As well as this, any configured block backed disks,
+# all sound device, and all PTY devices are allowed.
+#
+# This will only need setting if newer QEMU suddenly
+# wants some device we don't already know a bout.
+#
+#cgroup_device_acl = [
+#    "/dev/null", "/dev/full", "/dev/zero",
+#    "/dev/random", "/dev/urandom",
+#    "/dev/ptmx", "/dev/kvm", "/dev/kqemu",
+#    "/dev/rtc", "/dev/hpet", "/dev/net/tun",
+#]

src/qemu_conf.c

@@ -94,6 +94,7 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
     virConfValuePtr p;
     char *user;
     char *group;
+    int i;
 
     /* Setup 2 critical defaults */
     if (!(driver->vncListen = strdup("127.0.0.1"))) {
@@ -218,6 +219,66 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
     }
     VIR_FREE(group);
 
+    p = virConfGetValue (conf, "cgroup_controllers");
+    CHECK_TYPE ("cgroup_controllers", VIR_CONF_LIST);
+    if (p) {
+        virConfValuePtr pp;
+        for (i = 0, pp = p->list; pp; ++i, pp = pp->next) {
+            int ctl;
+            if (pp->type != VIR_CONF_STRING) {
+                VIR_ERROR("%s", _("cgroup_device_acl must be a list of strings"));
+                virConfFree(conf);
+                return -1;
+            }
+            ctl = virCgroupControllerTypeFromString(pp->str);
+            if (ctl < 0) {
+                VIR_ERROR("Unknown cgroup controller '%s'", pp->str);
+                virConfFree(conf);
+                return -1;
+            }
+            driver->cgroupControllers |= (1 << ctl);
+        }
+    } else {
+        driver->cgroupControllers =
+            (1 << VIR_CGROUP_CONTROLLER_CPU) |
+            (1 << VIR_CGROUP_CONTROLLER_DEVICES);
+    }
+    for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) {
+        if (driver->cgroupControllers & (1 << i)) {
+            VIR_INFO("Configured cgroup controller '%s'",
+                     virCgroupControllerTypeToString(i));
+        }
+    }
+
+    p = virConfGetValue (conf, "cgroup_device_acl");
+    CHECK_TYPE ("cgroup_device_acl", VIR_CONF_LIST);
+    if (p) {
+        int len = 0;
+        virConfValuePtr pp;
+        for (pp = p->list; pp; pp = pp->next)
+            len++;
+        if (VIR_ALLOC_N(driver->cgroupDeviceACL, 1+len) < 0) {
+            virReportOOMError(NULL);
+            virConfFree(conf);
+            return -1;
+        }
+        for (i = 0, pp = p->list; pp; ++i, pp = pp->next) {
+            if (pp->type != VIR_CONF_STRING) {
+                VIR_ERROR("%s", _("cgroup_device_acl must be a list of strings"));
+                virConfFree(conf);
+                return -1;
+            }
+            driver->cgroupDeviceACL[i] = strdup (pp->str);
+            if (driver->cgroupDeviceACL[i] == NULL) {
+                virReportOOMError(NULL);
+                virConfFree(conf);
+                return -1;
+            }
+
+        }
+        driver->cgroupDeviceACL[i] = NULL;
+    }
+
     virConfFree (conf);
     return 0;
 }

src/qemu_conf.h

@@ -79,6 +79,9 @@ struct qemud_driver {
     int nextvmid;
 
     virCgroupPtr cgroup;
+    int cgroupControllers;
+    char **cgroupDeviceACL;
+
     virDomainObjList domains;
 
     brControl *brctl;

src/qemu_driver.c

@@ -125,6 +125,15 @@ static int qemudDetectVcpuPIDs(virConnectPtr conn,
 
 static struct qemud_driver *qemu_driver = NULL;
 
+static int qemuCgroupControllerActive(struct qemud_driver *driver,
+                                      int controller)
+{
+    if (driver->cgroup == NULL)
+        return 0;
+    if (driver->cgroupControllers & (1 << controller))
+        return 1;
+    return 0;
+}
 
 static int
 qemudLogFD(virConnectPtr conn, struct qemud_driver *driver, const char* name)
@@ -1405,7 +1414,10 @@ static int qemuSetupCgroup(virConnectPtr conn,
     virCgroupPtr cgroup = NULL;
     int rc;
     unsigned int i;
-    const char *const *deviceACL = defaultDeviceACL;
+    const char *const *deviceACL =
+        driver->cgroupDeviceACL ?
+        (const char *const *)driver->cgroupDeviceACL :
+        defaultDeviceACL;
 
     if (driver->cgroup == NULL)
         return 0; /* Not supported, so claim success */
@@ -1418,58 +1430,60 @@ static int qemuSetupCgroup(virConnectPtr conn,
         goto cleanup;
     }
 
-    rc = virCgroupDenyAllDevices(cgroup);
+    if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) {
-    if (rc != 0) {
+        rc = virCgroupDenyAllDevices(cgroup);
-        if (rc == -EPERM) {
-            VIR_WARN0("Group devices ACL is not accessible, disabling whitelisting");
-            goto done;
-        }
-
-        virReportSystemError(conn, -rc,
-                             _("Unable to deny all devices for %s"), vm->def->name);
-        goto cleanup;
-    }
-
-    for (i = 0; i < vm->def->ndisks ; i++) {
-        if (vm->def->disks[i]->type != VIR_DOMAIN_DISK_TYPE_BLOCK ||
-            vm->def->disks[i]->src == NULL)
-            continue;
-
-        rc = virCgroupAllowDevicePath(cgroup,
-                                      vm->def->disks[i]->src);
         if (rc != 0) {
+            if (rc == -EPERM) {
+                VIR_WARN0("Group devices ACL is not accessible, disabling whitelisting");
+                goto done;
+            }
+
             virReportSystemError(conn, -rc,
-                                 _("Unable to allow device %s for %s"),
+                                 _("Unable to deny all devices for %s"), vm->def->name);
-                                 vm->def->disks[i]->src, vm->def->name);
             goto cleanup;
         }
-    }
 
-    rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR);
+        for (i = 0; i < vm->def->ndisks ; i++) {
-    if (rc != 0) {
+            if (vm->def->disks[i]->type != VIR_DOMAIN_DISK_TYPE_BLOCK ||
-        virReportSystemError(conn, -rc, "%s",
+                vm->def->disks[i]->src == NULL)
-                             _("unable to allow /dev/pts/ devices"));
+                continue;
-        goto cleanup;
-    }
 
-    if (vm->def->nsounds) {
+            rc = virCgroupAllowDevicePath(cgroup,
-        rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR);
+                                          vm->def->disks[i]->src);
+            if (rc != 0) {
+                virReportSystemError(conn, -rc,
+                                     _("Unable to allow device %s for %s"),
+                                     vm->def->disks[i]->src, vm->def->name);
+                goto cleanup;
+            }
+        }
+
+        rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR);
         if (rc != 0) {
             virReportSystemError(conn, -rc, "%s",
-                                 _("unable to allow /dev/snd/ devices"));
+                                 _("unable to allow /dev/pts/ devices"));
             goto cleanup;
         }
-    }
 
-    for (i = 0; deviceACL[i] != NULL ; i++) {
+        if (vm->def->nsounds) {
-        rc = virCgroupAllowDevicePath(cgroup,
+            rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR);
-                                      deviceACL[i]);
+            if (rc != 0) {
-        if (rc < 0 &&
+                virReportSystemError(conn, -rc, "%s",
-            rc != -ENOENT) {
+                                     _("unable to allow /dev/snd/ devices"));
-            virReportSystemError(conn, -rc,
+                goto cleanup;
-                                 _("unable to allow device %s"),
+            }
-                                 deviceACL[i]);
+        }
-            goto cleanup;
+
+        for (i = 0; deviceACL[i] != NULL ; i++) {
+            rc = virCgroupAllowDevicePath(cgroup,
+                                          deviceACL[i]);
+            if (rc < 0 &&
+                rc != -ENOENT) {
+                virReportSystemError(conn, -rc,
+                                     _("unable to allow device %s"),
+                                     deviceACL[i]);
+                goto cleanup;
+            }
         }
     }
 
@@ -4934,7 +4948,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
         goto cleanup;
 
     if (dev->type == VIR_DOMAIN_DEVICE_DISK) {
-        if (driver->cgroup != NULL) {
+        if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) {
             if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) !=0 ) {
                 qemudReportError(dom->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
                                  _("Unable to find cgroup for %s\n"),
@@ -5379,7 +5393,7 @@ static char *qemuGetSchedulerType(virDomainPtr dom,
     struct qemud_driver *driver = dom->conn->privateData;
     char *ret;
 
-    if (driver->cgroup == NULL) {
+    if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) {
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
                          __FUNCTION__);
         return NULL;
@@ -5404,7 +5418,7 @@ static int qemuSetSchedulerParameters(virDomainPtr dom,
     virDomainObjPtr vm = NULL;
     int ret = -1;
 
-    if (driver->cgroup == NULL) {
+    if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) {
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
                          __FUNCTION__);
         return -1;
@@ -5469,7 +5483,7 @@ static int qemuGetSchedulerParameters(virDomainPtr dom,
     int ret = -1;
     int rc;
 
-    if (driver->cgroup == NULL) {
+    if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) {
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
                          __FUNCTION__);
         return -1;