Skip to content

Commit

Permalink
VMware: Add missing storage profile requirement
Browse files Browse the repository at this point in the history
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
  • Loading branch information
vbalachandran committed Dec 3, 2014
1 parent 530c178 commit b8b3590
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 5 deletions.
23 changes: 23 additions & 0 deletions cinder/tests/test_vmware_vmdk.py
Expand Up @@ -133,6 +133,8 @@ def __init__(self, obj=None):
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."""

Expand Down Expand Up @@ -1531,6 +1533,27 @@ def _test_retype(self, ds_sel, vops, get_volume_type_extra_specs,
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.
Expand Down
10 changes: 5 additions & 5 deletions cinder/volume/drivers/vmware/vmdk.py
Expand Up @@ -1459,11 +1459,7 @@ def retype(self, ctxt, volume, new_type, diff, host):
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.
Expand Down Expand Up @@ -1530,6 +1526,10 @@ def retype(self, ctxt, volume, new_type, diff, host):

# 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)
Expand Down

0 comments on commit b8b3590

Please sign in to comment.