lxc: Let the driver detect CGroups earlier

This is the bug I'm facing. I deliberately configured a container
so that the source of a <filesystem/> to passthrough doesn't
exist. The start fails with:

  lxcContainerPivotRoot:669 : Failed to create /non-existent/path/.oldroot: Permission denied

which is expected. But what is NOT expected is that CGroup
hierarchy is left behind. This is because the controller sets up
the CGroup hierarchy, user namespace, moves interfaces, etc. and
finally checks whether container setup (done in a separate
process) succeeded. Only after all this the error is propagated
to the LXC driver. The driver aborts the startup and tries to
perform the cleanup, but this is missing CGroups because those
weren't detected yet.

Ideally, whenever a function fails, it tries to unroll back so
that is has no artifacts left behind (look at all those frees/FD
closes/etc. at end of functions). But with CGroups it is
different - the controller process can't clean up after itself,
because it is still running inside that CGroup.

Therefore, what we have to do is to let the driver detect CGroups
as soon as they are created, and proceed with controller
execution only after that.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 8f0ece98cdc90da4cdfcf2e1266221943bdf7114..066e013ed47b4aec90e93b3283a161d6552d70e1 100644 (file)
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -348,7 +348,7 @@ static int virLXCControllerConsoleSetNonblocking(virLXCControllerConsole *consol
 }
 
 
-static int virLXCControllerDaemonHandshake(virLXCController *ctrl)
+static int virLXCControllerDaemonHandshakeCont(virLXCController *ctrl)
 {
     if (lxcContainerSendContinue(ctrl->handshakeFds[1]) < 0) {
         virReportSystemError(errno, "%s",
@@ -358,6 +358,15 @@ static int virLXCControllerDaemonHandshake(virLXCController *ctrl)
     return 0;
 }
 
+static int virLXCControllerDaemonHandshakeWait(virLXCController *ctrl)
+{
+    if (lxcContainerWaitForContinue(ctrl->handshakeFds[0]) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("error waiting for continue signal from daemon"));
+        return -1;
+    }
+    return 0;
+}
 
 static int virLXCControllerValidateNICs(virLXCController *ctrl)
 {
@@ -2372,6 +2381,11 @@ virLXCControllerRun(virLXCController *ctrl)
     if (virLXCControllerSetupCgroupLimits(ctrl) < 0)
         goto cleanup;
 
+    /* Allow daemon to detect CGroups. */
+    if (virLXCControllerDaemonHandshakeCont(ctrl) < 0 ||
+        virLXCControllerDaemonHandshakeWait(ctrl) < 0)
+        goto cleanup;
+
     if (virLXCControllerSetupUserns(ctrl) < 0)
         goto cleanup;
 
@@ -2401,7 +2415,8 @@ virLXCControllerRun(virLXCController *ctrl)
         if (virLXCControllerConsoleSetNonblocking(&(ctrl->consoles[i])) < 0)
             goto cleanup;
 
-    if (virLXCControllerDaemonHandshake(ctrl) < 0)
+    /* Allow daemon to connect to the monitor. */
+    if (virLXCControllerDaemonHandshakeCont(ctrl) < 0)
         goto cleanup;
 
     /* and preemptively close handshakeFds */

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index dd51c778a4ace289d2090889c2a647d18422b69d..cfa009f14e4965297f5eeaabb4df5f9a1a43c36a 100644 (file)
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1473,6 +1473,7 @@ int virLXCProcessStart(virConnectPtr conn,
     if (g_atomic_int_add(&driver->nactive, 1) == 0 && driver->inhibitCallback)
         driver->inhibitCallback(true, driver->inhibitOpaque);
 
+    /* The first synchronization point is when the controller creates CGroups. */
     if (lxcContainerWaitForContinue(handshakefds[0]) < 0) {
         char out[1024];
 
@@ -1504,6 +1505,25 @@ int virLXCProcessStart(virConnectPtr conn,
         goto cleanup;
     }
 
+    if (lxcContainerSendContinue(handshakefds[3]) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("Failed to send continue signal to controller"));
+        goto cleanup;
+    }
+
+    /* The second synchronization point is when the controller finished
+     * creating the container. */
+    if (lxcContainerWaitForContinue(handshakefds[0]) < 0) {
+        char out[1024];
+
+        if (!(virLXCProcessReadLogOutput(vm, logfile, pos, out, 1024) < 0)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("guest failed to start: %s"), out);
+        }
+
+        goto cleanup;
+    }
+
     /* And we can get the first monitor connection now too */
     if (!(priv->monitor = virLXCProcessConnectMonitor(driver, vm))) {
         /* Intentionally overwrite the real monitor error message,