openstack
/
os-win
Code Issues Proposed changes

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
This commit is contained in:
Claudiu Belu 2017-02-07 00:08:18 +02:00
parent b1982dc8da
commit 6c5d895ba4
4 changed files with 37 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
os_win/tests/unit/utils/compute/test_vmutils.py
View File

@ -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,

4
os_win/tests/unit/utils/metrics/test_metricsutils.py
View File

@ -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)

40
os_win/utils/compute/vmutils.py
View File

@ -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.

3
os_win/utils/metrics/metricsutils.py
View File

@ -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