diff --git a/nova/tests/unit/virt/vmwareapi/fake.py b/nova/tests/unit/virt/vmwareapi/fake.py index 2c09afb8ec..5aced6b805 100644 --- a/nova/tests/unit/virt/vmwareapi/fake.py +++ b/nova/tests/unit/virt/vmwareapi/fake.py @@ -529,22 +529,16 @@ class VirtualMachine(ManagedObject): if len(val.deviceChange) < 2: return - # Case of Reconfig of VM to attach disk - controller_key = val.deviceChange[0].device.controllerKey - filename = val.deviceChange[0].device.backing.fileName + # Case of Reconfig of VM to attach disk, just take the device... + disk = val.deviceChange[0].device - disk = VirtualDisk() - disk.controllerKey = controller_key - - disk_backing = VirtualDiskFlatVer2BackingInfo() - disk_backing.fileName = filename - disk_backing.key = -101 - disk.backing = disk_backing - disk.capacityInBytes = 1024 - disk.capacityInKB = 1 + # ...and fill out missing information (in case of a disk) + if isinstance(disk.backing, VirtualDiskFlatVer2BackingInfo): + disk.capacityInBytes = 1024 + disk.capacityInKB = 1 controller = VirtualLsiLogicController() - controller.key = controller_key + controller.key = disk.controllerKey devices = _create_array_of_type('VirtualDevice') devices.VirtualDevice = [disk, controller, self.device[0]] diff --git a/nova/tests/unit/virt/vmwareapi/test_driver_api.py b/nova/tests/unit/virt/vmwareapi/test_driver_api.py index ac473c8c09..b83549096d 100644 --- a/nova/tests/unit/virt/vmwareapi/test_driver_api.py +++ b/nova/tests/unit/virt/vmwareapi/test_driver_api.py @@ -1233,9 +1233,9 @@ class VMwareAPIVMTestCase(test.NoDBTestCase, def test_snapshot_no_root_disk(self): self._iso_disk_type_created(flavor='m1.micro') - self.assertRaises(error_util.NoRootDiskDefined, self.conn.snapshot, - self.context, self.instance, "Test-Snapshot", - lambda *args, **kwargs: None) + self.assertRaises(error_util.NoRootDiskDefined, + self.conn.snapshot, self.context, self.instance, + "Test-Snapshot", lambda *args, **kwargs: None) def test_snapshot_non_existent(self): self._create_instance() diff --git a/nova/tests/unit/virt/vmwareapi/test_images.py b/nova/tests/unit/virt/vmwareapi/test_images.py index f3fff83d90..7ec5742547 100644 --- a/nova/tests/unit/virt/vmwareapi/test_images.py +++ b/nova/tests/unit/virt/vmwareapi/test_images.py @@ -169,7 +169,7 @@ class VMwareImagesTestCase(test.NoDBTestCase): mock_image_transfer.assert_called_once_with(mock_read_handle, mock_write_handle) mock_get_vmdk_info.assert_called_once_with( - session, mock.sentinel.vm_ref, 'fake-vm') + session, mock.sentinel.vm_ref) session._call_method.assert_called_once_with( session.vim, "UnregisterVM", mock.sentinel.vm_ref) @@ -219,7 +219,7 @@ class VMwareImagesTestCase(test.NoDBTestCase): session._call_method.assert_called_once_with( session.vim, "UnregisterVM", mock.sentinel.vm_ref) mock_get_vmdk_info.assert_called_once_with( - session, mock.sentinel.vm_ref, 'fake-vm') + session, mock.sentinel.vm_ref) def test_from_image_with_image_ref(self): raw_disk_size_in_gb = 83 diff --git a/nova/tests/unit/virt/vmwareapi/test_vm_util.py b/nova/tests/unit/virt/vmwareapi/test_vm_util.py index 37de5d85c4..8ec3363022 100644 --- a/nova/tests/unit/virt/vmwareapi/test_vm_util.py +++ b/nova/tests/unit/virt/vmwareapi/test_vm_util.py @@ -299,6 +299,7 @@ class VMwareVMUtilTestCase(test.NoDBTestCase): controller_key = 1000 disk = fake.VirtualDisk() disk.controllerKey = controller_key + disk.unitNumber = 0 disk_backing = fake.VirtualDiskFlatVer2BackingInfo() disk_backing.fileName = filename disk.capacityInBytes = 1024 @@ -308,13 +309,14 @@ class VMwareVMUtilTestCase(test.NoDBTestCase): # Ephemeral disk e_disk = fake.VirtualDisk() e_disk.controllerKey = controller_key + e_disk.unitNumber = 1 disk_backing = fake.VirtualDiskFlatVer2BackingInfo() disk_backing.fileName = '[test_datastore] uuid/ephemeral_0.vmdk' e_disk.capacityInBytes = 512 e_disk.backing = disk_backing controller = fake.VirtualLsiLogicSASController() controller.key = controller_key - devices = [disk, e_disk, controller] + devices = [e_disk, disk, controller] return devices def test_get_vmdk_path_and_adapter_type(self): @@ -325,33 +327,9 @@ class VMwareVMUtilTestCase(test.NoDBTestCase): vmdk = vm_util.get_vmdk_info(session, None) self.assertEqual(constants.ADAPTER_TYPE_LSILOGICSAS, vmdk.adapter_type) - self.assertEqual('[test_datastore] uuid/ephemeral_0.vmdk', - vmdk.path) - self.assertEqual(512, vmdk.capacity_in_bytes) - self.assertEqual(devices[1], vmdk.device) - - def test_get_vmdk_path_and_adapter_type_with_match(self): - n_filename = '[test_datastore] uuid/uuid.vmdk' - devices = self._vmdk_path_and_adapter_type_devices(n_filename) - session = fake.FakeSession() - with mock.patch.object(session, '_call_method', return_value=devices): - vmdk = vm_util.get_vmdk_info(session, None, uuid='uuid') - self.assertEqual(constants.ADAPTER_TYPE_LSILOGICSAS, - vmdk.adapter_type) - self.assertEqual(n_filename, vmdk.path) + self.assertEqual(filename, vmdk.path) self.assertEqual(1024, vmdk.capacity_in_bytes) - self.assertEqual(devices[0], vmdk.device) - - def test_get_vmdk_path_and_adapter_type_with_nomatch(self): - n_filename = '[test_datastore] diuu/diuu.vmdk' - session = fake.FakeSession() - devices = self._vmdk_path_and_adapter_type_devices(n_filename) - with mock.patch.object(session, '_call_method', return_value=devices): - vmdk = vm_util.get_vmdk_info(session, None, uuid='uuid') - self.assertIsNone(vmdk.adapter_type) - self.assertIsNone(vmdk.path) - self.assertEqual(0, vmdk.capacity_in_bytes) - self.assertIsNone(vmdk.device) + self.assertEqual(devices[1], vmdk.device) def test_get_vmdk_adapter_type(self): # Test for the adapter_type to be used in vmdk descriptor diff --git a/nova/tests/unit/virt/vmwareapi/test_vmops.py b/nova/tests/unit/virt/vmwareapi/test_vmops.py index 19990b8b32..0928ec0a51 100644 --- a/nova/tests/unit/virt/vmwareapi/test_vmops.py +++ b/nova/tests/unit/virt/vmwareapi/test_vmops.py @@ -650,7 +650,7 @@ class VMwareVMOpsTestCase(test.NoDBTestCase): self._vmops._resize_create_ephemerals_and_swap( 'vm-ref', self._instance, 'block-devices') mock_get_vmdk_info.assert_called_once_with( - self._session, 'vm-ref', uuid=self._instance.uuid) + self._session, 'vm-ref') if vmdk.device: mock_get_datastore_by_ref.assert_called_once_with( self._session, datastore.ref) @@ -770,7 +770,7 @@ class VMwareVMOpsTestCase(test.NoDBTestCase): 'fake-spec') # Validate disk configuration fake_get_vmdk_info.assert_called_once_with( - self._session, 'fake-ref', uuid=self._instance.uuid) + self._session, 'fake-ref') fake_get_browser.assert_called_once_with('fake-ref') fake_original_exists.assert_called_once_with( self._session, 'fake-browser', @@ -862,8 +862,7 @@ class VMwareVMOpsTestCase(test.NoDBTestCase): 'fake_vm', 'cluster_ref', None) self.assertIs(ds, ret) - mock_get_vmdk.assert_called_once_with(self._session, 'fake_vm', - uuid=self._instance.uuid) + mock_get_vmdk.assert_called_once_with(self._session, 'fake_vm') mock_get_ds.assert_called_once_with(self._session, ds_ref) @mock.patch.object(vm_util, 'get_vmdk_info') @@ -889,8 +888,7 @@ class VMwareVMOpsTestCase(test.NoDBTestCase): 'fake_vm', 'cluster_ref', None) self.assertIs(ds, ret) - mock_get_vmdk.assert_called_once_with(self._session, 'fake_vm', - uuid=self._instance.uuid) + mock_get_vmdk.assert_called_once_with(self._session, 'fake_vm') mock_get_ds.assert_called_once_with(self._session, 'cluster_ref', None) @@ -1053,7 +1051,7 @@ class VMwareVMOpsTestCase(test.NoDBTestCase): fake_get_vm_ref.assert_called_once_with(self._session, self._instance) fake_get_vmdk_info.assert_called_once_with( - self._session, 'fake-ref', uuid=self._instance.uuid) + self._session, 'fake-ref') fake_get_browser.assert_called_once_with('fake-ref') fake_original_exists.assert_called_once_with( self._session, 'fake-browser', diff --git a/nova/virt/vmwareapi/images.py b/nova/virt/vmwareapi/images.py index 3482312696..589e64adea 100644 --- a/nova/virt/vmwareapi/images.py +++ b/nova/virt/vmwareapi/images.py @@ -360,7 +360,7 @@ def fetch_image_stream_optimized(context, instance, session, vm_name, LOG.info("Downloaded image file data %(image_ref)s", {'image_ref': instance.image_ref}, instance=instance) - vmdk = vm_util.get_vmdk_info(session, imported_vm_ref, vm_name) + vmdk = vm_util.get_vmdk_info(session, imported_vm_ref) session._call_method(session.vim, "UnregisterVM", imported_vm_ref) LOG.info("The imported VM was unregistered", instance=instance) return vmdk.capacity_in_bytes @@ -424,8 +424,7 @@ def fetch_image_ova(context, instance, session, vm_name, ds_name, {'image_ref': instance.image_ref}, instance=instance) imported_vm_ref = write_handle.get_imported_vm() vmdk = vm_util.get_vmdk_info(session, - imported_vm_ref, - vm_name) + imported_vm_ref) session._call_method(session.vim, "UnregisterVM", imported_vm_ref) LOG.info("The imported VM was unregistered", diff --git a/nova/virt/vmwareapi/vm_util.py b/nova/virt/vmwareapi/vm_util.py index 14314c22a4..019aa955e9 100644 --- a/nova/virt/vmwareapi/vm_util.py +++ b/nova/virt/vmwareapi/vm_util.py @@ -26,7 +26,6 @@ from oslo_service import loopingcall from oslo_utils import excutils from oslo_utils import units from oslo_vmware import exceptions as vexc -from oslo_vmware.objects import datastore as ds_obj from oslo_vmware import pbm from oslo_vmware import vim_util as vutil @@ -675,46 +674,57 @@ def get_hardware_devices(session, vm_ref): 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.""" +def _is_before_in_boot_order(disk1, disk2): + return (disk1.controllerKey < disk2.controllerKey or + disk1.controllerKey == disk2.controllerKey and + disk1.unitNumber < disk2.unitNumber) + + +def get_vmdk_info(session, vm_ref): + """Returns information for the first VMDK attached to the given VM. + + The order follows the boot order of the hypervisor and is determined as + follows: + - if disks are attached at multiple controllers, disks at a controller with + a lower device number take precedence. + - if more than one disk is attached at the same controller disks with a + lower unit number will take precedence + """ hardware_devices = get_hardware_devices(session, vm_ref) vmdk_file_path = None vmdk_controller_key = None disk_type = None capacity_in_bytes = 0 - # Determine if we need to get the details of the root disk - root_disk = None - root_device = None - if uuid: - root_disk = '%s.vmdk' % uuid - vmdk_device = None - + # Find the lowest_disk_device and fill the dict adapter_type_dict. + lowest_disk_device = None adapter_type_dict = {} for device in hardware_devices: + # For the disk-devices: Check if the current disk-device is before the + # lowest_disk_device in the boot order if device.__class__.__name__ == "VirtualDisk": - if device.backing.__class__.__name__ == \ - "VirtualDiskFlatVer2BackingInfo": - path = ds_obj.DatastorePath.parse(device.backing.fileName) - if root_disk and path.basename == root_disk: - root_device = device - vmdk_device = device + if (device.backing.__class__.__name__ == + "VirtualDiskFlatVer2BackingInfo"): + if (lowest_disk_device is None or + _is_before_in_boot_order(device, lowest_disk_device)): + lowest_disk_device = device + # For the controller devices: Memorize for controller-type as + # defined by the class name for the device.key. So, we can look it up + # easily, when we know which is actually the first. 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 - - if vmdk_device: - vmdk_file_path = vmdk_device.backing.fileName - capacity_in_bytes = _get_device_capacity(vmdk_device) - vmdk_controller_key = vmdk_device.controllerKey - disk_type = _get_device_disk_type(vmdk_device) + if lowest_disk_device: + vmdk_file_path = lowest_disk_device.backing.fileName + capacity_in_bytes = _get_device_capacity(lowest_disk_device) + vmdk_controller_key = lowest_disk_device.controllerKey + disk_type = _get_device_disk_type(lowest_disk_device) + # Get the adapter type for the disk-device (if found). adapter_type = adapter_type_dict.get(vmdk_controller_key) return VmdkInfo(vmdk_file_path, adapter_type, disk_type, - capacity_in_bytes, vmdk_device) + capacity_in_bytes, lowest_disk_device) scsi_controller_classes = { diff --git a/nova/virt/vmwareapi/vmops.py b/nova/virt/vmwareapi/vmops.py index 7da453bdb1..8660d7aae9 100644 --- a/nova/virt/vmwareapi/vmops.py +++ b/nova/virt/vmwareapi/vmops.py @@ -987,8 +987,7 @@ class VMwareVMOps(object): def _get_vm_and_vmdk_attribs(): # Get the vmdk info that the VM is pointing to - vmdk = vm_util.get_vmdk_info(self._session, vm_ref, - instance.uuid) + vmdk = vm_util.get_vmdk_info(self._session, vm_ref) if not vmdk.path: LOG.debug("No root disk defined. Unable to snapshot.", instance=instance) @@ -1204,8 +1203,7 @@ class VMwareVMOps(object): vm_ref = vm_util.get_vm_ref(self._session, instance) # Get the root disk vmdk object - vmdk = vm_util.get_vmdk_info(self._session, vm_ref, - uuid=instance.uuid) + vmdk = vm_util.get_vmdk_info(self._session, vm_ref) ds_ref = vmdk.device.backing.datastore datastore = ds_obj.get_datastore_by_ref(self._session, ds_ref) dc_info = self.get_datacenter_ref_and_name(datastore.ref) @@ -1412,8 +1410,7 @@ class VMwareVMOps(object): def _resize_create_ephemerals_and_swap(self, vm_ref, instance, block_device_info): - vmdk = vm_util.get_vmdk_info(self._session, vm_ref, - uuid=instance.uuid) + vmdk = vm_util.get_vmdk_info(self._session, vm_ref) if not vmdk.device: LOG.debug("No root disk attached!", instance=instance) return @@ -1432,8 +1429,7 @@ class VMwareVMOps(object): off the instance before the end. """ vm_ref = vm_util.get_vm_ref(self._session, instance) - vmdk = vm_util.get_vmdk_info(self._session, vm_ref, - uuid=instance.uuid) + vmdk = vm_util.get_vmdk_info(self._session, vm_ref) # Checks if the migration needs a disk resize down. if (flavor.root_gb < instance.flavor.root_gb or @@ -1478,8 +1474,7 @@ class VMwareVMOps(object): def confirm_migration(self, migration, instance, network_info): """Confirms a resize, destroying the source VM.""" vm_ref = vm_util.get_vm_ref(self._session, instance) - vmdk = vm_util.get_vmdk_info(self._session, vm_ref, - uuid=instance.uuid) + vmdk = vm_util.get_vmdk_info(self._session, vm_ref) if not vmdk.device: return ds_ref = vmdk.device.backing.datastore @@ -1544,8 +1539,7 @@ class VMwareVMOps(object): metadata=metadata) vm_util.reconfigure_vm(self._session, vm_ref, vm_resize_spec) - vmdk = vm_util.get_vmdk_info(self._session, vm_ref, - uuid=instance.uuid) + vmdk = vm_util.get_vmdk_info(self._session, vm_ref) if vmdk.device: self._revert_migration_update_disks(vm_ref, instance, vmdk, block_device_info) @@ -1595,7 +1589,7 @@ class VMwareVMOps(object): migrated to. Return the current datastore if it is already connected to the specified cluster. """ - vmdk = vm_util.get_vmdk_info(self._session, vm_ref, uuid=instance.uuid) + vmdk = vm_util.get_vmdk_info(self._session, vm_ref) ds_ref = vmdk.device.backing.datastore cluster_datastores = self._session._call_method(vutil, 'get_object_property',