From 0ef0824814d36a572cfb23386b33dbdac07408c3 Mon Sep 17 00:00:00 2001 From: apoorvad Date: Tue, 15 Dec 2015 11:27:42 -0800 Subject: [PATCH] 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 --- cinder/tests/unit/test_nfs.py | 34 ++++++++++++---------------------- cinder/volume/drivers/nfs.py | 35 +++++++++++++++++++++++++---------- 2 files changed, 37 insertions(+), 32 deletions(-) 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 -- 2.45.2