Create + setup cgroups atomically for LXC process

Currently the LXC driver creates the VM's cgroup prior to
forking, and then libvirt_lxc moves the child process
into the cgroup. This won't work with systemd whose APIs
do the creation of cgroups + attachment of processes atomically.

Fortunately we simply move the entire cgroups setup into
the libvirt_lxc child process. We make it take place before
fork'ing into the background, so by the time virCommandRun
returns in the LXC driver, the cgroup is guaranteed to be
present.

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

diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 41d69b321f7b44eb5cf28aef800f0341efaab037..bbec34422b5cfe091f1f084486d68aa6d5a6d0bd 100644 (file)
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -128,6 +128,8 @@ struct _virLXCController {
     bool inShutdown;
     int timerShutdown;
 
+    virCgroupPtr cgroup;
+
     virLXCFusePtr fuse;
 };
 
@@ -275,6 +277,8 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl)
     virObjectUnref(ctrl->server);
     virLXCControllerFreeFuse(ctrl);
 
+    virCgroupFree(&ctrl->cgroup);
+
     /* This must always be the last thing to be closed */
     VIR_FORCE_CLOSE(ctrl->handshakeFd);
     VIR_FREE(ctrl);
@@ -657,8 +661,7 @@ cleanup:
  *
  * Returns 0 on success or -1 in case of error
  */
-static int virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl,
-                                               virCgroupPtr cgroup)
+static int virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl)
 {
     virBitmapPtr nodemask = NULL;
     int ret = -1;
@@ -670,7 +673,7 @@ static int virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl,
     if (virLXCControllerSetupCpuAffinity(ctrl) < 0)
         goto cleanup;
 
-    if (virLXCCgroupSetup(ctrl->def, cgroup, nodemask) < 0)
+    if (virLXCCgroupSetup(ctrl->def, ctrl->cgroup, nodemask) < 0)
         goto cleanup;
 
     ret = 0;
@@ -2102,7 +2105,6 @@ virLXCControllerRun(virLXCControllerPtr ctrl)
     int containerhandshake[2] = { -1, -1 };
     char **containerTTYPaths = NULL;
     size_t i;
-    virCgroupPtr cgroup = NULL;
 
     if (VIR_ALLOC_N(containerTTYPaths, ctrl->nconsoles) < 0)
         goto cleanup;
@@ -2122,13 +2124,10 @@ virLXCControllerRun(virLXCControllerPtr ctrl)
     if (virLXCControllerSetupPrivateNS() < 0)
         goto cleanup;
 
-    if (!(cgroup = virLXCCgroupJoin(ctrl->def)))
-        goto cleanup;
-
     if (virLXCControllerSetupLoopDevices(ctrl) < 0)
         goto cleanup;
 
-    if (virLXCControllerSetupResourceLimits(ctrl, cgroup) < 0)
+    if (virLXCControllerSetupResourceLimits(ctrl) < 0)
         goto cleanup;
 
     if (virLXCControllerSetupDevPTS(ctrl) < 0)
@@ -2214,7 +2213,6 @@ cleanup:
         VIR_FREE(containerTTYPaths[i]);
     VIR_FREE(containerTTYPaths);
 
-    virCgroupFree(&cgroup);
     virLXCControllerStopInit(ctrl);
 
     return rc;
@@ -2390,6 +2388,9 @@ int main(int argc, char *argv[])
     if (virLXCControllerValidateConsoles(ctrl) < 0)
         goto cleanup;
 
+    if (!(ctrl->cgroup = virLXCCgroupJoin(ctrl->def)))
+        goto cleanup;
+
     if (virLXCControllerSetupServer(ctrl) < 0)
         goto cleanup;
 

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 36429455ceacbfe91f5f3cd62fc69d4c58562409..02ac5d44d4ade589ec7b931b38bd89a4f8b6d9f3 100644 (file)
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -49,6 +49,7 @@
 #include "virhook.h"
 #include "virstring.h"
 #include "viratomic.h"
+#include "virprocess.h"
 
 #define VIR_FROM_THIS VIR_FROM_LXC
 
@@ -701,9 +702,9 @@ int virLXCProcessStop(virLXCDriverPtr driver,
             return -1;
         }
     } else {
-        /* If cgroup doesn't exist, the VM pids must have already
-         * died and so we're just cleaning up stale state
-         */
+        /* If cgroup doesn't exist, just try cleaning up the
+         * libvirt_lxc process */
+        virProcessKillPainfully(vm->pid, true);
     }
 
     virLXCProcessCleanup(driver, vm, reason);
@@ -971,33 +972,33 @@ int virLXCProcessStart(virConnectPtr conn,
     virCapsPtr caps = NULL;
     virErrorPtr err = NULL;
     virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
+    virCgroupPtr selfcgroup;
 
-    virCgroupFree(&priv->cgroup);
-
-    if (!(priv->cgroup = virLXCCgroupCreate(vm->def)))
+    if (virCgroupNewSelf(&selfcgroup) < 0)
         return -1;
 
-    if (!virCgroupHasController(priv->cgroup,
+    if (!virCgroupHasController(selfcgroup,
                                 VIR_CGROUP_CONTROLLER_CPUACCT)) {
-        virCgroupFree(&priv->cgroup);
+        virCgroupFree(&selfcgroup);
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Unable to find 'cpuacct' cgroups controller mount"));
         return -1;
     }
-    if (!virCgroupHasController(priv->cgroup,
+    if (!virCgroupHasController(selfcgroup,
                                 VIR_CGROUP_CONTROLLER_DEVICES)) {
-        virCgroupFree(&priv->cgroup);
+        virCgroupFree(&selfcgroup);
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Unable to find 'devices' cgroups controller mount"));
         return -1;
     }
-    if (!virCgroupHasController(priv->cgroup,
+    if (!virCgroupHasController(selfcgroup,
                                 VIR_CGROUP_CONTROLLER_MEMORY)) {
-        virCgroupFree(&priv->cgroup);
+        virCgroupFree(&selfcgroup);
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Unable to find 'memory' cgroups controller mount"));
         return -1;
     }
+    virCgroupFree(&selfcgroup);
 
     if (virFileMakePath(cfg->logDir) < 0) {
         virReportSystemError(errno,
@@ -1170,7 +1171,7 @@ int virLXCProcessStart(virConnectPtr conn,
 
     /* Connect to the controller as a client *first* because
      * this will block until the child has written their
-     * pid file out to disk */
+     * pid file out to disk & created their cgroup */
     if (!(priv->monitor = virLXCProcessConnectMonitor(driver, vm)))
         goto cleanup;
 
@@ -1188,6 +1189,19 @@ int virLXCProcessStart(virConnectPtr conn,
         goto cleanup;
     }
 
+    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;
+    }
+
     priv->stopReason = VIR_DOMAIN_EVENT_STOPPED_FAILED;
     priv->wantReboot = false;
     vm->def->id = vm->pid;