From 6c5d895ba4984464e1e11a4fcd6e83d82a3d6893 Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Tue, 7 Feb 2017 00:08:18 +0200 Subject: [PATCH] removes the VirtualSystemType kwarg when fetching VMs A VM can be either "Realized" or "Planned", but not both. Thus, using the VirtualSystemType as a filter is not useful. Because of this, the "is_planned_vm" argument from vmutils methods will no longer be unused, and will be removed once it is no longer used. This will make vmutils easier to use for Planned VMs, which will be used for cold migration in the future. Plus, not all vmutils methods have the "is_planned_vm" argument, which would have been necessary in order to update the Planned VM (e.g.: get_vm_config_dir method). Partial-Bug: #1663238 Change-Id: Ic153dcfd621a1e6235dd3a75241274ee7d7c868b --- .../tests/unit/utils/compute/test_vmutils.py | 74 ++++++------------- .../unit/utils/metrics/test_metricsutils.py | 4 +- os_win/utils/compute/vmutils.py | 40 ++++------ os_win/utils/metrics/metricsutils.py | 3 +- 4 files changed, 37 insertions(+), 84 deletions(-) diff --git a/os_win/tests/unit/utils/compute/test_vmutils.py b/os_win/tests/unit/utils/compute/test_vmutils.py index df2b7888..91d73f1b 100644 --- a/os_win/tests/unit/utils/compute/test_vmutils.py +++ b/os_win/tests/unit/utils/compute/test_vmutils.py @@ -63,7 +63,6 @@ class VMUtilsTestCase(test_base.OsWinBaseTestCase): _DEFINE_SYSTEM = 'DefineSystem' _DESTROY_SYSTEM = 'DestroySystem' _DESTROY_SNAPSHOT = 'DestroySnapshot' - _SETTING_TYPE = 'VirtualSystemType' _VM_GEN = constants.VM_GEN_2 _VIRTUAL_SYSTEM_TYPE_REALIZED = 'Microsoft:Hyper-V:System:Realized' @@ -118,11 +117,15 @@ class VMUtilsTestCase(test_base.OsWinBaseTestCase): as_vssd=False) def test_lookup_vm_as_vssd(self): + vssd = mock.MagicMock() + expected_vssd = mock.MagicMock( + VirtualSystemType=self._vmutils._VIRTUAL_SYSTEM_TYPE_REALIZED) + self._vmutils._conn.Msvm_VirtualSystemSettingData.return_value = [ - mock.sentinel.fake_vssd] + vssd, expected_vssd] vssd = self._vmutils._lookup_vm_check(self._FAKE_VM_NAME) - self.assertEqual(mock.sentinel.fake_vssd, vssd) + self.assertEqual(expected_vssd, vssd) def test_set_vm_memory_static(self): self._test_set_vm_memory_dynamic(dynamic_memory_ratio=1.0) @@ -217,40 +220,27 @@ class VMUtilsTestCase(test_base.OsWinBaseTestCase): @mock.patch.object(vmutils.VMUtils, '_get_vm_disks') @mock.patch.object(vmutils.VMUtils, '_lookup_vm_check') - @mock.patch.object(vmutils.VMUtils, '_get_virtual_system_type') - def test_get_vm_storage_paths(self, mock_get_virtual_system_type, - mock_lookup_vm_check, mock_get_vm_disks): - virtual_system_type = mock_get_virtual_system_type.return_value + def test_get_vm_storage_paths(self, mock_lookup_vm_check, + mock_get_vm_disks): mock_rasds = self._create_mock_disks() mock_get_vm_disks.return_value = ([mock_rasds[0]], [mock_rasds[1]]) - storage = self._vmutils.get_vm_storage_paths( - self._FAKE_VM_NAME, - is_planned_vm=False) + storage = self._vmutils.get_vm_storage_paths(self._FAKE_VM_NAME) (disk_files, volume_drives) = storage self.assertEqual([self._FAKE_VHD_PATH], disk_files) self.assertEqual([self._FAKE_VOLUME_DRIVE_PATH], volume_drives) - mock_get_virtual_system_type.assert_called_once_with(False) - mock_lookup_vm_check.assert_called_once_with( - self._FAKE_VM_NAME, virtual_system_type=virtual_system_type) + mock_lookup_vm_check.assert_called_once_with(self._FAKE_VM_NAME) @mock.patch.object(vmutils.VMUtils, '_get_vm_disks') - @mock.patch.object(vmutils.VMUtils, '_get_virtual_system_type') - def test_get_vm_disks_by_instance_name(self, - mock_get_virtual_system_type, - mock_get_vm_disks): - virtual_system_type = mock_get_virtual_system_type.return_value + def test_get_vm_disks_by_instance_name(self, mock_get_vm_disks): self._lookup_vm() mock_get_vm_disks.return_value = mock.sentinel.vm_disks - vm_disks = self._vmutils.get_vm_disks( - self._FAKE_VM_NAME, is_planned_vm=False) + vm_disks = self._vmutils.get_vm_disks(self._FAKE_VM_NAME) - mock_get_virtual_system_type.assert_called_once_with(False) self._vmutils._lookup_vm_check.assert_called_once_with( - self._FAKE_VM_NAME, - virtual_system_type=virtual_system_type) + self._FAKE_VM_NAME) self.assertEqual(mock.sentinel.vm_disks, vm_disks) @mock.patch.object(_wqlutils, 'get_element_associated_class') @@ -304,7 +294,6 @@ class VMUtilsTestCase(test_base.OsWinBaseTestCase): @ddt.data( {'configuration_root_dir': mock.sentinel.configuration_root_dir}, - {'is_planned_vm': True}, {'host_shutdown_action': mock.sentinel.shutdown_action}, {}) @ddt.unpack @@ -312,26 +301,19 @@ class VMUtilsTestCase(test_base.OsWinBaseTestCase): @mock.patch.object(vmutils.VMUtils, '_set_vm_vcpus') @mock.patch.object(vmutils.VMUtils, '_set_vm_memory') @mock.patch.object(vmutils.VMUtils, '_lookup_vm_check') - @mock.patch.object(vmutils.VMUtils, '_get_virtual_system_type') - def test_update_vm(self, mock_get_virtual_system_type, - mock_lookup_vm_check, mock_set_mem, mock_set_vcpus, - mock_modify_virtual_system, + def test_update_vm(self, mock_lookup_vm_check, mock_set_mem, + mock_set_vcpus, mock_modify_virtual_system, host_shutdown_action=None, - configuration_root_dir=None, - is_planned_vm=False): + configuration_root_dir=None): mock_vmsettings = mock_lookup_vm_check.return_value - virtual_system_type = mock_get_virtual_system_type.return_value self._vmutils.update_vm( mock.sentinel.vm_name, mock.sentinel.memory_mb, mock.sentinel.memory_per_numa, mock.sentinel.vcpus_num, mock.sentinel.vcpus_per_numa, mock.sentinel.limit_cpu_features, mock.sentinel.dynamic_mem_ratio, configuration_root_dir, - host_shutdown_action=host_shutdown_action, - is_planned_vm=is_planned_vm) + host_shutdown_action=host_shutdown_action) - mock_get_virtual_system_type.assert_called_once_with(is_planned_vm) - mock_lookup_vm_check.assert_called_once_with( - mock.sentinel.vm_name, virtual_system_type=virtual_system_type) + mock_lookup_vm_check.assert_called_once_with(mock.sentinel.vm_name) mock_set_mem.assert_called_once_with( mock_vmsettings, mock.sentinel.memory_mb, mock.sentinel.memory_per_numa, mock.sentinel.dynamic_mem_ratio) @@ -377,14 +359,11 @@ class VMUtilsTestCase(test_base.OsWinBaseTestCase): @mock.patch.object(vmutils.VMUtils, '_set_vm_vcpus') @mock.patch.object(vmutils.VMUtils, '_set_vm_memory') def test_old_create_vm(self, mock_set_mem, mock_set_vcpus): + mock_vmsetting = self._lookup_vm() mock_svc = self._vmutils._vs_man_svc getattr(mock_svc, self._DEFINE_SYSTEM).return_value = ( None, self._FAKE_JOB_PATH, self._FAKE_RET_VAL) - mock_vmsetting = mock.MagicMock() - self._vmutils._conn.Msvm_VirtualSystemSettingData.return_value = [ - mock_vmsetting] - self._vmutils.create_vm(self._FAKE_VM_NAME, self._FAKE_MEMORY_MB, self._FAKE_VCPUS_NUM, False, self._FAKE_DYNAMIC_MEMORY_RATIO, @@ -599,11 +578,9 @@ class VMUtilsTestCase(test_base.OsWinBaseTestCase): mock_get_vm_disks.return_value = ([], [mock_phys_disk]) - result = self._vmutils.get_vm_physical_disk_mapping( - self._FAKE_VM_NAME, is_planned_vm=False) + result = self._vmutils.get_vm_physical_disk_mapping(self._FAKE_VM_NAME) self.assertEqual(expected_mapping, result) - mock_get_vm_disks.assert_called_once_with( - self._FAKE_VM_NAME, is_planned_vm=False) + mock_get_vm_disks.assert_called_once_with(self._FAKE_VM_NAME) @mock.patch.object(vmutils.VMUtils, '_get_wmi_obj') def test_set_disk_host_res(self, mock_get_wmi_obj): @@ -1309,15 +1286,6 @@ class VMUtilsTestCase(test_base.OsWinBaseTestCase): disk_resource) self.assertEqual(disk_resource.HostResource, [mock.sentinel.new_path]) - @ddt.data(True, False) - def test_get_virtual_system_type(self, is_planned_vm): - exp_result = ( - self._vmutils._VIRTUAL_SYSTEM_TYPE_PLANNED if is_planned_vm - else self._vmutils._VIRTUAL_SYSTEM_TYPE_REALIZED) - actual_result = self._vmutils._get_virtual_system_type( - is_planned_vm=is_planned_vm) - self.assertEqual(exp_result, actual_result) - def test_add_pci_device(self): self.assertRaises(NotImplementedError, self._vmutils.add_pci_device, diff --git a/os_win/tests/unit/utils/metrics/test_metricsutils.py b/os_win/tests/unit/utils/metrics/test_metricsutils.py index 62c33fa5..cbc43a75 100644 --- a/os_win/tests/unit/utils/metrics/test_metricsutils.py +++ b/os_win/tests/unit/utils/metrics/test_metricsutils.py @@ -387,9 +387,7 @@ class MetricsUtilsTestCase(test_base.OsWinBaseTestCase): self.assertEqual(mock_unique_result.return_value, result) conn_class = self.utils._conn.Msvm_VirtualSystemSettingData - conn_class.assert_called_once_with( - ElementName=mock.sentinel.vm_name, - VirtualSystemType=self.utils._VIRTUAL_SYSTEM_TYPE_REALIZED) + conn_class.assert_called_once_with(ElementName=mock.sentinel.vm_name) mock_unique_result.assert_called_once_with(conn_class.return_value, mock.sentinel.vm_name) diff --git a/os_win/utils/compute/vmutils.py b/os_win/utils/compute/vmutils.py index 0214303e..d1c7c204 100644 --- a/os_win/utils/compute/vmutils.py +++ b/os_win/utils/compute/vmutils.py @@ -42,6 +42,9 @@ from os_win.utils import pathutils LOG = logging.getLogger(__name__) +# TODO(claudiub): remove the is_planned_vm argument from methods once it is not +# used anymore. + class VMUtils(baseutils.BaseUtilsVirt): @@ -190,21 +193,19 @@ class VMUtils(baseutils.BaseUtilsVirt): settings = self.get_vm_summary_info(vm_name) return settings['EnabledState'] - def _lookup_vm_check(self, vm_name, as_vssd=True, for_update=False, - virtual_system_type=_VIRTUAL_SYSTEM_TYPE_REALIZED): - vm = self._lookup_vm(vm_name, as_vssd, for_update, - virtual_system_type) + def _lookup_vm_check(self, vm_name, as_vssd=True, for_update=False): + vm = self._lookup_vm(vm_name, as_vssd, for_update) if not vm: raise exceptions.HyperVVMNotFoundException(vm_name=vm_name) return vm - def _lookup_vm(self, vm_name, as_vssd=True, for_update=False, - virtual_system_type=_VIRTUAL_SYSTEM_TYPE_REALIZED): + def _lookup_vm(self, vm_name, as_vssd=True, for_update=False): if as_vssd: conn = self._compat_conn if for_update else self._conn - vms = conn.Msvm_VirtualSystemSettingData( - ElementName=vm_name, - VirtualSystemType=virtual_system_type) + vms = conn.Msvm_VirtualSystemSettingData(ElementName=vm_name) + vms = [v for v in vms if + v.VirtualSystemType in [self._VIRTUAL_SYSTEM_TYPE_PLANNED, + self._VIRTUAL_SYSTEM_TYPE_REALIZED]] else: vms = self._conn.Msvm_ComputerSystem(ElementName=vm_name) n = len(vms) @@ -283,10 +284,7 @@ class VMUtils(baseutils.BaseUtilsVirt): configuration_root_dir=None, snapshot_dir=None, host_shutdown_action=None, is_planned_vm=False): - virtual_system_type = self._get_virtual_system_type(is_planned_vm) - - vmsetting = self._lookup_vm_check( - vm_name, virtual_system_type=virtual_system_type) + vmsetting = self._lookup_vm_check(vm_name) if host_shutdown_action: vmsetting.AutomaticShutdownAction = host_shutdown_action @@ -525,7 +523,7 @@ class VMUtils(baseutils.BaseUtilsVirt): def get_vm_physical_disk_mapping(self, vm_name, is_planned_vm=False): mapping = {} physical_disks = ( - self.get_vm_disks(vm_name, is_planned_vm=is_planned_vm)[1]) + self.get_vm_disks(vm_name)[1]) for diskdrive in physical_disks: mapping[diskdrive.ElementName] = dict( resource_path=diskdrive.path_(), @@ -635,10 +633,7 @@ class VMUtils(baseutils.BaseUtilsVirt): return vmsettings.ConfigurationDataRoot def get_vm_storage_paths(self, vm_name, is_planned_vm=False): - virtual_system_type = self._get_virtual_system_type(is_planned_vm) - vmsettings = self._lookup_vm_check( - vm_name, - virtual_system_type=virtual_system_type) + vmsettings = self._lookup_vm_check(vm_name) (disk_resources, volume_resources) = self._get_vm_disks(vmsettings) volume_drives = [] @@ -654,9 +649,7 @@ class VMUtils(baseutils.BaseUtilsVirt): return (disk_files, volume_drives) def get_vm_disks(self, vm_name, is_planned_vm=False): - virtual_system_type = self._get_virtual_system_type(is_planned_vm) - vmsettings = self._lookup_vm_check( - vm_name, virtual_system_type=virtual_system_type) + vmsettings = self._lookup_vm_check(vm_name) return self._get_vm_disks(vmsettings) def _get_vm_disks(self, vmsettings): @@ -1125,11 +1118,6 @@ class VMUtils(baseutils.BaseUtilsVirt): disk_resource.HostResource = [new_disk_path] self._jobutils.modify_virt_resource(disk_resource) - def _get_virtual_system_type(self, is_planned_vm): - return ( - self._VIRTUAL_SYSTEM_TYPE_PLANNED if is_planned_vm - else self._VIRTUAL_SYSTEM_TYPE_REALIZED) - def add_pci_device(self, vm_name, vendor_id, product_id): """Adds the given PCI device to the given VM. diff --git a/os_win/utils/metrics/metricsutils.py b/os_win/utils/metrics/metricsutils.py index 3b982c08..bb549399 100644 --- a/os_win/utils/metrics/metricsutils.py +++ b/os_win/utils/metrics/metricsutils.py @@ -270,8 +270,7 @@ class MetricsUtils(baseutils.BaseUtilsVirt): def _get_vm_setting_data(self, vm_name): vssds = self._conn.Msvm_VirtualSystemSettingData( - ElementName=vm_name, - VirtualSystemType=self._VIRTUAL_SYSTEM_TYPE_REALIZED) + ElementName=vm_name) return self._unique_result(vssds, vm_name) @staticmethod