From: Clinton Knight Date: Fri, 26 Feb 2016 21:21:59 +0000 (-0500) Subject: NetApp: volume resize using clone fails with QoS X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=f3c7688aebffc9a4b1c8fcf905e81be786d49583;p=openstack-build%2Fcinder-build.git NetApp: volume resize using clone fails with QoS Data ONTAP cannot resize a LUN past the geometry established when the LUN was created. So to resize past that limit, the DOT drivers create a new LUN of the needed size and clone the original LUN into it. This process doesn't always work if a QoS policy is in place, and the fix is to not pass the QoS policy to the clone operation. The fix is trivial, and I improved the surrounding code a little, but there was no unit test coverage for the method in question, so this commit also adds full coverage for the LUN clone method. Change-Id: I04dcf7ba329a9b94a39580b1115a9dec41231b85 Closes-Bug: #1550478 --- diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py index 9d282cb56..02eec8ac5 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py @@ -1028,6 +1028,214 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): self.assertFalse(mock_do_direct_resize.called) self.assertFalse(mock_do_sub_clone_resize.called) + def test_do_sub_clone_resize(self): + + fake_lun = block_base.NetAppLun(fake.LUN_HANDLE, + fake.LUN_ID, + fake.LUN_SIZE, + fake.LUN_METADATA) + new_lun_size = fake.LUN_SIZE * 10 + new_lun_name = 'new-%s' % fake.LUN_NAME + block_count = fake.LUN_SIZE * units.Gi / 512 + + mock_get_lun_from_table = self.mock_object( + self.library, '_get_lun_from_table', + mock.Mock(return_value=fake_lun)) + mock_get_vol_option = self.mock_object( + self.library, '_get_vol_option', mock.Mock(return_value='off')) + mock_get_lun_block_count = self.mock_object( + self.library, '_get_lun_block_count', + mock.Mock(return_value=block_count)) + mock_create_lun = self.mock_object( + self.library.zapi_client, 'create_lun') + mock_clone_lun = self.mock_object(self.library, '_clone_lun') + mock_post_sub_clone_resize = self.mock_object( + self.library, '_post_sub_clone_resize') + mock_destroy_lun = self.mock_object( + self.library.zapi_client, 'destroy_lun') + + self.library._do_sub_clone_resize(fake.LUN_PATH, + new_lun_size, + fake.QOS_POLICY_GROUP_NAME) + + mock_get_lun_from_table.assert_called_once_with(fake.LUN_NAME) + mock_get_vol_option.assert_called_once_with('vol0', 'compression') + mock_get_lun_block_count.assert_called_once_with(fake.LUN_PATH) + mock_create_lun.assert_called_once_with( + 'vol0', new_lun_name, new_lun_size, fake.LUN_METADATA, + qos_policy_group_name=fake.QOS_POLICY_GROUP_NAME) + mock_clone_lun.assert_called_once_with( + fake.LUN_NAME, new_lun_name, block_count=block_count) + mock_post_sub_clone_resize.assert_called_once_with(fake.LUN_PATH) + self.assertFalse(mock_destroy_lun.called) + + def test_do_sub_clone_resize_compression_on(self): + + fake_lun = block_base.NetAppLun(fake.LUN_HANDLE, + fake.LUN_ID, + fake.LUN_SIZE, + fake.LUN_METADATA) + new_lun_size = fake.LUN_SIZE * 10 + block_count = fake.LUN_SIZE * units.Gi / 512 + + mock_get_lun_from_table = self.mock_object( + self.library, '_get_lun_from_table', + mock.Mock(return_value=fake_lun)) + mock_get_vol_option = self.mock_object( + self.library, '_get_vol_option', mock.Mock(return_value='on')) + mock_get_lun_block_count = self.mock_object( + self.library, '_get_lun_block_count', + mock.Mock(return_value=block_count)) + mock_create_lun = self.mock_object( + self.library.zapi_client, 'create_lun') + mock_clone_lun = self.mock_object(self.library, '_clone_lun') + mock_post_sub_clone_resize = self.mock_object( + self.library, '_post_sub_clone_resize') + mock_destroy_lun = self.mock_object( + self.library.zapi_client, 'destroy_lun') + + self.assertRaises(exception.VolumeBackendAPIException, + self.library._do_sub_clone_resize, + fake.LUN_PATH, + new_lun_size, + fake.QOS_POLICY_GROUP_NAME) + + mock_get_lun_from_table.assert_called_once_with(fake.LUN_NAME) + mock_get_vol_option.assert_called_once_with('vol0', 'compression') + self.assertFalse(mock_get_lun_block_count.called) + self.assertFalse(mock_create_lun.called) + self.assertFalse(mock_clone_lun.called) + self.assertFalse(mock_post_sub_clone_resize.called) + self.assertFalse(mock_destroy_lun.called) + + def test_do_sub_clone_resize_no_blocks(self): + + fake_lun = block_base.NetAppLun(fake.LUN_HANDLE, + fake.LUN_ID, + fake.LUN_SIZE, + fake.LUN_METADATA) + new_lun_size = fake.LUN_SIZE * 10 + block_count = 0 + + mock_get_lun_from_table = self.mock_object( + self.library, '_get_lun_from_table', + mock.Mock(return_value=fake_lun)) + mock_get_vol_option = self.mock_object( + self.library, '_get_vol_option', mock.Mock(return_value='off')) + mock_get_lun_block_count = self.mock_object( + self.library, '_get_lun_block_count', + mock.Mock(return_value=block_count)) + mock_create_lun = self.mock_object( + self.library.zapi_client, 'create_lun') + mock_clone_lun = self.mock_object(self.library, '_clone_lun') + mock_post_sub_clone_resize = self.mock_object( + self.library, '_post_sub_clone_resize') + mock_destroy_lun = self.mock_object( + self.library.zapi_client, 'destroy_lun') + + self.assertRaises(exception.VolumeBackendAPIException, + self.library._do_sub_clone_resize, + fake.LUN_PATH, + new_lun_size, + fake.QOS_POLICY_GROUP_NAME) + + mock_get_lun_from_table.assert_called_once_with(fake.LUN_NAME) + mock_get_vol_option.assert_called_once_with('vol0', 'compression') + mock_get_lun_block_count.assert_called_once_with(fake.LUN_PATH) + self.assertFalse(mock_create_lun.called) + self.assertFalse(mock_clone_lun.called) + self.assertFalse(mock_post_sub_clone_resize.called) + self.assertFalse(mock_destroy_lun.called) + + def test_do_sub_clone_resize_create_error(self): + + fake_lun = block_base.NetAppLun(fake.LUN_HANDLE, + fake.LUN_ID, + fake.LUN_SIZE, + fake.LUN_METADATA) + new_lun_size = fake.LUN_SIZE * 10 + new_lun_name = 'new-%s' % fake.LUN_NAME + block_count = fake.LUN_SIZE * units.Gi / 512 + + mock_get_lun_from_table = self.mock_object( + self.library, '_get_lun_from_table', + mock.Mock(return_value=fake_lun)) + mock_get_vol_option = self.mock_object( + self.library, '_get_vol_option', mock.Mock(return_value='off')) + mock_get_lun_block_count = self.mock_object( + self.library, '_get_lun_block_count', + mock.Mock(return_value=block_count)) + mock_create_lun = self.mock_object( + self.library.zapi_client, 'create_lun', + mock.Mock(side_effect=netapp_api.NaApiError)) + mock_clone_lun = self.mock_object(self.library, '_clone_lun') + mock_post_sub_clone_resize = self.mock_object( + self.library, '_post_sub_clone_resize') + mock_destroy_lun = self.mock_object( + self.library.zapi_client, 'destroy_lun') + + self.assertRaises(netapp_api.NaApiError, + self.library._do_sub_clone_resize, + fake.LUN_PATH, + new_lun_size, + fake.QOS_POLICY_GROUP_NAME) + + mock_get_lun_from_table.assert_called_once_with(fake.LUN_NAME) + mock_get_vol_option.assert_called_once_with('vol0', 'compression') + mock_get_lun_block_count.assert_called_once_with(fake.LUN_PATH) + mock_create_lun.assert_called_once_with( + 'vol0', new_lun_name, new_lun_size, fake.LUN_METADATA, + qos_policy_group_name=fake.QOS_POLICY_GROUP_NAME) + self.assertFalse(mock_clone_lun.called) + self.assertFalse(mock_post_sub_clone_resize.called) + self.assertFalse(mock_destroy_lun.called) + + def test_do_sub_clone_resize_clone_error(self): + + fake_lun = block_base.NetAppLun(fake.LUN_HANDLE, + fake.LUN_ID, + fake.LUN_SIZE, + fake.LUN_METADATA) + new_lun_size = fake.LUN_SIZE * 10 + new_lun_name = 'new-%s' % fake.LUN_NAME + new_lun_path = '/vol/vol0/%s' % new_lun_name + block_count = fake.LUN_SIZE * units.Gi / 512 + + mock_get_lun_from_table = self.mock_object( + self.library, '_get_lun_from_table', + mock.Mock(return_value=fake_lun)) + mock_get_vol_option = self.mock_object( + self.library, '_get_vol_option', mock.Mock(return_value='off')) + mock_get_lun_block_count = self.mock_object( + self.library, '_get_lun_block_count', + mock.Mock(return_value=block_count)) + mock_create_lun = self.mock_object( + self.library.zapi_client, 'create_lun') + mock_clone_lun = self.mock_object( + self.library, '_clone_lun', + mock.Mock(side_effect=netapp_api.NaApiError)) + mock_post_sub_clone_resize = self.mock_object( + self.library, '_post_sub_clone_resize') + mock_destroy_lun = self.mock_object( + self.library.zapi_client, 'destroy_lun') + + self.assertRaises(netapp_api.NaApiError, + self.library._do_sub_clone_resize, + fake.LUN_PATH, + new_lun_size, + fake.QOS_POLICY_GROUP_NAME) + + mock_get_lun_from_table.assert_called_once_with(fake.LUN_NAME) + mock_get_vol_option.assert_called_once_with('vol0', 'compression') + mock_get_lun_block_count.assert_called_once_with(fake.LUN_PATH) + mock_create_lun.assert_called_once_with( + 'vol0', new_lun_name, new_lun_size, fake.LUN_METADATA, + qos_policy_group_name=fake.QOS_POLICY_GROUP_NAME) + mock_clone_lun.assert_called_once_with( + fake.LUN_NAME, new_lun_name, block_count=block_count) + self.assertFalse(mock_post_sub_clone_resize.called) + mock_destroy_lun.assert_called_once_with(new_lun_path) + def test_configure_chap_generate_username_and_password(self): """Ensure that a CHAP username and password are generated.""" initiator_name = fake.ISCSI_CONNECTOR['initiator'] diff --git a/cinder/volume/drivers/netapp/dataontap/block_base.py b/cinder/volume/drivers/netapp/dataontap/block_base.py index d28705c00..50abc00bc 100644 --- a/cinder/volume/drivers/netapp/dataontap/block_base.py +++ b/cinder/volume/drivers/netapp/dataontap/block_base.py @@ -553,45 +553,42 @@ class NetAppBlockStorageLibrary(object): break return value - def _do_sub_clone_resize(self, path, new_size_bytes, + def _do_sub_clone_resize(self, lun_path, new_size_bytes, qos_policy_group_name=None): - """Does sub LUN clone after verification. + """Resize a LUN beyond its original geometry using sub-LUN cloning. - Clones the block ranges and swaps - the LUNs also deletes older LUN - after a successful clone. + Clones the block ranges, swaps the LUNs, and deletes the source LUN. """ - seg = path.split("/") - LOG.info(_LI("Resizing LUN %s to new size using clone operation."), - seg[-1]) - name = seg[-1] + seg = lun_path.split("/") + LOG.info(_LI("Resizing LUN %s using clone operation."), seg[-1]) + lun_name = seg[-1] vol_name = seg[2] - lun = self._get_lun_from_table(name) + lun = self._get_lun_from_table(lun_name) metadata = lun.metadata + compression = self._get_vol_option(vol_name, 'compression') if compression == "on": msg = _('%s cannot be resized using clone operation' ' as it is hosted on compressed volume') - raise exception.VolumeBackendAPIException(data=msg % name) - else: - block_count = self._get_lun_block_count(path) - if block_count == 0: - msg = _('%s cannot be resized using clone operation' - ' as it contains no blocks.') - raise exception.VolumeBackendAPIException(data=msg % name) - new_lun = 'new-%s' % name - self.zapi_client.create_lun( - vol_name, new_lun, new_size_bytes, metadata, - qos_policy_group_name=qos_policy_group_name) - try: - self._clone_lun(name, new_lun, block_count=block_count, - qos_policy_group_name=qos_policy_group_name) - - self._post_sub_clone_resize(path) - except Exception: - with excutils.save_and_reraise_exception(): - new_path = '/vol/%s/%s' % (vol_name, new_lun) - self.zapi_client.destroy_lun(new_path) + raise exception.VolumeBackendAPIException(data=msg % lun_name) + + block_count = self._get_lun_block_count(lun_path) + if block_count == 0: + msg = _('%s cannot be resized using clone operation' + ' as it contains no blocks.') + raise exception.VolumeBackendAPIException(data=msg % lun_name) + + new_lun_name = 'new-%s' % lun_name + self.zapi_client.create_lun( + vol_name, new_lun_name, new_size_bytes, metadata, + qos_policy_group_name=qos_policy_group_name) + try: + self._clone_lun(lun_name, new_lun_name, block_count=block_count) + self._post_sub_clone_resize(lun_path) + except Exception: + with excutils.save_and_reraise_exception(): + new_lun_path = '/vol/%s/%s' % (vol_name, new_lun_name) + self.zapi_client.destroy_lun(new_lun_path) def _post_sub_clone_resize(self, path): """Try post sub clone resize in a transactional manner."""