From ec93474a84aff1c184a58fe436ea4a0842bd9825 Mon Sep 17 00:00:00 2001 From: Ivaylo Mitev Date: Thu, 18 Apr 2019 05:36:22 -0700 Subject: [PATCH] VMware VMDK detach: get adapter type from instance VM Detach VMDK using adapter type from instance instead of shadow VM. Closes-Bug: #1835733 Change-Id: I8668d57f6b302ad8cdc1e4af28cf6bb8145a738d --- .../unit/virt/vmwareapi/test_volumeops.py | 58 ++++++++++++------- nova/virt/vmwareapi/vim_util.py | 10 ++++ nova/virt/vmwareapi/vm_util.py | 32 +++++----- nova/virt/vmwareapi/volumeops.py | 20 ++++--- 4 files changed, 77 insertions(+), 43 deletions(-) diff --git a/nova/tests/unit/virt/vmwareapi/test_volumeops.py b/nova/tests/unit/virt/vmwareapi/test_volumeops.py index 073c703a8fcf..96c7fe9754f3 100644 --- a/nova/tests/unit/virt/vmwareapi/test_volumeops.py +++ b/nova/tests/unit/virt/vmwareapi/test_volumeops.py @@ -226,9 +226,15 @@ class VMwareVolumeOpsTestCase(test.NoDBTestCase): disk_uuid) def test_detach_volume_vmdk(self): + client_factory = self._volumeops._session.vim.client.factory + + virtual_controller = client_factory.create( + 'ns0:VirtualLsiLogicController') + virtual_controller.key = 100 + + virtual_disk = client_factory.create('ns0:VirtualDisk') + virtual_disk.controllerKey = virtual_controller.key - vmdk_info = vm_util.VmdkInfo('fake-path', 'lsiLogic', 'thin', - 1024, 'fake-device') with test.nested( mock.patch.object(vm_util, 'get_vm_ref', return_value=mock.sentinel.vm_ref), @@ -236,15 +242,17 @@ class VMwareVolumeOpsTestCase(test.NoDBTestCase): return_value=mock.sentinel.volume_ref), mock.patch.object(self._volumeops, '_get_vmdk_backed_disk_device', - return_value=mock.sentinel.device), - mock.patch.object(vm_util, 'get_vmdk_info', - return_value=vmdk_info), + return_value=virtual_disk), + mock.patch.object(vm_util, '_get_device_disk_type', + return_value='fake-disk-type'), mock.patch.object(self._volumeops, '_consolidate_vmdk_volume'), mock.patch.object(self._volumeops, 'detach_disk_from_vm'), mock.patch.object(self._volumeops, '_update_volume_details'), + mock.patch.object(self._volumeops._session, '_call_method', + return_value=[virtual_controller]) ) as (get_vm_ref, get_volume_ref, get_vmdk_backed_disk_device, - get_vmdk_info, consolidate_vmdk_volume, detach_disk_from_vm, - update_volume_details): + _get_device_disk_type, consolidate_vmdk_volume, + detach_disk_from_vm, update_volume_details, session_call_method): connection_info = {'driver_volume_type': 'vmdk', 'serial': 'volume-fake-id', @@ -262,39 +270,46 @@ class VMwareVolumeOpsTestCase(test.NoDBTestCase): connection_info['data']['volume']) get_vmdk_backed_disk_device.assert_called_once_with( mock.sentinel.vm_ref, connection_info['data']) - get_vmdk_info.assert_called_once_with(self._volumeops._session, - mock.sentinel.volume_ref) + adapter_type = vm_util.CONTROLLER_TO_ADAPTER_TYPE.get( + virtual_controller.__class__.__name__) consolidate_vmdk_volume.assert_called_once_with( - instance, mock.sentinel.vm_ref, mock.sentinel.device, - mock.sentinel.volume_ref, adapter_type=vmdk_info.adapter_type, - disk_type=vmdk_info.disk_type) + instance, mock.sentinel.vm_ref, virtual_disk, + mock.sentinel.volume_ref, adapter_type=adapter_type, + disk_type='fake-disk-type') detach_disk_from_vm.assert_called_once_with(mock.sentinel.vm_ref, instance, - mock.sentinel.device) + virtual_disk) update_volume_details.assert_called_once_with( mock.sentinel.vm_ref, connection_info['data']['volume_id'], "") def test_detach_volume_vmdk_invalid(self): + client_factory = self._volumeops._session.vim.client.factory + + virtual_controller = client_factory.create( + 'ns0:VirtualIDEController') + virtual_controller.key = 100 + + virtual_disk = client_factory.create('ns0:VirtualDisk') + virtual_disk.controllerKey = virtual_controller.key + connection_info = {'driver_volume_type': 'vmdk', 'serial': 'volume-fake-id', 'data': {'volume': 'vm-10', 'volume_id': 'volume-fake-id'}} instance = mock.MagicMock(name='fake-name', vm_state=vm_states.ACTIVE) - vmdk_info = vm_util.VmdkInfo('fake-path', constants.ADAPTER_TYPE_IDE, - constants.DISK_TYPE_PREALLOCATED, 1024, - 'fake-device') with test.nested( mock.patch.object(vm_util, 'get_vm_ref', return_value=mock.sentinel.vm_ref), mock.patch.object(self._volumeops, '_get_volume_ref'), mock.patch.object(self._volumeops, - '_get_vmdk_backed_disk_device'), - mock.patch.object(vm_util, 'get_vmdk_info', - return_value=vmdk_info), + '_get_vmdk_backed_disk_device', + return_value=virtual_disk), mock.patch.object(vm_util, 'get_vm_state', - return_value=power_state.RUNNING) + return_value=power_state.RUNNING), + mock.patch.object(self._volumeops._session, '_call_method', + return_value=[virtual_controller]) ) as (get_vm_ref, get_volume_ref, get_vmdk_backed_disk_device, - get_vmdk_info, get_vm_state): + get_vm_state, session_call_method): self.assertRaises(exception.Invalid, self._volumeops._detach_volume_vmdk, connection_info, instance) @@ -305,7 +320,6 @@ class VMwareVolumeOpsTestCase(test.NoDBTestCase): connection_info['data']['volume']) get_vmdk_backed_disk_device.assert_called_once_with( mock.sentinel.vm_ref, connection_info['data']) - self.assertTrue(get_vmdk_info.called) get_vm_state.assert_called_once_with(self._volumeops._session, instance) diff --git a/nova/virt/vmwareapi/vim_util.py b/nova/virt/vmwareapi/vim_util.py index 30340eee9295..699660de40f8 100644 --- a/nova/virt/vmwareapi/vim_util.py +++ b/nova/virt/vmwareapi/vim_util.py @@ -158,3 +158,13 @@ def get_about_info(vim): def get_entity_name(session, entity): return session._call_method(vutil, 'get_object_property', entity, 'name') + + +def get_array_items(array_obj): + """Get contained items if the object is a vSphere API array.""" + array_prefix = 'ArrayOf' + if array_obj.__class__.__name__.startswith(array_prefix): + attr_name = array_obj.__class__.__name__.replace(array_prefix, '', 1) + if hasattr(array_obj, attr_name): + return getattr(array_obj, attr_name) + return array_obj diff --git a/nova/virt/vmwareapi/vm_util.py b/nova/virt/vmwareapi/vm_util.py index 22820d5d8432..30df4fb447eb 100644 --- a/nova/virt/vmwareapi/vm_util.py +++ b/nova/virt/vmwareapi/vm_util.py @@ -46,6 +46,14 @@ ALL_SUPPORTED_NETWORK_DEVICES = ['VirtualE1000', 'VirtualE1000e', 'VirtualPCNet32', 'VirtualSriovEthernetCard', 'VirtualVmxnet', 'VirtualVmxnet3'] +CONTROLLER_TO_ADAPTER_TYPE = { + "VirtualLsiLogicController": constants.DEFAULT_ADAPTER_TYPE, + "VirtualBusLogicController": constants.ADAPTER_TYPE_BUSLOGIC, + "VirtualIDEController": constants.ADAPTER_TYPE_IDE, + "VirtualLsiLogicSASController": constants.ADAPTER_TYPE_LSILOGICSAS, + "ParaVirtualSCSIController": constants.ADAPTER_TYPE_PARAVIRTUAL +} + # A simple cache for storing inventory folder references. # Format: {inventory_path: folder_ref} _FOLDER_PATH_REF_MAPPING = {} @@ -674,14 +682,17 @@ def _get_device_disk_type(device): return constants.DEFAULT_DISK_TYPE -def get_vmdk_info(session, vm_ref, uuid=None): - """Returns information for the primary VMDK attached to the given VM.""" +def get_hardware_devices(session, vm_ref): hardware_devices = session._call_method(vutil, "get_object_property", vm_ref, "config.hardware.device") - if hardware_devices.__class__.__name__ == "ArrayOfVirtualDevice": - hardware_devices = hardware_devices.VirtualDevice + return vim_util.get_array_items(hardware_devices) + + +def get_vmdk_info(session, vm_ref, uuid=None): + """Returns information for the primary VMDK attached to the given VM.""" + hardware_devices = get_hardware_devices(session, vm_ref) vmdk_file_path = None vmdk_controller_key = None disk_type = None @@ -703,16 +714,9 @@ def get_vmdk_info(session, vm_ref, uuid=None): if root_disk and path.basename == root_disk: root_device = device vmdk_device = device - elif device.__class__.__name__ == "VirtualLsiLogicController": - adapter_type_dict[device.key] = constants.DEFAULT_ADAPTER_TYPE - elif device.__class__.__name__ == "VirtualBusLogicController": - adapter_type_dict[device.key] = constants.ADAPTER_TYPE_BUSLOGIC - elif device.__class__.__name__ == "VirtualIDEController": - adapter_type_dict[device.key] = constants.ADAPTER_TYPE_IDE - elif device.__class__.__name__ == "VirtualLsiLogicSASController": - adapter_type_dict[device.key] = constants.ADAPTER_TYPE_LSILOGICSAS - elif device.__class__.__name__ == "ParaVirtualSCSIController": - adapter_type_dict[device.key] = constants.ADAPTER_TYPE_PARAVIRTUAL + elif device.__class__.__name__ in CONTROLLER_TO_ADAPTER_TYPE: + adapter_type_dict[device.key] = CONTROLLER_TO_ADAPTER_TYPE[ + device.__class__.__name__] if root_disk: vmdk_device = root_device diff --git a/nova/virt/vmwareapi/volumeops.py b/nova/virt/vmwareapi/volumeops.py index 28aac62f24c9..eddb7b870511 100644 --- a/nova/virt/vmwareapi/volumeops.py +++ b/nova/virt/vmwareapi/volumeops.py @@ -516,20 +516,26 @@ class VMwareVolumeOps(object): device = self._get_vmdk_backed_disk_device(vm_ref, data) - # Get details required for adding disk device such as - # adapter_type, disk_type - vmdk = vm_util.get_vmdk_info(self._session, volume_ref) + hardware_devices = vm_util.get_hardware_devices(self._session, vm_ref) + adapter_type = None + for hw_device in hardware_devices: + if hw_device.key == device.controllerKey: + adapter_type = vm_util.CONTROLLER_TO_ADAPTER_TYPE.get( + hw_device.__class__.__name__) + break # IDE does not support disk hotplug - if vmdk.adapter_type == constants.ADAPTER_TYPE_IDE: + if adapter_type == constants.ADAPTER_TYPE_IDE: state = vm_util.get_vm_state(self._session, instance) if state != power_state.SHUTDOWN: raise exception.Invalid(_('%s does not support disk ' - 'hotplug.') % vmdk.adapter_type) + 'hotplug.') % adapter_type) + + disk_type = vm_util._get_device_disk_type(device) self._consolidate_vmdk_volume(instance, vm_ref, device, volume_ref, - adapter_type=vmdk.adapter_type, - disk_type=vmdk.disk_type) + adapter_type=adapter_type, + disk_type=disk_type) self.detach_disk_from_vm(vm_ref, instance, device)