[libvirt] [PATCH 5/8] lxc: only write XML once for lxc controller

[Daniel P. Berrange]

Currently when launching the LXC controller we first write out
the plain, inactive XML configuration, then launch the controller,
then replace the file with the live status XML configuration.
By good fortune this hasn't caused any problems other than some
misleading error messages during failure scenarios.

This simplifies the code so it only writes out the XML once and
always writes the live status XML. To do this we need to handshake
with the child process, to make execution pause just before exec()
so we can write the XML status with the child PID present.
---
 src/conf/domain_conf.c   |  2 +-
 src/conf/domain_conf.h   |  5 +++++
 src/lxc/lxc_controller.c | 12 +++++++-----
 src/lxc/lxc_process.c    | 33 ++++++++++++++++++++-------------
 4 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8792f5e..aaea685 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14634,7 +14634,7 @@ virDomainObjParseNode(xmlDocPtr xml,
 }
 
 
-static virDomainObjPtr
+virDomainObjPtr
 virDomainObjParseFile(const char *filename,
                       virCapsPtr caps,
                       virDomainXMLOptionPtr xmlopt,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 09ab194..563c18b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2489,6 +2489,11 @@ virDomainDefPtr virDomainDefParseNode(xmlDocPtr doc,
                                       virDomainXMLOptionPtr xmlopt,
                                       unsigned int expectedVirtTypes,
                                       unsigned int flags);
+virDomainObjPtr virDomainObjParseFile(const char *filename,
+                                      virCapsPtr caps,
+                                      virDomainXMLOptionPtr xmlopt,
+                                      unsigned int expectedVirtTypes,
+                                      unsigned int flags);
 
 bool virDomainDefCheckABIStability(virDomainDefPtr src,
                                    virDomainDefPtr dst);
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 53a2c8d..f2c0b57 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -100,6 +100,7 @@ typedef struct _virLXCController virLXCController;
 typedef virLXCController *virLXCControllerPtr;
 struct _virLXCController {
     char *name;
+    virDomainObjPtr vm;
     virDomainDefPtr def;
 
     int handshakeFd;
@@ -175,11 +176,12 @@ static virLXCControllerPtr virLXCControllerNew(const char *name)
                                           ctrl->name)) == NULL)
         goto error;
 
-    if ((ctrl->def = virDomainDefParseFile(configFile,
-                                           caps, xmlopt,
-                                           1 << VIR_DOMAIN_VIRT_LXC,
-                                           0)) == NULL)
+    if ((ctrl->vm = virDomainObjParseFile(configFile,
+                                          caps, xmlopt,
+                                          1 << VIR_DOMAIN_VIRT_LXC,
+                                          0)) == NULL)
         goto error;
+    ctrl->def = ctrl->vm->def;
 
     if ((ctrl->timerShutdown = virEventAddTimeout(-1,
                                                   virLXCControllerQuitTimer, ctrl,
@@ -269,7 +271,7 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl)
 
     VIR_FREE(ctrl->devptmx);
 
-    virDomainDefFree(ctrl->def);
+    virObjectUnref(ctrl->vm);
     VIR_FREE(ctrl->name);
 
     if (ctrl->timerShutdown != -1)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 764cdab..e2b2d30 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -823,6 +823,9 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
     virCommandSetPidFile(cmd, pidfile);
     virCommandSetOutputFD(cmd, &logfd);
     virCommandSetErrorFD(cmd, &logfd);
+    /* So we can pause before exec'ing the controller to
+     * write the live domain status XML with the PID */
+    virCommandRequireHandshake(cmd);
 
     return cmd;
  cleanup:
@@ -1171,10 +1174,6 @@ int virLXCProcessStart(virConnectPtr conn,
     if (virLXCProcessSetupInterfaces(conn, vm->def, &nveths, &veths) < 0)
         goto cleanup;
 
-    /* Save the configuration for the controller */
-    if (virDomainSaveConfig(cfg->stateDir, vm->def) < 0)
-        goto cleanup;
-
     if ((logfd = open(logfile, O_WRONLY | O_APPEND | O_CREAT,
              S_IRUSR|S_IWUSR)) < 0) {
         virReportSystemError(errno,
@@ -1273,6 +1272,23 @@ int virLXCProcessStart(virConnectPtr conn,
         goto error;
     }
 
+    if (VIR_CLOSE(handshakefds[1]) < 0) {
+        virReportSystemError(errno, "%s", _("could not close handshake fd"));
+        goto error;
+    }
+
+    if (virCommandHandshakeWait(cmd) < 0)
+        goto error;
+
+    /* Write domain status to disk for the controller to
+     * read when it starts */
+    if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
+        goto error;
+
+    /* Allow the child to exec the controller */
+    if (virCommandHandshakeNotify(cmd) < 0)
+        goto error;
+
     if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)
         driver->inhibitCallback(true, driver->inhibitOpaque);
 
@@ -1330,15 +1346,6 @@ int virLXCProcessStart(virConnectPtr conn,
     /* We don't need the temporary NIC names anymore, clear them */
     virLXCProcessCleanInterfaces(vm->def);
 
-    /* Write domain status to disk.
-     *
-     * XXX: Earlier we wrote the plain "live" domain XML to this
-     * location for the benefit of libvirt_lxc. We're now overwriting
-     * it with the live status XML instead. This is a (currently
-     * harmless) inconsistency we should fix one day */
-    if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
-        goto error;
-
     /* finally we can call the 'started' hook script if any */
     if (virHookPresent(VIR_HOOK_DRIVER_LXC)) {
         char *xml = virDomainDefFormat(vm->def, 0);
-- 
2.1.0