From d2775a4181b28f8999b672835e73dea54b218d93 Mon Sep 17 00:00:00 2001 From: "Erlon R. Cruz" Date: Fri, 19 Jan 2018 14:13:40 -0200 Subject: [PATCH] Netapp Ontap: Adds support for auto-max-over-subscription After the addition of the auto-max-over-subscription on Cinder, drivers that somehow used the CONF.max_over_subscription_ratio in the driver where marked as not supporting the feature. As Netapp Ontap drivers used the option to do some calculations, they were marked as so. In this patch we remove the function that did this calculation as the function (_share_has_space_for_clone() is not actually necessary as currectly all volume clones are checked by the scheduler.) Change-Id: I25713b402ad93e0b4aa1861b4ccd9e05ee496200 Depends-on: If30bb6276f58532c0f78ac268544a8804008770e --- .../drivers/netapp/dataontap/test_nfs_base.py | 58 ------------------- .../netapp/dataontap/test_nfs_cmode.py | 35 ++--------- .../drivers/netapp/dataontap/block_base.py | 2 +- .../drivers/netapp/dataontap/nfs_base.py | 15 +---- .../drivers/netapp/dataontap/nfs_cmode.py | 11 +--- cinder/volume/drivers/nfs.py | 3 +- 6 files changed, 9 insertions(+), 115 deletions(-) diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py index d85058621de..3cca4d986c8 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py @@ -671,64 +671,6 @@ class NetAppNfsDriverTestCase(test.TestCase): fake.NFS_VOLUME, fake.NFS_SHARE) - @ddt.data( - {'size': 12, 'thin': False, 'over': 1.0, 'res': 0, 'expected': True}, - {'size': 12, 'thin': False, 'over': 1.0, 'res': 5, 'expected': False}, - {'size': 12, 'thin': True, 'over': 1.0, 'res': 5, 'expected': False}, - {'size': 12, 'thin': True, 'over': 1.1, 'res': 5, 'expected': True}, - {'size': 240, 'thin': True, 'over': 20.0, 'res': 0, 'expected': True}, - {'size': 241, 'thin': True, 'over': 20.0, 'res': 0, 'expected': False}, - ) - @ddt.unpack - def test_share_has_space_for_clone(self, size, thin, over, res, expected): - total_bytes = 20 * units.Gi - available_bytes = 12 * units.Gi - - with mock.patch.object(self.driver, - '_get_capacity_info', - return_value=( - total_bytes, available_bytes)): - with mock.patch.object(self.driver, - 'max_over_subscription_ratio', - over): - with mock.patch.object(self.driver, - 'reserved_percentage', - res): - result = self.driver._share_has_space_for_clone( - fake.NFS_SHARE, - size, - thin=thin) - self.assertEqual(expected, result) - - @ddt.data( - {'size': 12, 'thin': False, 'over': 1.0, 'res': 0, 'expected': True}, - {'size': 12, 'thin': False, 'over': 1.0, 'res': 5, 'expected': False}, - {'size': 12, 'thin': True, 'over': 1.0, 'res': 5, 'expected': False}, - {'size': 12, 'thin': True, 'over': 1.1, 'res': 5, 'expected': True}, - {'size': 240, 'thin': True, 'over': 20.0, 'res': 0, 'expected': True}, - {'size': 241, 'thin': True, 'over': 20.0, 'res': 0, 'expected': False}, - ) - @ddt.unpack - @mock.patch.object(nfs_base.NetAppNfsDriver, '_get_capacity_info') - def test_share_has_space_for_clone2(self, - mock_get_capacity, - size, thin, over, res, expected): - total_bytes = 20 * units.Gi - available_bytes = 12 * units.Gi - mock_get_capacity.return_value = (total_bytes, available_bytes) - - with mock.patch.object(self.driver, - 'max_over_subscription_ratio', - over): - with mock.patch.object(self.driver, - 'reserved_percentage', - res): - result = self.driver._share_has_space_for_clone( - fake.NFS_SHARE, - size, - thin=thin) - self.assertEqual(expected, result) - def test_get_share_mount_and_vol_from_vol_ref(self): self.mock_object(na_utils, 'resolve_hostname', return_value='10.12.142.11') diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_cmode.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_cmode.py index 24bbae98836..d14264a694a 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_cmode.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_cmode.py @@ -999,20 +999,14 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): 0)]) mock_super_add_looping_tasks.assert_called_once_with() - @ddt.data({'has_space': True, 'type_match': True, 'expected': True}, - {'has_space': True, 'type_match': False, 'expected': False}, - {'has_space': False, 'type_match': True, 'expected': False}, - {'has_space': False, 'type_match': False, 'expected': False}) + @ddt.data({'type_match': True, 'expected': True}, + {'type_match': False, 'expected': False}) @ddt.unpack - def test_is_share_clone_compatible(self, has_space, type_match, expected): + def test_is_share_clone_compatible(self, type_match, expected): mock_get_flexvol_name_for_share = self.mock_object( self.driver, '_get_flexvol_name_for_share', return_value='fake_flexvol') - mock_is_volume_thin_provisioned = self.mock_object( - self.driver, '_is_volume_thin_provisioned', return_value='thin') - mock_share_has_space_for_clone = self.mock_object( - self.driver, '_share_has_space_for_clone', return_value=has_space) mock_is_share_vol_type_match = self.mock_object( self.driver, '_is_share_vol_type_match', return_value=type_match) @@ -1021,28 +1015,7 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): self.assertEqual(expected, result) mock_get_flexvol_name_for_share.assert_called_once_with(fake.NFS_SHARE) - mock_is_volume_thin_provisioned.assert_called_once_with('fake_flexvol') - mock_share_has_space_for_clone.assert_called_once_with( - fake.NFS_SHARE, fake.SIZE, 'thin') - if has_space: - mock_is_share_vol_type_match.assert_called_once_with( - fake.VOLUME, fake.NFS_SHARE, 'fake_flexvol') - - @ddt.data({'thin': True, 'expected': True}, - {'thin': False, 'expected': False}, - {'thin': None, 'expected': False}) - @ddt.unpack - def test_is_volume_thin_provisioned(self, thin, expected): - - ssc_data = {'thin_provisioning_support': thin} - mock_get_ssc_for_flexvol = self.mock_object( - self.driver.ssc_library, 'get_ssc_for_flexvol', - return_value=ssc_data) - - result = self.driver._is_volume_thin_provisioned('fake_flexvol') - - self.assertEqual(expected, result) - mock_get_ssc_for_flexvol.assert_called_once_with('fake_flexvol') + mock_is_share_vol_type_match.assert_called_once() @ddt.data({'flexvols': ['volume1', 'volume2'], 'expected': True}, {'flexvols': ['volume3', 'volume4'], 'expected': False}, diff --git a/cinder/volume/drivers/netapp/dataontap/block_base.py b/cinder/volume/drivers/netapp/dataontap/block_base.py index 821de9e4664..819d2949ca6 100644 --- a/cinder/volume/drivers/netapp/dataontap/block_base.py +++ b/cinder/volume/drivers/netapp/dataontap/block_base.py @@ -117,7 +117,7 @@ class NetAppBlockStorageLibrary(object): self.max_over_subscription_ratio = ( volume_utils.get_max_over_subscription_ratio( self.configuration.max_over_subscription_ratio, - supports_auto=False)) + supports_auto=True)) self.reserved_percentage = self._get_reserved_percentage() self.loopingcalls = loopingcalls.LoopingCalls() diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_base.py b/cinder/volume/drivers/netapp/dataontap/nfs_base.py index 63315b085c5..7025a68c0f8 100644 --- a/cinder/volume/drivers/netapp/dataontap/nfs_base.py +++ b/cinder/volume/drivers/netapp/dataontap/nfs_base.py @@ -74,6 +74,7 @@ class NetAppNfsDriver(driver.ManageableVD, self._execute = None self._context = None self.app_version = kwargs.pop("app_version", "unknown") + kwargs['supports_auto_mosr'] = True super(NetAppNfsDriver, self).__init__(*args, **kwargs) self.configuration.append_config_values(na_opts.netapp_connection_opts) self.configuration.append_config_values(na_opts.netapp_basicauth_opts) @@ -832,20 +833,6 @@ class NetAppNfsDriver(driver.ManageableVD, """Checks if share is compatible with volume to host its clone.""" raise NotImplementedError() - def _share_has_space_for_clone(self, share, size_in_gib, thin=True): - """Is there space on the share for a clone given the original size?""" - requested_size = size_in_gib * units.Gi - - total_size, total_available = self._get_capacity_info(share) - - reserved_ratio = self.reserved_percentage / 100.0 - reserved = int(round(total_size * reserved_ratio)) - available = max(0, total_available - reserved) - if thin: - available = available * self.max_over_subscription_ratio - - return available >= requested_size - def _check_share_can_hold_size(self, share, size): """Checks if volume can hold image with size.""" _tot_size, tot_available = self._get_capacity_info( diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py b/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py index 24101435ff2..fd1d1c6a61a 100644 --- a/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py +++ b/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py @@ -385,16 +385,7 @@ class NetAppCmodeNfsDriver(nfs_base.NetAppNfsDriver, def _is_share_clone_compatible(self, volume, share): """Checks if share is compatible with volume to host its clone.""" flexvol_name = self._get_flexvol_name_for_share(share) - thin = self._is_volume_thin_provisioned(flexvol_name) - return ( - self._share_has_space_for_clone(share, volume['size'], thin) and - self._is_share_vol_type_match(volume, share, flexvol_name) - ) - - def _is_volume_thin_provisioned(self, flexvol_name): - """Checks if a flexvol is thin (sparse file or thin provisioned).""" - ssc_info = self.ssc_library.get_ssc_for_flexvol(flexvol_name) - return ssc_info.get('thin_provisioning_support') or False + return self._is_share_vol_type_match(volume, share, flexvol_name) def _is_share_vol_type_match(self, volume, share, flexvol_name): """Checks if share matches volume type.""" diff --git a/cinder/volume/drivers/nfs.py b/cinder/volume/drivers/nfs.py index 344f92bb4b6..0d57c3b9f2f 100644 --- a/cinder/volume/drivers/nfs.py +++ b/cinder/volume/drivers/nfs.py @@ -114,12 +114,13 @@ class NfsDriver(remotefs.RemoteFSSnapDriverDistributed): nfs_mount_point_base=self.base, nfs_mount_options=opts) + supports_auto_mosr = kwargs.get('supports_auto_mosr', False) self._sparse_copy_volume_data = True self.reserved_percentage = self.configuration.reserved_percentage self.max_over_subscription_ratio = ( vutils.get_max_over_subscription_ratio( self.configuration.max_over_subscription_ratio, - supports_auto=False)) + supports_auto=supports_auto_mosr)) def initialize_connection(self, volume, connector):