From: git-harry <git-harry@live.co.uk>
Date: Fri, 28 Nov 2014 12:35:43 +0000 (+0000)
Subject: Fix _get_disk_of_partition edgecase in utils
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=3d6b8dec1e2c47a5de437562f398f2a994cbea3f;p=openstack-build%2Fcinder-build.git

Fix _get_disk_of_partition edgecase in utils

_get_disk_of_partition in cinder/utils returns a disk device path,
calculated from a supplied partition device path, and stat for a device.

If the disk path calculated by the function, using a regex, is not a
block device the function will return the original device path argument
and the stat object for disk path that was not a block device.

This commit modifies the function so that if the supplied device path is
returned by the function the stat of that path is guaranteed to be
returned.

Change-Id: I7ce0a748fe4a0ebaa02cba11baadb8c71c007aa2
---

diff --git a/cinder/tests/test_utils.py b/cinder/tests/test_utils.py
index 9381de239..6e02759f1 100644
--- a/cinder/tests/test_utils.py
+++ b/cinder/tests/test_utils.py
@@ -603,6 +603,49 @@ class GenericUtilsTestCase(test.TestCase):
         self.assertIsNone(dev)
 
 
+class GetDiskOfPartitionTestCase(test.TestCase):
+    def test_devpath_is_diskpath(self):
+        devpath = '/some/path'
+        st_mock = mock.Mock()
+        output = utils._get_disk_of_partition(devpath, st_mock)
+        self.assertEqual('/some/path', output[0])
+        self.assertIs(st_mock, output[1])
+
+        with mock.patch('os.stat') as mock_stat:
+            devpath = '/some/path'
+            output = utils._get_disk_of_partition(devpath)
+            mock_stat.assert_called_once_with(devpath)
+            self.assertEqual(devpath, output[0])
+            self.assertIs(mock_stat.return_value, output[1])
+
+    @mock.patch('os.stat', side_effect=OSError)
+    def test_stat_oserror(self, mock_stat):
+        st_mock = mock.Mock()
+        devpath = '/some/path1'
+        output = utils._get_disk_of_partition(devpath, st_mock)
+        mock_stat.assert_called_once_with('/some/path')
+        self.assertEqual(devpath, output[0])
+        self.assertIs(st_mock, output[1])
+
+    @mock.patch('stat.S_ISBLK', return_value=True)
+    @mock.patch('os.stat')
+    def test_diskpath_is_block_device(self, mock_stat, mock_isblk):
+        st_mock = mock.Mock()
+        devpath = '/some/path1'
+        output = utils._get_disk_of_partition(devpath, st_mock)
+        self.assertEqual('/some/path', output[0])
+        self.assertEqual(mock_stat.return_value, output[1])
+
+    @mock.patch('stat.S_ISBLK', return_value=False)
+    @mock.patch('os.stat')
+    def test_diskpath_is_not_block_device(self, mock_stat, mock_isblk):
+        st_mock = mock.Mock()
+        devpath = '/some/path1'
+        output = utils._get_disk_of_partition(devpath, st_mock)
+        self.assertEqual(devpath, output[0])
+        self.assertEqual(st_mock, output[1])
+
+
 class MonkeyPatchTestCase(test.TestCase):
     """Unit test for utils.monkey_patch()."""
     def setUp(self):
diff --git a/cinder/utils.py b/cinder/utils.py
index eca0d9121..c2d751f3f 100644
--- a/cinder/utils.py
+++ b/cinder/utils.py
@@ -627,17 +627,17 @@ def _get_disk_of_partition(devpath, st=None):
     for '/dev/disk1p1' ('p' is prepended to the partition number if the disk
     name ends with numbers).
     """
-    if st is None:
-        st = os.stat(devpath)
     diskpath = re.sub('(?:(?<=\d)p)?\d+$', '', devpath)
     if diskpath != devpath:
         try:
-            st = os.stat(diskpath)
-            if stat.S_ISBLK(st.st_mode):
-                return (diskpath, st)
+            st_disk = os.stat(diskpath)
+            if stat.S_ISBLK(st_disk.st_mode):
+                return (diskpath, st_disk)
         except OSError:
             pass
     # devpath is not a partition
+    if st is None:
+        st = os.stat(devpath)
     return (devpath, st)