On Fri, May 17, 2013 at 11:14 AM, Guido Trotter <[email protected]>wrote:
> > > > On Fri, May 17, 2013 at 10:00 AM, Balazs Lecz <[email protected]> wrote: > >> 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 <[email protected]> >> --- >> lib/hypervisor/hv_xen.py | 19 +++++++++++++++++-- >> lib/pathutils.py | 2 ++ >> lib/tools/ensure_dirs.py | 1 + >> test/py/ganeti.hypervisor.hv_xen_unittest.py | 10 ++++++---- >> 4 files changed, 26 insertions(+), 6 deletions(-) >> >> diff --git a/lib/hypervisor/hv_xen.py b/lib/hypervisor/hv_xen.py >> index 1d0b587..d5273f4 100644 >> --- 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.log" % >> + (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 >> --- 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..e9dab95 100644 >> --- 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.masterd_uid, >> getent.daemons_gid), >> > > Why masterd and not noded here? > Good catch! I copied the line for the OS log dir, assuming it was correct. Turns out it isn't. Will fix that in a separate patch. Interdiff: diff --git a/lib/tools/ensure_dirs.py b/lib/tools/ensure_dirs.py index e9dab95..a8fc21a 100644 --- a/lib/tools/ensure_dirs.py +++ b/lib/tools/ensure_dirs.py @@ -195,7 +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.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, > Thanks, > > Guido > > -- Google Ireland Ltd.,1 Grand Canal Plaza, Lower Grand Canal Street, Dublin 4, Ireland Registered in Dublin, Ireland Registration Number: 368047
