From 766d13218763f43ba91c2a15adff438a0dd84e37 Mon Sep 17 00:00:00 2001 From: Radoslav Gerganov Date: Fri, 15 Jan 2016 13:41:13 +0200 Subject: [PATCH] VMware: Factor out relocate_vm() VM relocation is not volume specific operation and should be moved to vm_util.py Change-Id: Ic129a8da2b3e6eed3635f3d8b07f32a60ec9d8e3 --- .../unit/virt/vmwareapi/test_driver_api.py | 7 +++-- .../unit/virt/vmwareapi/test_volumeops.py | 26 ++++++++++--------- nova/virt/vmwareapi/vm_util.py | 17 +++++++++--- nova/virt/vmwareapi/volumeops.py | 24 ++++------------- 4 files changed, 35 insertions(+), 39 deletions(-) diff --git a/nova/tests/unit/virt/vmwareapi/test_driver_api.py b/nova/tests/unit/virt/vmwareapi/test_driver_api.py index d5320d1905d9..88aaeae1d0fc 100644 --- a/nova/tests/unit/virt/vmwareapi/test_driver_api.py +++ b/nova/tests/unit/virt/vmwareapi/test_driver_api.py @@ -1121,8 +1121,7 @@ class VMwareAPIVMTestCase(test.NoDBTestCase, self._check_vm_info(info, power_state.RUNNING) self.assertTrue(self.exception) - @mock.patch('nova.virt.vmwareapi.volumeops.VMwareVolumeOps.' - '_relocate_vmdk_volume') + @mock.patch.object(vm_util, 'relocate_vm') @mock.patch('nova.virt.vmwareapi.volumeops.VMwareVolumeOps.' 'attach_volume') @mock.patch('nova.virt.vmwareapi.volumeops.VMwareVolumeOps.' @@ -1133,7 +1132,7 @@ class VMwareAPIVMTestCase(test.NoDBTestCase, mock_volume_in_mapping, mock_get_res_pool_of_vm, mock_attach_volume, - mock_relocate_vmdk_volume, + mock_relocate_vm, set_image_ref=True): self._create_instance(set_image_ref=set_image_ref) @@ -1152,7 +1151,7 @@ class VMwareAPIVMTestCase(test.NoDBTestCase, mock_info_get_mapping.assert_called_once_with(mock.ANY) mock_get_res_pool_of_vm.assert_called_once_with(mock.ANY) - mock_relocate_vmdk_volume.assert_called_once_with(mock.ANY, + mock_relocate_vm.assert_called_once_with(mock.ANY, mock.ANY, 'fake_res_pool', mock.ANY) mock_attach_volume.assert_called_once_with(connection_info, self.instance, constants.DEFAULT_ADAPTER_TYPE) diff --git a/nova/tests/unit/virt/vmwareapi/test_volumeops.py b/nova/tests/unit/virt/vmwareapi/test_volumeops.py index 7a14b1ff57e1..450a38ded068 100644 --- a/nova/tests/unit/virt/vmwareapi/test_volumeops.py +++ b/nova/tests/unit/virt/vmwareapi/test_volumeops.py @@ -494,9 +494,9 @@ class VMwareVolumeOpsTestCase(test.NoDBTestCase): @mock.patch.object(volumeops.VMwareVolumeOps, '_get_vmdk_base_volume_device') - @mock.patch.object(volumeops.VMwareVolumeOps, '_relocate_vmdk_volume') + @mock.patch.object(vm_util, 'relocate_vm') def test_consolidate_vmdk_volume_with_no_relocate( - self, relocate_vmdk_volume, get_vmdk_base_volume_device): + self, relocate_vm, get_vmdk_base_volume_device): file_name = mock.sentinel.file_name backing = mock.Mock(fileName=file_name) original_device = mock.Mock(backing=backing) @@ -511,18 +511,18 @@ class VMwareVolumeOpsTestCase(test.NoDBTestCase): device, volume_ref) get_vmdk_base_volume_device.assert_called_once_with(volume_ref) - self.assertFalse(relocate_vmdk_volume.called) + self.assertFalse(relocate_vm.called) @mock.patch.object(volumeops.VMwareVolumeOps, '_get_vmdk_base_volume_device') - @mock.patch.object(volumeops.VMwareVolumeOps, '_relocate_vmdk_volume') + @mock.patch.object(vm_util, 'relocate_vm') @mock.patch.object(volumeops.VMwareVolumeOps, '_get_host_of_vm') @mock.patch.object(volumeops.VMwareVolumeOps, '_get_res_pool_of_host') @mock.patch.object(volumeops.VMwareVolumeOps, 'detach_disk_from_vm') @mock.patch.object(volumeops.VMwareVolumeOps, 'attach_disk_to_vm') def test_consolidate_vmdk_volume_with_relocate( self, attach_disk_to_vm, detach_disk_from_vm, get_res_pool_of_host, - get_host_of_vm, relocate_vmdk_volume, get_vmdk_base_volume_device): + get_host_of_vm, relocate_vm, get_vmdk_base_volume_device): file_name = mock.sentinel.file_name backing = mock.Mock(fileName=file_name) original_device = mock.Mock(backing=backing) @@ -551,7 +551,7 @@ class VMwareVolumeOpsTestCase(test.NoDBTestCase): disk_type) get_vmdk_base_volume_device.assert_called_once_with(volume_ref) - relocate_vmdk_volume.assert_called_once_with( + relocate_vm.assert_called_once_with(self._session, volume_ref, rp, datastore, host) detach_disk_from_vm.assert_called_once_with( volume_ref, instance, original_device, destroy_disk=True) @@ -561,14 +561,14 @@ class VMwareVolumeOpsTestCase(test.NoDBTestCase): @mock.patch.object(volumeops.VMwareVolumeOps, '_get_vmdk_base_volume_device') - @mock.patch.object(volumeops.VMwareVolumeOps, '_relocate_vmdk_volume') + @mock.patch.object(vm_util, 'relocate_vm') @mock.patch.object(volumeops.VMwareVolumeOps, '_get_host_of_vm') @mock.patch.object(volumeops.VMwareVolumeOps, '_get_res_pool_of_host') @mock.patch.object(volumeops.VMwareVolumeOps, 'detach_disk_from_vm') @mock.patch.object(volumeops.VMwareVolumeOps, 'attach_disk_to_vm') def test_consolidate_vmdk_volume_with_missing_vmdk( self, attach_disk_to_vm, detach_disk_from_vm, get_res_pool_of_host, - get_host_of_vm, relocate_vmdk_volume, get_vmdk_base_volume_device): + get_host_of_vm, relocate_vm, get_vmdk_base_volume_device): file_name = mock.sentinel.file_name backing = mock.Mock(fileName=file_name) original_device = mock.Mock(backing=backing) @@ -584,7 +584,7 @@ class VMwareVolumeOpsTestCase(test.NoDBTestCase): rp = mock.sentinel.rp get_res_pool_of_host.return_value = rp - relocate_vmdk_volume.side_effect = [ + relocate_vm.side_effect = [ oslo_vmw_exceptions.FileNotFoundException, None] instance = mock.sentinel.instance @@ -599,9 +599,11 @@ class VMwareVolumeOpsTestCase(test.NoDBTestCase): get_vmdk_base_volume_device.assert_called_once_with(volume_ref) - relocate_calls = [mock.call(volume_ref, rp, datastore, host), - mock.call(volume_ref, rp, datastore, host)] - self.assertEqual(relocate_calls, relocate_vmdk_volume.call_args_list) + relocate_calls = [mock.call(self._session, volume_ref, rp, datastore, + host), + mock.call(self._session, volume_ref, rp, datastore, + host)] + self.assertEqual(relocate_calls, relocate_vm.call_args_list) detach_disk_from_vm.assert_called_once_with( volume_ref, instance, original_device) attach_disk_to_vm.assert_called_once_with( diff --git a/nova/virt/vmwareapi/vm_util.py b/nova/virt/vmwareapi/vm_util.py index 7d0ea119c2ab..85d48b369efb 100644 --- a/nova/virt/vmwareapi/vm_util.py +++ b/nova/virt/vmwareapi/vm_util.py @@ -933,17 +933,26 @@ def clone_vm_spec(client_factory, location, return clone_spec -def relocate_vm_spec(client_factory, datastore=None, host=None, +def relocate_vm_spec(client_factory, res_pool=None, datastore=None, host=None, disk_move_type="moveAllDiskBackingsAndAllowSharing"): - """Builds the VM relocation spec.""" rel_spec = client_factory.create('ns0:VirtualMachineRelocateSpec') rel_spec.datastore = datastore + rel_spec.host = host + rel_spec.pool = res_pool rel_spec.diskMoveType = disk_move_type - if host: - rel_spec.host = host return rel_spec +def relocate_vm(session, vm_ref, res_pool=None, datastore=None, host=None, + disk_move_type="moveAllDiskBackingsAndAllowSharing"): + client_factory = session.vim.client.factory + rel_spec = relocate_vm_spec(client_factory, res_pool, datastore, host, + disk_move_type) + relocate_task = session._call_method(session.vim, "RelocateVM_Task", + vm_ref, spec=rel_spec) + session._wait_for_task(relocate_task) + + def get_machine_id_change_spec(client_factory, machine_id_str): """Builds the machine id change config spec.""" virtual_machine_config_spec = client_factory.create( diff --git a/nova/virt/vmwareapi/volumeops.py b/nova/virt/vmwareapi/volumeops.py index 221fa4a7bc06..28aac62f24c9 100644 --- a/nova/virt/vmwareapi/volumeops.py +++ b/nova/virt/vmwareapi/volumeops.py @@ -385,22 +385,6 @@ class VMwareVolumeOps(object): else: raise exception.VolumeDriverNotFound(driver_type=driver_type) - def _relocate_vmdk_volume(self, volume_ref, res_pool, datastore, - host=None): - """Relocate the volume. - - The move type will be moveAllDiskBackingsAndAllowSharing. - """ - client_factory = self._session.vim.client.factory - spec = vm_util.relocate_vm_spec(client_factory, - datastore=datastore, - host=host) - spec.pool = res_pool - task = self._session._call_method(self._session.vim, - "RelocateVM_Task", volume_ref, - spec=spec) - self._session._wait_for_task(task) - def _get_host_of_vm(self, vm_ref): """Get the ESX host of given VM.""" return self._session._call_method(vutil, 'get_object_property', @@ -476,7 +460,8 @@ class VMwareVolumeOps(object): {'backing': volume_ref, 'rp': res_pool, 'ds': datastore, 'host': host}) try: - self._relocate_vmdk_volume(volume_ref, res_pool, datastore, host) + vm_util.relocate_vm(self._session, volume_ref, res_pool, datastore, + host) except oslo_vmw_exceptions.FileNotFoundException: # Volume's vmdk was moved; remove the device so that we can # relocate the volume. @@ -486,7 +471,8 @@ class VMwareVolumeOps(object): "reattempting relocate.") self.detach_disk_from_vm(volume_ref, instance, original_device) detached = True - self._relocate_vmdk_volume(volume_ref, res_pool, datastore, host) + vm_util.relocate_vm(self._session, volume_ref, res_pool, datastore, + host) # Volume's backing is relocated now; detach the old vmdk if not done # already. @@ -604,6 +590,6 @@ class VMwareVolumeOps(object): # Pick the resource pool on which the instance resides. Move the # volume to the datastore of the instance. res_pool = self._get_res_pool_of_vm(vm_ref) - self._relocate_vmdk_volume(volume_ref, res_pool, datastore) + vm_util.relocate_vm(self._session, volume_ref, res_pool, datastore) self.attach_volume(connection_info, instance, adapter_type)