From: apoorvad <apps.desh@gmail.com>
Date: Tue, 15 Dec 2015 19:27:42 +0000 (-0800)
Subject: Making NFS _find_share efficient
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=0ef0824814d36a572cfb23386b33dbdac07408c3;p=openstack-build%2Fcinder-build.git
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 3c8ce6c82..0dfd076a4 100644
--- 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 aff48ee45..21d241c49 100644
--- 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