Auto-detect existing cgroup placement

Use the new virCgroupNewDetect function to determine cgroup
placement of existing running VMs. This will allow the legacy
cgroups creation APIs to be removed entirely

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

src/lxc/lxc_cgroup.c

@@ -429,12 +429,12 @@ cleanup:
 }
 
 
-virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def, bool startup)
+virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def)
 {
     virCgroupPtr parent = NULL;
     virCgroupPtr cgroup = NULL;
 
-    if (!def->resource && startup) {
+    if (!def->resource) {
         virDomainResourceDefPtr res;
 
         if (VIR_ALLOC(res) < 0)
@@ -448,41 +448,26 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def, bool startup)
         def->resource = res;
     }
 
-    if (def->resource &&
+    if (def->resource->partition[0] != '/') {
-        def->resource->partition) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-        if (def->resource->partition[0] != '/') {
+                       _("Resource partition '%s' must start with '/'"),
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       def->resource->partition);
-                           _("Resource partition '%s' must start with '/'"),
+        goto cleanup;
-                           def->resource->partition);
-            goto cleanup;
-        }
-        /* We only auto-create the default partition. In other
-         * cases we expec the sysadmin/app to have done so */
-        if (virCgroupNewPartition(def->resource->partition,
-                                  STREQ(def->resource->partition, "/machine"),
-                                  -1,
-                                  &parent) < 0)
-            goto cleanup;
-
-        if (virCgroupNewDomainPartition(parent,
-                                        "lxc",
-                                        def->name,
-                                        true,
-                                        &cgroup) < 0)
-            goto cleanup;
-    } else {
-        if (virCgroupNewDriver("lxc",
-                               true,
-                               -1,
-                               &parent) < 0)
-            goto cleanup;
-
-        if (virCgroupNewDomainDriver(parent,
-                                     def->name,
-                                     true,
-                                     &cgroup) < 0)
-            goto cleanup;
     }
+    /* We only auto-create the default partition. In other
+     * cases we expect the sysadmin/app to have done so */
+    if (virCgroupNewPartition(def->resource->partition,
+                              STREQ(def->resource->partition, "/machine"),
+                              -1,
+                              &parent) < 0)
+        goto cleanup;
+
+    if (virCgroupNewDomainPartition(parent,
+                                    "lxc",
+                                    def->name,
+                                    true,
+                                    &cgroup) < 0)
+        goto cleanup;
 
 cleanup:
     virCgroupFree(&parent);
@@ -495,7 +480,7 @@ virCgroupPtr virLXCCgroupJoin(virDomainDefPtr def)
     virCgroupPtr cgroup = NULL;
     int ret = -1;
 
-    if (!(cgroup = virLXCCgroupCreate(def, true)))
+    if (!(cgroup = virLXCCgroupCreate(def)))
         return NULL;
 
     if (virCgroupAddTask(cgroup, getpid()) < 0)

src/lxc/lxc_cgroup.h

@@ -27,7 +27,7 @@
 # include "lxc_fuse.h"
 # include "virusb.h"
 
-virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def, bool startup);
+virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def);
 virCgroupPtr virLXCCgroupJoin(virDomainDefPtr def);
 int virLXCCgroupSetup(virDomainDefPtr def,
                       virCgroupPtr cgroup,

src/lxc/lxc_process.c

@@ -974,7 +974,7 @@ int virLXCProcessStart(virConnectPtr conn,
 
     virCgroupFree(&priv->cgroup);
 
-    if (!(priv->cgroup = virLXCCgroupCreate(vm->def, true)))
+    if (!(priv->cgroup = virLXCCgroupCreate(vm->def)))
         return -1;
 
     if (!virCgroupHasController(priv->cgroup,
@@ -1385,9 +1385,19 @@ virLXCProcessReconnectDomain(virDomainObjPtr vm,
         if (!(priv->monitor = virLXCProcessConnectMonitor(driver, vm)))
             goto error;
 
-        if (!(priv->cgroup = virLXCCgroupCreate(vm->def, false)))
+        if (virCgroupNewDetect(vm->pid, &priv->cgroup) < 0)
             goto error;
 
+        if (!virCgroupIsValidMachineGroup(priv->cgroup,
+                                          vm->def->name,
+                                          "lxc")) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Cgroup name is not valid for machine %s"),
+                           vm->def->name);
+            virCgroupFree(&priv->cgroup);
+            goto error;
+        }
+
         if (virLXCUpdateActiveUsbHostdevs(driver, vm->def) < 0)
             goto error;
 

src/qemu/qemu_cgroup.c

@@ -627,10 +627,9 @@ qemuSetupCpuCgroup(virDomainObjPtr vm)
 }
 
 
-int
+static int
 qemuInitCgroup(virQEMUDriverPtr driver,
-               virDomainObjPtr vm,
+               virDomainObjPtr vm)
-               bool startup)
 {
     int ret = -1;
     qemuDomainObjPrivatePtr priv = vm->privateData;
@@ -645,7 +644,7 @@ qemuInitCgroup(virQEMUDriverPtr driver,
 
     virCgroupFree(&priv->cgroup);
 
-    if (!vm->def->resource && startup) {
+    if (!vm->def->resource) {
         virDomainResourceDefPtr res;
 
         if (VIR_ALLOC(res) < 0)
@@ -659,49 +658,30 @@ qemuInitCgroup(virQEMUDriverPtr driver,
         vm->def->resource = res;
     }
 
-    if (vm->def->resource &&
+    if (vm->def->resource->partition[0] != '/') {
-        vm->def->resource->partition) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-        if (vm->def->resource->partition[0] != '/') {
+                       _("Resource partition '%s' must start with '/'"),
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       vm->def->resource->partition);
-                           _("Resource partition '%s' must start with '/'"),
+        goto cleanup;
-                           vm->def->resource->partition);
-            goto cleanup;
-        }
-        /* We only auto-create the default partition. In other
-         * cases we expec the sysadmin/app to have done so */
-        if (virCgroupNewPartition(vm->def->resource->partition,
-                                  STREQ(vm->def->resource->partition, "/machine"),
-                                  cfg->cgroupControllers,
-                                  &parent) < 0) {
-            if (virCgroupNewIgnoreError())
-                goto done;
-
-            goto cleanup;
-        }
-
-        if (virCgroupNewDomainPartition(parent,
-                                        "qemu",
-                                        vm->def->name,
-                                        true,
-                                        &priv->cgroup) < 0)
-            goto cleanup;
-    } else {
-        if (virCgroupNewDriver("qemu",
-                               true,
-                               cfg->cgroupControllers,
-                               &parent) < 0) {
-            if (virCgroupNewIgnoreError())
-                goto done;
-
-            goto cleanup;
-        }
-
-        if (virCgroupNewDomainDriver(parent,
-                                     vm->def->name,
-                                     true,
-                                     &priv->cgroup) < 0)
-            goto cleanup;
     }
+    /* We only auto-create the default partition. In other
+     * cases we expect the sysadmin/app to have done so */
+    if (virCgroupNewPartition(vm->def->resource->partition,
+                              STREQ(vm->def->resource->partition, "/machine"),
+                              cfg->cgroupControllers,
+                              &parent) < 0) {
+        if (virCgroupNewIgnoreError())
+            goto done;
+
+        goto cleanup;
+    }
+
+    if (virCgroupNewDomainPartition(parent,
+                                    "qemu",
+                                    vm->def->name,
+                                    true,
+                                    &priv->cgroup) < 0)
+        goto cleanup;
 
 done:
     ret = 0;
@@ -712,6 +692,43 @@ cleanup:
 }
 
 
+int
+qemuConnectCgroup(virQEMUDriverPtr driver,
+                  virDomainObjPtr vm)
+{
+    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    int ret = -1;
+
+    if (!cfg->privileged)
+        goto done;
+
+    if (!virCgroupAvailable())
+        goto done;
+
+    virCgroupFree(&priv->cgroup);
+
+    if (virCgroupNewDetect(vm->pid, &priv->cgroup) < 0) {
+        if (virCgroupNewIgnoreError())
+            goto done;
+        goto cleanup;
+    }
+
+    if (!virCgroupIsValidMachineGroup(priv->cgroup,
+                                      vm->def->name,
+                                      "qemu")) {
+        VIR_DEBUG("Cgroup name is not valid for machine");
+        virCgroupFree(&priv->cgroup);
+        goto done;
+    }
+
+done:
+    ret = 0;
+cleanup:
+    virObjectUnref(cfg);
+    return ret;
+}
+
 int
 qemuSetupCgroup(virQEMUDriverPtr driver,
                 virDomainObjPtr vm,
@@ -721,7 +738,7 @@ qemuSetupCgroup(virQEMUDriverPtr driver,
     virCapsPtr caps = NULL;
     int ret = -1;
 
-    if (qemuInitCgroup(driver, vm, true) < 0)
+    if (qemuInitCgroup(driver, vm) < 0)
         return -1;
 
     if (!priv->cgroup)

src/qemu/qemu_cgroup.h

@@ -39,9 +39,8 @@ int qemuSetupHostdevCGroup(virDomainObjPtr vm,
 int qemuTeardownHostdevCgroup(virDomainObjPtr vm,
                               virDomainHostdevDefPtr dev)
    ATTRIBUTE_RETURN_CHECK;
-int qemuInitCgroup(virQEMUDriverPtr driver,
+int qemuConnectCgroup(virQEMUDriverPtr driver,
-                   virDomainObjPtr vm,
+                      virDomainObjPtr vm);
-                   bool startup);
 int qemuSetupCgroup(virQEMUDriverPtr driver,
                     virDomainObjPtr vm,
                     virBitmapPtr nodemask);

src/qemu/qemu_process.c

@@ -3070,7 +3070,7 @@ qemuProcessReconnect(void *opaque)
     if (qemuUpdateActiveScsiHostdevs(driver, obj->def) < 0)
         goto error;
 
-    if (qemuInitCgroup(driver, obj, false) < 0)
+    if (qemuConnectCgroup(driver, obj) < 0)
         goto error;
 
     /* XXX: Need to change as long as lock is introduced for