From d00a730d2cd8294e06c46274a94cf6ace4dee4b1 Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Fri, 19 Aug 2011 11:11:19 +0200
Subject: [PATCH] ensure-dirs: Refine error handling on stat(2)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The β€œ_stat_fn” function is renamed to β€œ_lstat_fn” to reflect its
function. The try/except block just wraps calling lstat(2) and nothing
else.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: RenΓ© Nussbaumer <rn@google.com>
---
 lib/tools/ensure_dirs.py                  | 19 +++++++++----------
 test/ganeti.tools.ensure_dirs_unittest.py |  8 ++++----
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/lib/tools/ensure_dirs.py b/lib/tools/ensure_dirs.py
index 02a59a250..ad1fb09fe 100644
--- a/lib/tools/ensure_dirs.py
+++ b/lib/tools/ensure_dirs.py
@@ -83,7 +83,7 @@ def EnsurePermission(path, mode, uid=-1, gid=-1, must_exist=True,
                         (path, err))
 
 
-def EnsureDir(path, mode, uid, gid, _stat_fn=os.lstat, _mkdir_fn=os.mkdir,
+def EnsureDir(path, mode, uid, gid, _lstat_fn=os.lstat, _mkdir_fn=os.mkdir,
               _ensure_fn=EnsurePermission):
   """Ensures that given path is a dir and has given mode, uid and gid set.
 
@@ -99,16 +99,15 @@ def EnsureDir(path, mode, uid, gid, _stat_fn=os.lstat, _mkdir_fn=os.mkdir,
   logging.debug("Checking directory %s", path)
   try:
     # We don't want to follow symlinks
-    st_mode = _stat_fn(path)[stat.ST_MODE]
-
-    if not stat.S_ISDIR(st_mode):
-      raise EnsureError("Path %s is expected to be a directory, but it's not" %
-                        path)
+    st = _lstat_fn(path)
   except EnvironmentError, err:
-    if err.errno == errno.ENOENT:
-      _mkdir_fn(path)
-    else:
-      raise EnsureError("Error while do a stat() on %s: %s" % (path, err))
+    if err.errno != errno.ENOENT:
+      raise EnsureError("stat(2) on %s failed: %s" % (path, err))
+    _mkdir_fn(path)
+  else:
+    if not stat.S_ISDIR(st[stat.ST_MODE]):
+      raise EnsureError("Path %s is expected to be a directory, but isn't" %
+                        path)
 
   _ensure_fn(path, mode, uid=uid, gid=gid)
 
diff --git a/test/ganeti.tools.ensure_dirs_unittest.py b/test/ganeti.tools.ensure_dirs_unittest.py
index 6e0dff3c7..999da06ee 100755
--- a/test/ganeti.tools.ensure_dirs_unittest.py
+++ b/test/ganeti.tools.ensure_dirs_unittest.py
@@ -77,18 +77,18 @@ class TestEnsureDirsFunctions(unittest.TestCase):
 
     self.assertRaises(ensure_dirs.EnsureError, ensure_dirs.EnsureDir,
                       "/ganeti-qa-non-test", 0700, 0, 0,
-                      _stat_fn=not_dir_stat)
+                      _lstat_fn=not_dir_stat)
     self.assertRaises(ensure_dirs.EnsureError, ensure_dirs.EnsureDir,
                       "/ganeti-qa-non-test", 0700, 0, 0,
-                      _stat_fn=other_stat_raise)
+                      _lstat_fn=other_stat_raise)
     self.mkdir_called = False
     ensure_dirs.EnsureDir("/ganeti-qa-non-test", 0700, 0, 0,
-                          _stat_fn=non_exist_stat, _mkdir_fn=self._NoopMkdir,
+                          _lstat_fn=non_exist_stat, _mkdir_fn=self._NoopMkdir,
                           _ensure_fn=self._VerifyEnsure)
     self.assertTrue(self.mkdir_called)
     self.mkdir_called = False
     ensure_dirs.EnsureDir("/ganeti-qa-non-test", 0700, 0, 0,
-                          _stat_fn=is_dir_stat, _ensure_fn=self._VerifyEnsure)
+                          _lstat_fn=is_dir_stat, _ensure_fn=self._VerifyEnsure)
     self.assertFalse(self.mkdir_called)
 
   def testEnsurePermission(self):
-- 
GitLab