From: Vipin Balachandran Date: Wed, 13 Nov 2013 15:03:39 +0000 (+0530) Subject: VMDK:Using host mount info for datastore selection X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=b85cc49588c9f40e091801af33da8c7575058ec0;p=openstack-build%2Fcinder-build.git VMDK:Using host mount info for datastore selection Currently the VMDK driver picks a datastore for volume creation based on space utilization. The shadow VM corresponding to the volume is migrated to a different host and datastore if the volume is attached to an instance created in a host which cannot access the volume's current datastore. To reduce these migrations, this fix 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. Closes-Bug: #1250816 Change-Id: I5ecf22d782857fca0889799229465c49afe188a0 --- diff --git a/cinder/tests/test_vmware_vmdk.py b/cinder/tests/test_vmware_vmdk.py index 188f2c176..2673c975e 100644 --- a/cinder/tests/test_vmware_vmdk.py +++ b/cinder/tests/test_vmware_vmdk.py @@ -203,7 +203,7 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): """Test get_volume_stats.""" stats = self._driver.get_volume_stats() self.assertEqual(stats['vendor_name'], 'VMware') - self.assertEqual(stats['driver_version'], '1.0') + self.assertEqual(stats['driver_version'], '1.1.0') self.assertEqual(stats['storage_protocol'], 'LSI Logic SCSI') self.assertEqual(stats['reserved_percentage'], 0) self.assertEqual(stats['total_capacity_gb'], 'unknown') @@ -721,32 +721,55 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): 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(10, 10) - summary2 = FakeDatastoreSummary(25, 50) - summary3 = FakeDatastoreSummary(50, 50) - summary4 = FakeDatastoreSummary(100, 100) - moxd = self._volumeops.get_summary(datastore1) - moxd.MultipleTimes().AndReturn(summary1) - moxd = self._volumeops.get_summary(datastore2) - moxd.MultipleTimes().AndReturn(summary2) - moxd = self._volumeops.get_summary(datastore3) - moxd.MultipleTimes().AndReturn(summary3) - moxd = self._volumeops.get_summary(datastore4) - moxd.MultipleTimes().AndReturn(summary4) + 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, summary3) - summary = self._driver._select_datastore_summary(50, datastores) + self.assertEqual(summary, summary2) + + summary = self._driver._select_datastore_summary(40, datastores) self.assertEqual(summary, summary4) + self.assertRaises(error_util.VimException, self._driver._select_datastore_summary, 100, datastores) diff --git a/cinder/volume/drivers/vmware/vmdk.py b/cinder/volume/drivers/vmware/vmdk.py index 2d446c1f2..acbf9a458 100644 --- a/cinder/volume/drivers/vmware/vmdk.py +++ b/cinder/volume/drivers/vmware/vmdk.py @@ -16,7 +16,12 @@ # under the License. """ -Driver for virtual machines running on VMware supported datastores. +Volume driver for VMware vCenter/ESX managed datastores. + +The volumes created by this driver are backed by VMDK (Virtual Machine +Disk) files stored in datastores. For ease of managing the VMDKs, the +driver creates a virtual machine for each of the volumes. This virtual +machine is never powered on and is often referred as the shadow VM. """ from oslo.config import cfg @@ -33,6 +38,7 @@ from cinder.volume.drivers.vmware import volumeops from cinder.volume import volume_types LOG = logging.getLogger(__name__) + THIN_VMDK_TYPE = 'thin' THICK_VMDK_TYPE = 'thick' EAGER_ZEROED_THICK_VMDK_TYPE = 'eagerZeroedThick' @@ -118,7 +124,7 @@ def _get_volume_type_extra_spec(type_id, spec_key, possible_values, class VMwareEsxVmdkDriver(driver.VolumeDriver): """Manage volumes on VMware ESX server.""" - VERSION = '1.0' + VERSION = '1.1.0' def __init__(self, *args, **kwargs): super(VMwareEsxVmdkDriver, self).__init__(*args, **kwargs) @@ -231,37 +237,64 @@ 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 best summary from datastore list that can accommodate volume. + """Get the best datastore summary from the given datastore list. - The implementation selects datastore based on maximum relative - free space, which is (free_space/total_space) and has free space to - store the volume backing. + 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: Best datastore summary to be picked for the volume + :return: Summary of the best datastore selected for volume """ best_summary = None - best_ratio = 0 + max_host_count = 0 + best_space_utilization = 1.0 + for datastore in datastores: summary = self.volumeops.get_summary(datastore) if summary.freeSpace > size_bytes: - ratio = float(summary.freeSpace) / summary.capacity - if ratio > best_ratio: - best_ratio = ratio + 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.") - LOG.error(msg % {'size': size_bytes, 'dss': datastores}) - raise error_util.VimException(msg % - {'size': size_bytes, - 'dss': datastores}) + "from the datastores: %(dss)s.") % {'size': size_bytes, + 'dss': datastores} + LOG.error(msg) + raise error_util.VimException(msg) - LOG.debug(_("Selected datastore: %s for the volume.") % best_summary) + 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_folder_ds_summary(self, size_gb, resource_pool, datastores): diff --git a/cinder/volume/drivers/vmware/volumeops.py b/cinder/volume/drivers/vmware/volumeops.py index 6fde60e90..aa07309a6 100644 --- a/cinder/volume/drivers/vmware/volumeops.py +++ b/cinder/volume/drivers/vmware/volumeops.py @@ -139,6 +139,54 @@ class VMwareVolumeOps(object): self._session.invoke_api(vim_util, 'cancel_retrieval', self._session.vim, retrieve_result) + def _is_usable(self, datastore, mount_info): + """Check if the given datastore is usable as per the given mount info. + + The datastore is considered to be usable for a host only if it is + writable, mounted and accessible. + + :param datastore: Reference to the datastore entity + :param mount_info: host mount information + :return: True if datastore is usable + """ + + writable = mount_info.accessMode == "readWrite" + + # If mounted attribute is not set, then default is True + mounted = True + if hasattr(mount_info, "mounted"): + mounted = mount_info.mounted + + if hasattr(mount_info, "accessible"): + accessible = mount_info.accessible + else: + # If accessible attribute is not set, we look at summary + summary = self.get_summary(datastore) + accessible = summary.accessible + + return writable and mounted and accessible + + def get_connected_hosts(self, datastore): + """Get all the hosts to which the datastore is connected and usable. + + The datastore is considered to be usable for a host only if it is + writable, mounted and accessible. + + :param datastore: Reference to the datastore entity + :return: List of managed object references of all connected + hosts + """ + + host_mounts = self._session.invoke_api(vim_util, 'get_object_property', + self._session.vim, datastore, + 'host') + connected_hosts = [] + for host_mount in host_mounts.DatastoreHostMount: + if self._is_usable(datastore, host_mount.mountInfo): + connected_hosts.append(host_mount.key.value) + + return connected_hosts + def _is_valid(self, datastore, host): """Check if host's datastore is accessible, mounted and writable. @@ -152,19 +200,7 @@ class VMwareVolumeOps(object): 'host') for host_mount in host_mounts.DatastoreHostMount: if host_mount.key.value == host.value: - mntInfo = host_mount.mountInfo - writable = mntInfo.accessMode == "readWrite" - # If mounted attribute is not set, then default is True - mounted = True - if hasattr(mntInfo, "mounted"): - mounted = mntInfo.mounted - if hasattr(mntInfo, "accessible"): - accessible = mntInfo.accessible - else: - # If accessible attribute is not set, we look at summary - summary = self.get_summary(datastore) - accessible = summary.accessible - return (accessible and mounted and writable) + return self._is_usable(datastore, host_mount.mountInfo) return False def get_dss_rp(self, host): @@ -200,9 +236,9 @@ class VMwareVolumeOps(object): compute_resource, 'resourcePool') if not valid_dss: - msg = _("There are no valid datastores present under %s.") - LOG.error(msg % host) - raise error_util.VimException(msg % host) + msg = _("There are no valid datastores attached to %s.") % host + LOG.error(msg) + raise error_util.VimException(msg) return (valid_dss, resource_pool) def _get_parent(self, child, parent_type):