From: Vipin Balachandran <vbala@vmware.com>
Date: Thu, 27 Nov 2014 14:55:42 +0000 (+0530)
Subject: VMware: Add missing storage profile requirement
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=b8b3590dbd4554c5d49d54252e52c8ec448a8464;p=openstack-build%2Fcinder-build.git

VMware: Add missing storage profile requirement

If a storage profile is part of a volume type, it should be used as a
requirement for datastore selection-- only datastores which satisfy the
storage profile should be used for backing VM creation. Currently in
retype API, if storage profiles in old and new volume types are same,
it is not passed as a requirement. Hence, the backing VM corresponding
to the volume might end up in a datastore which doesn't satisfy the
storage profile. This patch fixes the problem.

Closes-Bug: #1398293
Change-Id: I49161e9fc5a8f2749ee6097fa5a136b78dfcf3ab
---

diff --git a/cinder/tests/test_vmware_vmdk.py b/cinder/tests/test_vmware_vmdk.py
index eea42dc85..6140fc25f 100644
--- a/cinder/tests/test_vmware_vmdk.py
+++ b/cinder/tests/test_vmware_vmdk.py
@@ -133,6 +133,8 @@ class FakeObj(object):
         self.obj = obj
 
 
+# TODO(vbala) Split test methods handling multiple cases into multiple methods,
+# each handling a specific case.
 class VMwareEsxVmdkDriverTestCase(test.TestCase):
     """Test class for VMwareEsxVmdkDriver."""
 
@@ -1531,6 +1533,27 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase):
         vops.change_backing_profile.assert_called_once_with(backing,
                                                             profile_id)
 
+        # Modify the previous case with no profile change.
+        get_volume_type_extra_specs.side_effect = [vmdk.THICK_VMDK_TYPE,
+                                                   vmdk.THIN_VMDK_TYPE,
+                                                   'gold-1',
+                                                   'gold-1']
+        ds_sel.select_datastore.reset_mock()
+        vops.relocate_backing.reset_mock()
+        vops.move_backing_to_folder.reset_mock()
+        vops.change_backing_profile.reset_mock()
+
+        self.assertTrue(self._driver.retype(context, vol, new_type, diff,
+                                            host))
+        exp_req = {hub.DatastoreSelector.HARD_ANTI_AFFINITY_DS: [ds_value],
+                   hub.DatastoreSelector.PROFILE_NAME: 'gold-1',
+                   hub.DatastoreSelector.SIZE_BYTES: units.Gi}
+        ds_sel.select_datastore.assert_called_once_with(exp_req)
+        vops.relocate_backing.assert_called_once_with(
+            backing, candidate_ds, rp, host, vmdk.THIN_VMDK_TYPE)
+        vops.move_backing_to_folder.assert_called_once_with(backing, folder)
+        self.assertFalse(vops.change_backing_profile.called)
+
         # Test with disk type conversion, profile change, backing with
         # no snapshots and candidate datastore which is same as the backing
         # datastore.
diff --git a/cinder/volume/drivers/vmware/vmdk.py b/cinder/volume/drivers/vmware/vmdk.py
index 633759fd3..9cfa8f26d 100644
--- a/cinder/volume/drivers/vmware/vmdk.py
+++ b/cinder/volume/drivers/vmware/vmdk.py
@@ -1459,11 +1459,7 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver):
                 req[hub.DatastoreSelector.HARD_ANTI_AFFINITY_DS] = (
                     [datastore.value])
 
-            if need_profile_change:
-                LOG.debug("Backing: %(backing)s needs a profile change to: "
-                          "%(profile)s.",
-                          {'backing': backing,
-                           'profile': new_profile})
+            if new_profile:
                 req[hub.DatastoreSelector.PROFILE_NAME] = new_profile
 
             # Select datastore satisfying the requirements.
@@ -1530,6 +1526,10 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver):
 
         # Update the backing's storage profile if needed.
         if need_profile_change:
+            LOG.debug("Backing: %(backing)s needs a profile change to:"
+                      " %(profile)s.",
+                      {'backing': backing,
+                       'profile': new_profile})
             profile_id = None
             if new_profile is not None:
                 profile_id = self.ds_sel.get_profile_id(new_profile)