Making NFS _find_share efficient

NfsDriver's _find_share calls self._is_share_eligible, which calls
get_capacity_info(). Then, it calls get_capacity_info() itself.

The method is optimized to not call underlying get_capacity_info()
twice.

Change-Id: I07a0601b9d902e54f84540f05cdadd5587522643
Closes-Bug: #1507682

diff --git a/cinder/tests/unit/test_nfs.py b/cinder/tests/unit/test_nfs.py
index 3c8ce6c821f99af86c325a6d3a54e7a3b51ed07e..0dfd076a48b15e818d943e3fa3cbadbfdfc0eb54 100644 (file)
--- a/cinder/tests/unit/test_nfs.py
+++ b/cinder/tests/unit/test_nfs.py
@@ -637,31 +637,21 @@ class NfsDriverTestCase(test.TestCase):
 
     def test_find_share(self):
         """_find_share simple use case."""
-        mox = self.mox
         drv = self._driver
-
         drv._mounted_shares = [self.TEST_NFS_EXPORT1, self.TEST_NFS_EXPORT2]
 
-        mox.StubOutWithMock(drv, '_get_capacity_info')
-        drv._get_capacity_info(self.TEST_NFS_EXPORT1).\
-            AndReturn((5 * units.Gi, 2 * units.Gi,
-                       2 * units.Gi))
-        drv._get_capacity_info(self.TEST_NFS_EXPORT1).\
-            AndReturn((5 * units.Gi, 2 * units.Gi,
-                       2 * units.Gi))
-        drv._get_capacity_info(self.TEST_NFS_EXPORT2).\
-            AndReturn((10 * units.Gi, 3 * units.Gi,
-                       1 * units.Gi))
-        drv._get_capacity_info(self.TEST_NFS_EXPORT2).\
-            AndReturn((10 * units.Gi, 3 * units.Gi,
-                       1 * units.Gi))
-
-        mox.ReplayAll()
-
-        self.assertEqual(self.TEST_NFS_EXPORT2,
-                         drv._find_share(self.TEST_SIZE_IN_GB))
-
-        mox.VerifyAll()
+        with mock.patch.object(drv, '_get_capacity_info')\
+                as mock_get_capacity_info:
+            mock_get_capacity_info.side_effect = [
+                (5 * units.Gi, 2 * units.Gi, 2 * units.Gi),
+                (10 * units.Gi, 3 * units.Gi, 1 * units.Gi)]
+            self.assertEqual(self.TEST_NFS_EXPORT2,
+                             drv._find_share(self.TEST_SIZE_IN_GB))
+            self.assertTrue(mock.call(self.TEST_NFS_EXPORT1) in
+                            mock_get_capacity_info.call_args_list)
+            self.assertTrue(mock.call(self.TEST_NFS_EXPORT2) in
+                            mock_get_capacity_info.call_args_list)
+            self.assertEqual(mock_get_capacity_info.call_count, 2)
 
     def test_find_share_should_throw_error_if_there_is_not_enough_space(self):
         """_find_share should throw error if there is no share to host vol."""

diff --git a/cinder/volume/drivers/nfs.py b/cinder/volume/drivers/nfs.py
index aff48ee459101f668f58588e7e2c285b85654f47..21d241c49ca74af9f7f1a5c74bdcb3a43cd3f59b 100644 (file)
--- a/cinder/volume/drivers/nfs.py
+++ b/cinder/volume/drivers/nfs.py
@@ -201,10 +201,15 @@ class NfsDriver(driver.ExtendVD, remotefs.RemoteFSDriver):
         target_share_reserved = 0
 
         for nfs_share in self._mounted_shares:
-            if not self._is_share_eligible(nfs_share, volume_size_in_gib):
+            total_size, total_available, total_allocated = (
+                self._get_capacity_info(nfs_share))
+            share_info = {'total_size': total_size,
+                          'total_available': total_available,
+                          'total_allocated': total_allocated,
+                          }
+            if not self._is_share_eligible(nfs_share, volume_size_in_gib,
+                                           share_info):
                 continue
-            _total_size, _total_available, total_allocated = \
-                self._get_capacity_info(nfs_share)
             if target_share is not None:
                 if target_share_reserved > total_allocated:
                     target_share = nfs_share
@@ -221,7 +226,8 @@ class NfsDriver(driver.ExtendVD, remotefs.RemoteFSDriver):
 
         return target_share
 
-    def _is_share_eligible(self, nfs_share, volume_size_in_gib):
+    def _is_share_eligible(self, nfs_share, volume_size_in_gib,
+                           share_info=None):
         """Verifies NFS share is eligible to host volume with given size.
 
         First validation step: ratio of actual space (used_space / total_space)
@@ -245,18 +251,26 @@ class NfsDriver(driver.ExtendVD, remotefs.RemoteFSDriver):
 
         # 'nfs_used_ratio' is deprecated, so derive used_ratio from
         # reserved_percentage.
+        if share_info is None:
+            total_size, total_available, total_allocated = (
+                self._get_capacity_info(nfs_share))
+            share_info = {'total_size': total_size,
+                          'total_available': total_available,
+                          'total_allocated': total_allocated,
+                          }
         used_percentage = 100 - self.reserved_percentage
         used_ratio = used_percentage / 100.0
 
         oversub_ratio = self.over_subscription_ratio
         requested_volume_size = volume_size_in_gib * units.Gi
 
-        total_size, total_available, total_allocated = \
-            self._get_capacity_info(nfs_share)
-        apparent_size = max(0, total_size * oversub_ratio)
-        apparent_available = max(0, apparent_size - total_allocated)
+        apparent_size = max(0, share_info['total_size'] * oversub_ratio)
+        apparent_available = max(0, apparent_size -
+                                 share_info['total_allocated'])
 
-        actual_used_ratio = (total_size - total_available) / float(total_size)
+        actual_used_ratio = ((share_info['total_size'] -
+                              share_info['total_available']) /
+                             float(share_info['total_size']))
         if actual_used_ratio > used_ratio:
             # NOTE(morganfainberg): We check the used_ratio first since
             # with oversubscription it is possible to not have the actual
@@ -268,7 +282,8 @@ class NfsDriver(driver.ExtendVD, remotefs.RemoteFSDriver):
         if apparent_available <= requested_volume_size:
             LOG.debug('%s is above nfs_oversub_ratio', nfs_share)
             return False
-        if total_allocated / total_size >= oversub_ratio:
+        if share_info['total_allocated'] / share_info['total_size'] >= (
+                oversub_ratio):
             LOG.debug('%s reserved space is above nfs_oversub_ratio',
                       nfs_share)
             return False