From b2447503b0ba98e644439d31435d47435c46cd69 Mon Sep 17 00:00:00 2001 From: Alex Meade Date: Thu, 20 Mar 2014 15:44:44 -0400 Subject: [PATCH] NetApp cmode nfs: Fix QOS extra spec This patch fixes the incorrect behavior where the NetApp cmode nfs driver will choose to create a volume on a flexVol that has a QOS policy group that matches the specified QOS policy group intended for the cinder volume. The correct behavior is to ignore the QOS policy group of the flexVol and instead assign the QOS policy group to the newly created cinder volume. Change-Id: I45d905da2a9a07b3ae8c00a225ab3b7f7ceb12d8 Closes-Bug: #1288283 --- cinder/tests/test_netapp_nfs.py | 40 +++++++++++++++++++++++ cinder/volume/drivers/netapp/nfs.py | 30 ++++++++++++++--- cinder/volume/drivers/netapp/ssc_utils.py | 8 +---- 3 files changed, 66 insertions(+), 12 deletions(-) diff --git a/cinder/tests/test_netapp_nfs.py b/cinder/tests/test_netapp_nfs.py index d6ceb635c..bf520b783 100644 --- a/cinder/tests/test_netapp_nfs.py +++ b/cinder/tests/test_netapp_nfs.py @@ -811,6 +811,46 @@ class NetappDirectCmodeNfsDriverOnlyTestCase(test.TestCase): self._driver.ssc_enabled = True self._driver.configuration.netapp_copyoffload_tool_path = 'cof_path' + @mock.patch.object(netapp_nfs, 'get_volume_extra_specs') + def test_create_volume(self, mock_volume_extra_specs): + drv = self._driver + drv.ssc_enabled = False + extra_specs = {} + mock_volume_extra_specs.return_value = extra_specs + fake_share = 'localhost:myshare' + fake_qos_policy = 'qos_policy_1' + with mock.patch.object(drv, '_ensure_shares_mounted'): + with mock.patch.object(drv, '_find_shares', + return_value=['localhost:myshare']): + with mock.patch.object(drv, '_do_create_volume'): + volume_info = self._driver.create_volume(FakeVolume(1)) + self.assertEqual(volume_info.get('provider_location'), + fake_share) + + @mock.patch.object(netapp_nfs, 'get_volume_extra_specs') + def test_create_volume_with_qos_policy(self, mock_volume_extra_specs): + drv = self._driver + drv.ssc_enabled = False + extra_specs = {'netapp:qos_policy_group': 'qos_policy_1'} + fake_volume = FakeVolume(1) + fake_share = 'localhost:myshare' + fake_qos_policy = 'qos_policy_1' + mock_volume_extra_specs.return_value = extra_specs + + with mock.patch.object(drv, '_ensure_shares_mounted'): + with mock.patch.object(drv, '_find_shares', + return_value=['localhost:myshare']): + with mock.patch.object(drv, '_do_create_volume'): + with mock.patch.object(drv, + '_set_qos_policy_group_on_volume' + ) as mock_set_qos: + volume_info = self._driver.create_volume(fake_volume) + self.assertEqual(volume_info.get('provider_location'), + 'localhost:myshare') + mock_set_qos.assert_called_once_with(fake_volume, + fake_share, + fake_qos_policy) + def test_copy_img_to_vol_copyoffload_success(self): drv = self._driver context = object() diff --git a/cinder/volume/drivers/netapp/nfs.py b/cinder/volume/drivers/netapp/nfs.py index 1e5fa3fe1..630bd4c64 100644 --- a/cinder/volume/drivers/netapp/nfs.py +++ b/cinder/volume/drivers/netapp/nfs.py @@ -774,6 +774,9 @@ class NetAppDirectCmodeNfsDriver (NetAppDirectNfsDriver): """ self._ensure_shares_mounted() extra_specs = get_volume_extra_specs(volume) + qos_policy_group = None + if extra_specs: + qos_policy_group = extra_specs.pop('netapp:qos_policy_group', None) eligible = self._find_shares(volume['size'], extra_specs) if not eligible: raise exception.NfsNoSuitableShareFound( @@ -783,12 +786,16 @@ class NetAppDirectCmodeNfsDriver (NetAppDirectNfsDriver): volume['provider_location'] = sh LOG.info(_('casted to %s') % volume['provider_location']) self._do_create_volume(volume) + if qos_policy_group: + self._set_qos_policy_group_on_volume(volume, sh, + qos_policy_group) return {'provider_location': volume['provider_location']} - except Exception: - LOG.warn(_("Exception creating vol %(name)s" - " on share %(share)s") - % {'name': volume['name'], - 'share': volume['provider_location']}) + except Exception as ex: + LOG.error(_("Exception creating vol %(name)s on " + "share %(share)s. Details: %(ex)s") + % {'name': volume['name'], + 'share': volume['provider_location'], + 'ex': ex}) volume['provider_location'] = None finally: if self.ssc_enabled: @@ -796,6 +803,19 @@ class NetAppDirectCmodeNfsDriver (NetAppDirectNfsDriver): msg = _("Volume %s could not be created on shares.") raise exception.VolumeBackendAPIException(data=msg % (volume['name'])) + def _set_qos_policy_group_on_volume(self, volume, share, qos_policy_group): + target_path = '%s' % (volume['name']) + export_path = share.split(':')[1] + flex_vol_name = self._get_vol_by_junc_vserver(self.vserver, + export_path) + file_assign_qos = NaElement.create_node_with_children( + 'file-assign-qos', + **{'volume': flex_vol_name, + 'qos-policy-group-name': qos_policy_group, + 'file': target_path, + 'vserver': self.vserver}) + self._invoke_successfully(file_assign_qos) + def _find_shares(self, size, extra_specs): """Finds suitable shares for given params.""" shares = [] diff --git a/cinder/volume/drivers/netapp/ssc_utils.py b/cinder/volume/drivers/netapp/ssc_utils.py index ebd058885..a588b388e 100644 --- a/cinder/volume/drivers/netapp/ssc_utils.py +++ b/cinder/volume/drivers/netapp/ssc_utils.py @@ -533,7 +533,6 @@ def get_volumes_for_specs(ssc_vols, specs): result = copy.deepcopy(ssc_vols['all']) raid_type = specs.get('netapp:raid_type') disk_type = specs.get('netapp:disk_type') - qos_policy_group = specs.get('netapp:qos_policy_group') bool_specs_list = ['netapp_mirrored', 'netapp_unmirrored', 'netapp_dedup', 'netapp_nodedup', 'netapp_compression', 'netapp_nocompression', @@ -582,7 +581,7 @@ def get_volumes_for_specs(ssc_vols, specs): result = result & ssc_vols['thin'] else: result = result - ssc_vols['thin'] - if raid_type or disk_type or qos_policy_group: + if raid_type or disk_type: tmp = copy.deepcopy(result) for vol in tmp: if raid_type: @@ -595,11 +594,6 @@ def get_volumes_for_specs(ssc_vols, specs): vol_dtype = vol_dtype.lower() if vol_dtype else None if disk_type.lower() != vol_dtype: result.discard(vol) - if qos_policy_group: - vol_qos = vol.qos['qos_policy_group'] - vol_qos = vol_qos.lower() if vol_qos else None - if qos_policy_group.lower() != vol_qos: - result.discard(vol) return result -- 2.45.2