From 9cf03af2784fb59238de5443f9121ea2f703537f Mon Sep 17 00:00:00 2001 From: Vipin Balachandran Date: Fri, 1 Feb 2019 14:51:29 -0800 Subject: [PATCH] VMware: Detach backing vmdk during disconnect During restore, the backing vmdk is overwritten using backup data. If the restored vmdk has a different uuid or size than the original one, it will not be reflected in vSphere. Due to this stale metadata, there is no way we can fix the uuid or backing vmdk size if it changes after restore. The recommended way is to detach the backing vmdk from the backend volume (shadow VM), delete it and attach the restored vmdk. This will ensure update of current vmdk size and uuid in vSphere. Change-Id: Ib3d5c115f9076c19c3e33eaf6cccf630439a4c26 Partial-bug: 1814346 Partial-bug: 1814347 --- os_brick/initiator/connectors/vmware.py | 76 ++++++++++-- .../tests/initiator/connectors/test_vmware.py | 115 +++++++++++++++--- 2 files changed, 161 insertions(+), 30 deletions(-) diff --git a/os_brick/initiator/connectors/vmware.py b/os_brick/initiator/connectors/vmware.py index 7d5d34122..cad56ea50 100644 --- a/os_brick/initiator/connectors/vmware.py +++ b/os_brick/initiator/connectors/vmware.py @@ -189,7 +189,64 @@ class VmdkConnector(initiator_connector.InitiatorConnector): cacerts=cacerts) image_transfer._start_transfer(read_handle, write_handle, timeout_secs) - def _disconnect(self, tmp_file_path, session, ds_ref, dc_ref, vmdk_path): + def _get_disk_device(self, session, backing): + hardware_devices = session.invoke_api(vim_util, + 'get_object_property', + session.vim, + backing, + 'config.hardware.device') + if hardware_devices.__class__.__name__ == "ArrayOfVirtualDevice": + hardware_devices = hardware_devices.VirtualDevice + for device in hardware_devices: + if device.__class__.__name__ == "VirtualDisk": + return device + + def _create_spec_for_disk_remove(self, session, disk_device): + cf = session.vim.client.factory + disk_spec = cf.create('ns0:VirtualDeviceConfigSpec') + disk_spec.operation = 'remove' + disk_spec.fileOperation = 'destroy' + disk_spec.device = disk_device + return disk_spec + + def _reconfigure_backing(self, session, backing, reconfig_spec): + LOG.debug("Reconfiguring backing VM: %(backing)s with spec: %(spec)s.", + {'backing': backing, + 'spec': reconfig_spec}) + reconfig_task = session.invoke_api(session.vim, + "ReconfigVM_Task", + backing, + spec=reconfig_spec) + LOG.debug("Task: %s created for reconfiguring backing VM.", + reconfig_task) + session.wait_for_task(reconfig_task) + + def _detach_disk_from_backing(self, session, backing, disk_device): + LOG.debug("Reconfiguring backing VM: %(backing)s to remove disk: " + "%(disk_device)s.", + {'backing': backing, 'disk_device': disk_device}) + + cf = session.vim.client.factory + reconfig_spec = cf.create('ns0:VirtualMachineConfigSpec') + spec = self._create_spec_for_disk_remove(session, disk_device) + reconfig_spec.deviceChange = [spec] + self._reconfigure_backing(session, backing, reconfig_spec) + + def _attach_disk_to_backing(self, session, backing, disk_device): + LOG.debug("Reconfiguring backing VM: %(backing)s to add disk: " + "%(disk_device)s.", + {'backing': backing, 'disk_device': disk_device}) + + cf = session.vim.client.factory + reconfig_spec = cf.create('ns0:VirtualMachineConfigSpec') + disk_spec = cf.create('ns0:VirtualDeviceConfigSpec') + disk_spec.operation = 'add' + disk_spec.device = disk_device + reconfig_spec.deviceChange = [disk_spec] + self._reconfigure_backing(session, backing, reconfig_spec) + + def _disconnect( + self, backing, tmp_file_path, session, ds_ref, dc_ref, vmdk_path): # The restored volume is in compressed (streamOptimized) format. # So we upload it to a temporary location in vCenter datastore and copy # the compressed vmdk to the volume vmdk. The copy operation @@ -212,20 +269,13 @@ class VmdkConnector(initiator_connector.InitiatorConnector): ds_path.rel_path, os.path.getsize(tmp_file_path), cacerts, self._timeout) - # Delete the current volume vmdk because the copy operation does not - # overwrite. - LOG.debug("Deleting %s", vmdk_path) - disk_mgr = session.vim.service_content.virtualDiskManager - task = session.invoke_api(session.vim, - 'DeleteVirtualDisk_Task', - disk_mgr, - name=vmdk_path, - datacenter=dc_ref) - session.wait_for_task(task) + disk_device = self._get_disk_device(session, backing) + self._detach_disk_from_backing(session, backing, disk_device) src = six.text_type(ds_path) LOG.debug("Copying %(src)s to %(dest)s", {'src': src, 'dest': vmdk_path}) + disk_mgr = session.vim.service_content.virtualDiskManager task = session.invoke_api(session.vim, 'CopyVirtualDisk_Task', disk_mgr, @@ -235,6 +285,8 @@ class VmdkConnector(initiator_connector.InitiatorConnector): destDatacenter=dc_ref) session.wait_for_task(task) + self._attach_disk_to_backing(session, backing, disk_device) + # Delete the compressed vmdk at the temporary location. LOG.debug("Deleting %s", src) file_mgr = session.vim.service_content.fileManager @@ -275,7 +327,7 @@ class VmdkConnector(initiator_connector.InitiatorConnector): connection_properties['datacenter'], "Datacenter") vmdk_path = connection_properties['vmdk_path'] self._disconnect( - tmp_file_path, session, ds_ref, dc_ref, vmdk_path) + backing, tmp_file_path, session, ds_ref, dc_ref, vmdk_path) finally: os.remove(tmp_file_path) if session: diff --git a/os_brick/tests/initiator/connectors/test_vmware.py b/os_brick/tests/initiator/connectors/test_vmware.py index 539d7b5f1..cdeed551c 100644 --- a/os_brick/tests/initiator/connectors/test_vmware.py +++ b/os_brick/tests/initiator/connectors/test_vmware.py @@ -221,9 +221,13 @@ class VmdkConnectorTestCase(test_connector.ConnectorTestCase): @mock.patch('os_brick.initiator.connectors.vmware.open', create=True) @mock.patch.object(VMDK_CONNECTOR, '_upload_vmdk') @mock.patch('os.path.getsize') + @mock.patch.object(VMDK_CONNECTOR, '_get_disk_device') + @mock.patch.object(VMDK_CONNECTOR, '_detach_disk_from_backing') + @mock.patch.object(VMDK_CONNECTOR, '_attach_disk_to_backing') def test_disconnect( - self, getsize, upload_vmdk, file_open, create_temp_ds_folder, - get_ds_by_ref): + self, attach_disk_to_backing, detach_disk_from_backing, + get_disk_device, getsize, upload_vmdk, file_open, + create_temp_ds_folder, get_ds_by_ref): ds_ref = mock.sentinel.ds_ref ds_name = 'datastore-1' dstore = datastore.Datastore(ds_ref, ds_name) @@ -236,20 +240,21 @@ class VmdkConnectorTestCase(test_connector.ConnectorTestCase): file_open.return_value = file_open_ret dc_name = mock.sentinel.dc_name - delete_task = mock.sentinel.delete_vdisk_task copy_task = mock.sentinel.copy_vdisk_task delete_file_task = mock.sentinel.delete_file_task session = mock.Mock() - session.invoke_api.side_effect = [ - dc_name, delete_task, copy_task, delete_file_task] + session.invoke_api.side_effect = [dc_name, copy_task, delete_file_task] getsize.return_value = units.Gi + disk_device = mock.sentinel.disk_device + get_disk_device.return_value = disk_device + backing = mock.sentinel.backing tmp_file_path = '/tmp/foo.vmdk' dc_ref = mock.sentinel.dc_ref vmdk_path = mock.sentinel.vmdk_path self._connector._disconnect( - tmp_file_path, session, ds_ref, dc_ref, vmdk_path) + backing, tmp_file_path, session, ds_ref, dc_ref, vmdk_path) tmp_folder_path = self._connector.TMP_IMAGES_DATASTORE_FOLDER_PATH ds_folder_path = '[%s] %s' % (ds_name, tmp_folder_path) @@ -268,30 +273,30 @@ class VmdkConnectorTestCase(test_connector.ConnectorTestCase): exp_rel_path, units.Gi, self._connector._ca_file, self._connector._timeout) - disk_mgr = session.vim.service_content.virtualDiskManager - self.assertEqual( - mock.call(session.vim, 'DeleteVirtualDisk_Task', disk_mgr, - name=vmdk_path, datacenter=dc_ref), - session.invoke_api.call_args_list[1]) - self.assertEqual(mock.call(delete_task), - session.wait_for_task.call_args_list[0]) + get_disk_device.assert_called_once_with(session, backing) + detach_disk_from_backing.assert_called_once_with( + session, backing, disk_device) src = '[%s] %s' % (ds_name, exp_rel_path) + disk_mgr = session.vim.service_content.virtualDiskManager self.assertEqual( mock.call(session.vim, 'CopyVirtualDisk_Task', disk_mgr, sourceName=src, sourceDatacenter=dc_ref, destName=vmdk_path, destDatacenter=dc_ref), - session.invoke_api.call_args_list[2]) + session.invoke_api.call_args_list[1]) self.assertEqual(mock.call(copy_task), - session.wait_for_task.call_args_list[1]) + session.wait_for_task.call_args_list[0]) + + attach_disk_to_backing.assert_called_once_with( + session, backing, disk_device) file_mgr = session.vim.service_content.fileManager self.assertEqual( mock.call(session.vim, 'DeleteDatastoreFile_Task', file_mgr, name=src, datacenter=dc_ref), - session.invoke_api.call_args_list[3]) + session.invoke_api.call_args_list[2]) self.assertEqual(mock.call(delete_file_task), - session.wait_for_task.call_args_list[2]) + session.wait_for_task.call_args_list[1]) @mock.patch('os.path.exists') def test_disconnect_volume_with_missing_temp_file(self, path_exists): @@ -361,6 +366,80 @@ class VmdkConnectorTestCase(test_connector.ConnectorTestCase): create_session.assert_called_once_with() snapshot_exists.assert_called_once_with(session, backing) disconnect.assert_called_once_with( - path, session, ds_ref, dc_ref, props['vmdk_path']) + backing, path, session, ds_ref, dc_ref, props['vmdk_path']) remove.assert_called_once_with(path) session.logout.assert_called_once_with() + + def test_get_disk_device(self): + disk_device = mock.Mock() + disk_device.__class__.__name__ = 'VirtualDisk' + + controller_device = mock.Mock() + controller_device.__class__.__name__ = 'VirtualLSILogicController' + + devices = mock.Mock() + devices.__class__.__name__ = "ArrayOfVirtualDevice" + devices.VirtualDevice = [disk_device, controller_device] + session = mock.Mock() + session.invoke_api.return_value = devices + + backing = mock.sentinel.backing + self.assertEqual(disk_device, + self._connector._get_disk_device(session, backing)) + session.invoke_api.assert_called_once_with( + vim_util, 'get_object_property', session.vim, + backing, 'config.hardware.device') + + def test_create_spec_for_disk_remove(self): + disk_spec = mock.Mock() + session = mock.Mock() + session.vim.client.factory.create.return_value = disk_spec + + disk_device = mock.sentinel.disk_device + self._connector._create_spec_for_disk_remove(session, disk_device) + + session.vim.client.factory.create.assert_called_once_with( + 'ns0:VirtualDeviceConfigSpec') + self.assertEqual('remove', disk_spec.operation) + self.assertEqual('destroy', disk_spec.fileOperation) + self.assertEqual(disk_device, disk_spec.device) + + @mock.patch.object(VMDK_CONNECTOR, '_create_spec_for_disk_remove') + @mock.patch.object(VMDK_CONNECTOR, '_reconfigure_backing') + def test_detach_disk_from_backing(self, reconfigure_backing, create_spec): + disk_spec = mock.sentinel.disk_spec + create_spec.return_value = disk_spec + + reconfig_spec = mock.Mock() + session = mock.Mock() + session.vim.client.factory.create.return_value = reconfig_spec + + backing = mock.sentinel.backing + disk_device = mock.sentinel.disk_device + self._connector._detach_disk_from_backing( + session, backing, disk_device) + + create_spec.assert_called_once_with(session, disk_device) + session.vim.client.factory.create.assert_called_once_with( + 'ns0:VirtualMachineConfigSpec') + self.assertEqual([disk_spec], reconfig_spec.deviceChange) + reconfigure_backing.assert_called_once_with( + session, backing, reconfig_spec) + + @mock.patch.object(VMDK_CONNECTOR, '_reconfigure_backing') + def test_attach_disk_to_backing(self, reconfigure_backing): + reconfig_spec = mock.Mock() + disk_spec = mock.Mock() + session = mock.Mock() + session.vim.client.factory.create.side_effect = [ + reconfig_spec, disk_spec] + + backing = mock.Mock() + disk_device = mock.sentinel.disk_device + self._connector._attach_disk_to_backing(session, backing, disk_device) + + self.assertEqual([disk_spec], reconfig_spec.deviceChange) + self.assertEqual('add', disk_spec.operation) + self.assertEqual(disk_device, disk_spec.device) + reconfigure_backing.assert_called_once_with( + session, backing, reconfig_spec)