From: Vipin Balachandran Date: Sat, 10 Jan 2015 17:49:16 +0000 (+0530) Subject: VMware: Remove unused methods X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=6968994ea635a05d7cf1d9786c2fa0ae27aff201;p=openstack-build%2Fcinder-build.git VMware: Remove unused methods Patch to remove unused datastore selection methods in vmdk module. Change-Id: I96f651081a5f0f5747e0bb1fb49028422fd4a4f9 --- diff --git a/cinder/tests/unit/test_vmware_vmdk.py b/cinder/tests/unit/test_vmware_vmdk.py index 098bf38c5..4c005439b 100644 --- a/cinder/tests/unit/test_vmware_vmdk.py +++ b/cinder/tests/unit/test_vmware_vmdk.py @@ -294,97 +294,6 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): m.UnsetStubs() m.VerifyAll() - def test_select_datastore_summary(self): - """Test _select_datastore_summary.""" - m = self.mox - m.StubOutWithMock(self._driver.__class__, 'volumeops') - self._driver.volumeops = self._volumeops - - datastore1 = FakeMor('Datastore', 'my_ds_1') - datastore2 = FakeMor('Datastore', 'my_ds_2') - datastore3 = FakeMor('Datastore', 'my_ds_3') - datastore4 = FakeMor('Datastore', 'my_ds_4') - datastores = [datastore1, datastore2, datastore3, datastore4] - - m.StubOutWithMock(self._volumeops, 'get_summary') - summary1 = FakeDatastoreSummary(5, 100) - summary2 = FakeDatastoreSummary(25, 100) - summary3 = FakeDatastoreSummary(50, 100) - summary4 = FakeDatastoreSummary(75, 100) - - self._volumeops.get_summary( - datastore1).MultipleTimes().AndReturn(summary1) - self._volumeops.get_summary( - datastore2).MultipleTimes().AndReturn(summary2) - self._volumeops.get_summary( - datastore3).MultipleTimes().AndReturn(summary3) - self._volumeops.get_summary( - datastore4).MultipleTimes().AndReturn(summary4) - - m.StubOutWithMock(self._volumeops, 'get_connected_hosts') - - host1 = FakeMor('HostSystem', 'my_host_1') - host2 = FakeMor('HostSystem', 'my_host_2') - host3 = FakeMor('HostSystem', 'my_host_3') - host4 = FakeMor('HostSystem', 'my_host_4') - - self._volumeops.get_connected_hosts( - datastore1).MultipleTimes().AndReturn([host1, host2, host3, host4]) - self._volumeops.get_connected_hosts( - datastore2).MultipleTimes().AndReturn([host1, host2, host3]) - self._volumeops.get_connected_hosts( - datastore3).MultipleTimes().AndReturn([host1, host2]) - self._volumeops.get_connected_hosts( - datastore4).MultipleTimes().AndReturn([host1, host2]) - - m.ReplayAll() - - summary = self._driver._select_datastore_summary(1, datastores) - self.assertEqual(summary, summary1) - - summary = self._driver._select_datastore_summary(10, datastores) - self.assertEqual(summary, summary2) - - summary = self._driver._select_datastore_summary(40, datastores) - self.assertEqual(summary, summary4) - - self.assertRaises(exceptions.VimException, - self._driver._select_datastore_summary, - 100, datastores) - m.UnsetStubs() - m.VerifyAll() - - @mock.patch('cinder.volume.drivers.vmware.vmdk.VMwareEsxVmdkDriver.' - 'session', new_callable=mock.PropertyMock) - @mock.patch('cinder.volume.drivers.vmware.vmdk.VMwareEsxVmdkDriver.' - 'volumeops', new_callable=mock.PropertyMock) - def test_get_folder_ds_summary(self, volumeops, session): - """Test _get_folder_ds_summary.""" - volumeops = volumeops.return_value - driver = self._driver - volume = {'size': 10, 'volume_type_id': 'fake_type'} - rp = mock.sentinel.resource_pool - dss = mock.sentinel.datastores - # patch method calls from _get_folder_ds_summary - volumeops.get_dc.return_value = mock.sentinel.dc - volumeops.get_vmfolder.return_value = mock.sentinel.folder - driver._get_storage_profile = mock.MagicMock() - driver._select_datastore_summary = mock.MagicMock() - driver._select_datastore_summary.return_value = mock.sentinel.summary - # call _get_folder_ds_summary - (folder, datastore_summary) = driver._get_folder_ds_summary(volume, - rp, dss) - # verify returned values and calls made - self.assertEqual(mock.sentinel.folder, folder, - "Folder returned is wrong.") - self.assertEqual(mock.sentinel.summary, datastore_summary, - "Datastore summary returned is wrong.") - volumeops.get_dc.assert_called_once_with(rp) - volumeops.get_vmfolder.assert_called_once_with(mock.sentinel.dc) - driver._get_storage_profile.assert_called_once_with(volume) - size = volume['size'] * units.Gi - driver._select_datastore_summary.assert_called_once_with(size, dss) - @mock.patch('cinder.volume.volume_types.get_volume_type_extra_specs') def test_get_disk_type(self, get_volume_type_extra_specs): """Test _get_disk_type.""" @@ -2390,95 +2299,6 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): profile = self._driver._get_storage_profile(volume) self.assertIsNone(profile) - @mock.patch('oslo_vmware.pbm.convert_datastores_to_hubs') - @mock.patch('oslo_vmware.pbm.get_profile_id_by_name') - @mock.patch('oslo_vmware.pbm.filter_hubs_by_profile') - @mock.patch('oslo_vmware.pbm.filter_datastores_by_hubs') - @mock.patch('cinder.volume.drivers.vmware.vmdk.VMwareVcVmdkDriver.' - 'session', new_callable=mock.PropertyMock) - def test_filter_ds_by_profile(self, session, filter_dss, filter_hubs, - get_profile_id, dss_to_hubs): - """Test vmdk _filter_ds_by_profile() method.""" - - session = session.return_value - - # Test with no profile id - datastores = [mock.sentinel.ds1, mock.sentinel.ds2] - profile = 'fake_profile' - get_profile_id.return_value = None - self.assertRaises(exceptions.VimException, - self._driver._filter_ds_by_profile, - datastores, profile) - get_profile_id.assert_called_once_with(session, profile) - - # Test with a fake profile id - profileId = 'fake_profile_id' - filtered_dss = [mock.sentinel.ds1] - # patch method calls from _filter_ds_by_profile - get_profile_id.return_value = profileId - pbm_cf = mock.sentinel.pbm_cf - session.pbm.client.factory = pbm_cf - hubs = [mock.sentinel.hub1, mock.sentinel.hub2] - dss_to_hubs.return_value = hubs - filter_hubs.return_value = mock.sentinel.hubs - filter_dss.return_value = filtered_dss - # call _filter_ds_by_profile with a fake profile - actual_dss = self._driver._filter_ds_by_profile(datastores, profile) - # verify return value and called methods - self.assertEqual(filtered_dss, actual_dss, - "Wrong filtered datastores returned.") - dss_to_hubs.assert_called_once_with(pbm_cf, datastores) - filter_hubs.assert_called_once_with(session, hubs, profileId) - filter_dss.assert_called_once_with(mock.sentinel.hubs, datastores) - - @mock.patch('cinder.volume.drivers.vmware.vmdk.VMwareVcVmdkDriver.' - 'session', new_callable=mock.PropertyMock) - @mock.patch('cinder.volume.drivers.vmware.vmdk.VMwareVcVmdkDriver.' - 'volumeops', new_callable=mock.PropertyMock) - def test_get_folder_ds_summary(self, volumeops, session): - """Test _get_folder_ds_summary.""" - volumeops = volumeops.return_value - driver = self._driver - driver._storage_policy_enabled = True - volume = {'size': 10, 'volume_type_id': 'fake_type'} - rp = mock.sentinel.resource_pool - dss = [mock.sentinel.datastore1, mock.sentinel.datastore2] - filtered_dss = [mock.sentinel.datastore1] - profile = mock.sentinel.profile - - def filter_ds(datastores, storage_profile): - return filtered_dss - - # patch method calls from _get_folder_ds_summary - volumeops.get_dc.return_value = mock.sentinel.dc - volumeops.get_vmfolder.return_value = mock.sentinel.vmfolder - volumeops.create_folder.return_value = mock.sentinel.folder - driver._get_storage_profile = mock.MagicMock() - driver._get_storage_profile.return_value = profile - driver._filter_ds_by_profile = mock.MagicMock(side_effect=filter_ds) - driver._select_datastore_summary = mock.MagicMock() - driver._select_datastore_summary.return_value = mock.sentinel.summary - # call _get_folder_ds_summary - (folder, datastore_summary) = driver._get_folder_ds_summary(volume, - rp, dss) - # verify returned values and calls made - self.assertEqual(mock.sentinel.folder, folder, - "Folder returned is wrong.") - self.assertEqual(mock.sentinel.summary, datastore_summary, - "Datastore summary returned is wrong.") - volumeops.get_dc.assert_called_once_with(rp) - volumeops.get_vmfolder.assert_called_once_with(mock.sentinel.dc) - volumeops.create_folder.assert_called_once_with(mock.sentinel.vmfolder, - self.VOLUME_FOLDER) - driver._get_storage_profile.assert_called_once_with(volume) - driver._filter_ds_by_profile.assert_called_once_with(dss, profile) - size = volume['size'] * units.Gi - driver._select_datastore_summary.assert_called_once_with(size, - filtered_dss) - - # Clear side effects. - driver._filter_ds_by_profile.side_effect = None - @mock.patch.object(VMDK_DRIVER, 'volumeops') def test_extend_vmdk_virtual_disk(self, volume_ops): """Test vmdk._extend_vmdk_virtual_disk.""" diff --git a/cinder/volume/drivers/vmware/vmdk.py b/cinder/volume/drivers/vmware/vmdk.py index 4189baa1b..b6040676b 100644 --- a/cinder/volume/drivers/vmware/vmdk.py +++ b/cinder/volume/drivers/vmware/vmdk.py @@ -362,66 +362,6 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver): """ return self.volumeops.get_vmfolder(datacenter) - def _compute_space_utilization(self, datastore_summary): - """Compute the space utilization of the given datastore. - - :param datastore_summary: Summary of the datastore for which - space utilization is to be computed - :return: space utilization in the range [0..1] - """ - return ( - 1.0 - - datastore_summary.freeSpace / float(datastore_summary.capacity) - ) - - def _select_datastore_summary(self, size_bytes, datastores): - """Get the best datastore summary from the given datastore list. - - The implementation selects a datastore which is connected to maximum - number of hosts, provided there is enough space to accommodate the - volume. Ties are broken based on space utilization; datastore with - low space utilization is preferred. - - :param size_bytes: Size in bytes of the volume - :param datastores: Datastores from which a choice is to be made - for the volume - :return: Summary of the best datastore selected for volume - """ - best_summary = None - max_host_count = 0 - best_space_utilization = 1.0 - - for datastore in datastores: - summary = self.volumeops.get_summary(datastore) - if summary.freeSpace > size_bytes: - host_count = len(self.volumeops.get_connected_hosts(datastore)) - if host_count > max_host_count: - max_host_count = host_count - best_space_utilization = self._compute_space_utilization( - summary - ) - best_summary = summary - elif host_count == max_host_count: - # break the tie based on space utilization - space_utilization = self._compute_space_utilization( - summary - ) - if space_utilization < best_space_utilization: - best_space_utilization = space_utilization - best_summary = summary - - if not best_summary: - msg = _("Unable to pick datastore to accommodate %(size)s bytes " - "from the datastores: %(dss)s.") % {'size': size_bytes, - 'dss': datastores} - LOG.error(msg) - raise exceptions.VimException(msg) - - LOG.debug("Selected datastore: %(datastore)s with %(host_count)d " - "connected host(s) for the volume.", - {'datastore': best_summary, 'host_count': max_host_count}) - return best_summary - def _get_extra_spec_storage_profile(self, type_id): """Get storage profile name in the given volume type's extra spec. @@ -438,61 +378,6 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver): """ return self._get_extra_spec_storage_profile(volume['volume_type_id']) - def _filter_ds_by_profile(self, datastores, storage_profile): - """Filter out datastores that do not match given storage profile. - - :param datastores: list of candidate datastores - :param storage_profile: storage profile name required to be satisfied - :return: subset of datastores that match storage_profile, or empty list - if none of the datastores match - """ - LOG.debug("Filter datastores matching storage profile %(profile)s: " - "%(dss)s.", - {'profile': storage_profile, 'dss': datastores}) - profileId = pbm.get_profile_id_by_name(self.session, storage_profile) - if not profileId: - msg = _("No such storage profile '%s; is defined in vCenter.") - LOG.error(msg, storage_profile) - raise exceptions.VimException(msg % storage_profile) - pbm_cf = self.session.pbm.client.factory - hubs = pbm.convert_datastores_to_hubs(pbm_cf, datastores) - filtered_hubs = pbm.filter_hubs_by_profile(self.session, hubs, - profileId) - return pbm.filter_datastores_by_hubs(filtered_hubs, datastores) - - def _get_folder_ds_summary(self, volume, resource_pool, datastores): - """Get folder and best datastore summary where volume can be placed. - - :param volume: volume to place into one of the datastores - :param resource_pool: Resource pool reference - :param datastores: Datastores from which a choice is to be made - for the volume - :return: Folder and best datastore summary where volume can be - placed on. - """ - datacenter = self.volumeops.get_dc(resource_pool) - folder = self._get_volume_group_folder(datacenter) - storage_profile = self._get_storage_profile(volume) - if self._storage_policy_enabled and storage_profile: - LOG.debug("Storage profile required for this volume: %s.", - storage_profile) - datastores = self._filter_ds_by_profile(datastores, - storage_profile) - if not datastores: - msg = _("Aborting since none of the datastores match the " - "given storage profile %s.") - LOG.error(msg, storage_profile) - raise exceptions.VimException(msg % storage_profile) - elif storage_profile: - LOG.warning(_LW("Ignoring storage profile %s requirement for this " - "volume since policy based placement is " - "disabled."), storage_profile) - - size_bytes = volume['size'] * units.Gi - datastore_summary = self._select_datastore_summary(size_bytes, - datastores) - return (folder, datastore_summary) - @staticmethod def _get_extra_spec_disk_type(type_id): """Get disk type from the given volume type's extra spec.