Stash Xen config file after a failed startup

This is a fix for a minor bug.
Currently, a failed Xen VM start results in a stale config file left
behind on the filesystem.
This change introduces a new log directory, where the Xen VM config
file is moved after a failed startup.

Signed-off-by: Balazs Lecz <leczb@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

diff --git a/lib/hypervisor/hv_xen.py b/lib/hypervisor/hv_xen.py
index 1d0b587..daf0b3f 100644 (file)
--- a/lib/hypervisor/hv_xen.py
+++ b/lib/hypervisor/hv_xen.py
@@ -414,6 +414,17 @@ class XenHypervisor(hv_base.BaseHypervisor):
     """
     utils.RemoveFile(self._ConfigFileName(instance_name))
 
+  def _StashConfigFile(self, instance_name):
+    """Move the Xen config file to the log directory and return its new path.
+
+    """
+    old_filename = self._ConfigFileName(instance_name)
+    base = ("%s-%s" %
+            (instance_name, utils.TimestampForFilename()))
+    new_filename = utils.PathJoin(pathutils.LOG_XEN_DIR, base)
+    utils.RenameFile(old_filename, new_filename)
+    return new_filename
+
   def _GetXmList(self, include_node):
     """Wrapper around module level L{_GetXmList}.
 
@@ -482,9 +493,13 @@ class XenHypervisor(hv_base.BaseHypervisor):
 
     result = self._RunXen(cmd)
     if result.failed:
-      raise errors.HypervisorError("Failed to start instance %s: %s (%s)" %
+      # Move the Xen configuration file to the log directory to avoid
+      # leaving a stale config file behind.
+      stashed_config = self._StashConfigFile(instance.name)
+      raise errors.HypervisorError("Failed to start instance %s: %s (%s). Moved"
+                                   " config file to %s" %
                                    (instance.name, result.fail_reason,
-                                    result.output))
+                                    result.output, stashed_config))
 
   def StopInstance(self, instance, force=False, retry=False, name=None):
     """Stop an instance.

diff --git a/lib/pathutils.py b/lib/pathutils.py
index 540148d..341d1ba 100644 (file)
--- a/lib/pathutils.py
+++ b/lib/pathutils.py
@@ -133,6 +133,8 @@ QUERY_SOCKET = SOCKET_DIR + "/ganeti-query"
 
 LOG_OS_DIR = LOG_DIR + "/os"
 LOG_ES_DIR = LOG_DIR + "/extstorage"
+#: Directory for storing Xen config files after failed instance starts
+LOG_XEN_DIR = LOG_DIR + "/xen"
 
 # Job queue paths
 JOB_QUEUE_LOCK_FILE = QUEUE_DIR + "/lock"

diff --git a/lib/tools/ensure_dirs.py b/lib/tools/ensure_dirs.py
index d62414a..a8fc21a 100644 (file)
--- a/lib/tools/ensure_dirs.py
+++ b/lib/tools/ensure_dirs.py
@@ -195,6 +195,7 @@ def GetPaths():
     (noded_log, FILE, 0600, getent.noded_uid, getent.masterd_gid, False),
     (rapi_log, FILE, 0600, getent.rapi_uid, getent.masterd_gid, False),
     (pathutils.LOG_OS_DIR, DIR, 0750, getent.masterd_uid, getent.daemons_gid),
+    (pathutils.LOG_XEN_DIR, DIR, 0750, getent.noded_uid, getent.daemons_gid),
     (cleaner_log_dir, DIR, 0750, getent.noded_uid, getent.noded_gid),
     (master_cleaner_log_dir, DIR, 0750, getent.masterd_uid, getent.masterd_gid),
     (pathutils.INSTANCE_REASON_DIR, DIR, 0755, getent.noded_uid,

diff --git a/test/py/ganeti.hypervisor.hv_xen_unittest.py b/test/py/ganeti.hypervisor.hv_xen_unittest.py
index 3679de4..e87e62f 100755 (executable)
--- a/test/py/ganeti.hypervisor.hv_xen_unittest.py
+++ b/test/py/ganeti.hypervisor.hv_xen_unittest.py
@@ -30,6 +30,7 @@ import os
 
 from ganeti import constants
 from ganeti import objects
+from ganeti import pathutils
 from ganeti import hypervisor
 from ganeti import utils
 from ganeti import errors
@@ -491,7 +492,6 @@ class _TestXenHypervisor(object):
       self.fail("Unhandled command: %s" % (cmd, ))
 
     return self._SuccessCommand(output, cmd)
-    #return self._FailingCommand(cmd)
 
   def _MakeInstance(self):
     # Copy default parameters
@@ -519,6 +519,7 @@ class _TestXenHypervisor(object):
 
   def testStartInstance(self):
     (inst, disks) = self._MakeInstance()
+    pathutils.LOG_XEN_DIR = self.tmpdir
 
     for failcreate in [False, True]:
       for paused in [False, True]:
@@ -537,11 +538,12 @@ class _TestXenHypervisor(object):
         if failcreate:
           self.assertRaises(errors.HypervisorError, hv.StartInstance,
                             inst, disks, paused)
+          # Check whether a stale config file is left behind
+          self.assertFalse(os.path.exists(cfgfile))
         else:
           hv.StartInstance(inst, disks, paused)
-
-        # Check if configuration was updated
-        lines = utils.ReadFile(cfgfile).splitlines()
+          # Check if configuration was updated
+          lines = utils.ReadFile(cfgfile).splitlines()
 
         if constants.HV_VNC_PASSWORD_FILE in inst.hvparams:
           self.assertTrue(("vncpasswd = '%s'" % self.vncpw) in lines)