diff --git a/nova/tests/virt/hyperv/test_vmutils.py b/nova/tests/virt/hyperv/test_vmutils.py index 7ee806201ba8..0dad728ff6bb 100644 --- a/nova/tests/virt/hyperv/test_vmutils.py +++ b/nova/tests/virt/hyperv/test_vmutils.py @@ -79,33 +79,42 @@ class VMUtilsTestCase(test.NoDBTestCase): else: self.assertFalse(mock_s.DynamicMemoryEnabled) - def test_get_vm_storage_paths(self): - mock_vm = self._lookup_vm() - - mock_vmsettings = [mock.MagicMock()] - mock_vm.associators.return_value = mock_vmsettings - mock_rasds = [] - mock_rasd1 = mock.MagicMock() - mock_rasd1.ResourceSubType = self._vmutils._IDE_DISK_RES_SUB_TYPE - mock_rasd1.Connection = [self._FAKE_VHD_PATH] - mock_rasd2 = mock.MagicMock() - mock_rasd2.ResourceSubType = self._vmutils._IDE_DVD_RES_SUB_TYPE - mock_rasd2.Connection = [self._FAKE_DVD_PATH] - mock_rasd3 = mock.MagicMock() - mock_rasd3.ResourceSubType = self._vmutils._PHYS_DISK_RES_SUB_TYPE - mock_rasd3.HostResource = [self._FAKE_VOLUME_DRIVE_PATH] - mock_rasds.append(mock_rasd1) - mock_rasds.append(mock_rasd2) - mock_rasds.append(mock_rasd3) - mock_vmsettings[0].associators.return_value = mock_rasds + @mock.patch('nova.virt.hyperv.vmutils.VMUtils._get_vm_disks') + def test_get_vm_storage_paths(self, mock_get_vm_disks): + self._lookup_vm() + 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) (disk_files, volume_drives) = storage - mock_vm.associators.assert_called_with( - wmi_result_class='Msvm_VirtualSystemSettingData') - mock_vmsettings[0].associators.assert_called_with( - wmi_result_class='Msvm_ResourceAllocationSettingData') - self.assertEqual([self._FAKE_VHD_PATH, self._FAKE_DVD_PATH], - disk_files) + self.assertEqual([self._FAKE_VHD_PATH], disk_files) self.assertEqual([self._FAKE_VOLUME_DRIVE_PATH], volume_drives) + + def test_get_vm_disks(self): + mock_vm = self._lookup_vm() + mock_vmsettings = [mock.MagicMock()] + mock_vm.associators.return_value = mock_vmsettings + + mock_rasds = self._create_mock_disks() + mock_vmsettings[0].associators.return_value = mock_rasds + + (disks, volumes) = self._vmutils._get_vm_disks(mock_vm) + + mock_vm.associators.assert_called_with( + wmi_result_class=self._vmutils._VIRTUAL_SYSTEM_SETTING_DATA_CLASS) + mock_vmsettings[0].associators.assert_called_with( + wmi_result_class=self._vmutils._STORAGE_ALLOC_SETTING_DATA_CLASS) + self.assertEqual([mock_rasds[0]], disks) + self.assertEqual([mock_rasds[1]], volumes) + + def _create_mock_disks(self): + mock_rasd1 = mock.MagicMock() + mock_rasd1.ResourceSubType = self._vmutils._IDE_DISK_RES_SUB_TYPE + mock_rasd1.Connection = [self._FAKE_VHD_PATH] + + mock_rasd2 = mock.MagicMock() + mock_rasd2.ResourceSubType = self._vmutils._PHYS_DISK_RES_SUB_TYPE + mock_rasd2.HostResource = [self._FAKE_VOLUME_DRIVE_PATH] + + return [mock_rasd1, mock_rasd2] diff --git a/nova/tests/virt/hyperv/test_vmutilsv2.py b/nova/tests/virt/hyperv/test_vmutilsv2.py index 5164d43157ab..95bb681b835b 100644 --- a/nova/tests/virt/hyperv/test_vmutilsv2.py +++ b/nova/tests/virt/hyperv/test_vmutilsv2.py @@ -242,25 +242,30 @@ class VMUtilsV2TestCase(test.NoDBTestCase): mock_svc.RemoveResourceSettings.assert_called_with( [self._FAKE_RES_PATH]) - def test_enable_vm_metrics_collection(self): + @mock.patch('nova.virt.hyperv.vmutils.VMUtils._get_vm_disks') + def test_enable_vm_metrics_collection(self, mock_get_vm_disks): self._lookup_vm() mock_svc = self._vmutils._conn.Msvm_MetricService()[0] metric_def = mock.MagicMock() + mock_disk = mock.MagicMock() + mock_disk.path_.return_value = self._FAKE_RES_PATH + mock_get_vm_disks.return_value = ([mock_disk], [mock_disk]) + + fake_metric_def_paths = ["fake_0", None] + fake_metric_resource_paths = [self._FAKE_VM_PATH, self._FAKE_RES_PATH] - fake_metric_def_paths = ["fake_0", "fake_1", "fake_2"] metric_def.path_.side_effect = fake_metric_def_paths - self._vmutils._conn.CIM_BaseMetricDefinition.return_value = [ metric_def] self._vmutils.enable_vm_metrics_collection(self._FAKE_VM_NAME) calls = [] - for fake_metric_def_path in fake_metric_def_paths: + for i in range(len(fake_metric_def_paths)): calls.append(mock.call( - Subject=self._FAKE_VM_PATH, - Definition=fake_metric_def_path, + Subject=fake_metric_resource_paths[i], + Definition=fake_metric_def_paths[i], MetricCollectionEnabled=self._vmutils._METRIC_ENABLED)) mock_svc.ControlMetrics.assert_has_calls(calls, any_order=True) diff --git a/nova/virt/hyperv/vmutils.py b/nova/virt/hyperv/vmutils.py index cb9efd9c6b1c..7df8169fd2d7 100644 --- a/nova/virt/hyperv/vmutils.py +++ b/nova/virt/hyperv/vmutils.py @@ -396,18 +396,7 @@ class VMUtils(object): def get_vm_storage_paths(self, vm_name): vm = self._lookup_vm_check(vm_name) - - vmsettings = vm.associators( - wmi_result_class=self._VIRTUAL_SYSTEM_SETTING_DATA_CLASS) - rasds = vmsettings[0].associators( - wmi_result_class=self._STORAGE_ALLOC_SETTING_DATA_CLASS) - disk_resources = [r for r in rasds - if r.ResourceSubType in - [self._IDE_DISK_RES_SUB_TYPE, - self._IDE_DVD_RES_SUB_TYPE]] - volume_resources = [r for r in rasds - if r.ResourceSubType == - self._PHYS_DISK_RES_SUB_TYPE] + (disk_resources, volume_resources) = self._get_vm_disks(vm) volume_drives = [] for volume_resource in volume_resources: @@ -421,6 +410,20 @@ class VMUtils(object): return (disk_files, volume_drives) + def _get_vm_disks(self, vm): + vmsettings = vm.associators( + wmi_result_class=self._VIRTUAL_SYSTEM_SETTING_DATA_CLASS) + rasds = vmsettings[0].associators( + wmi_result_class=self._STORAGE_ALLOC_SETTING_DATA_CLASS) + disk_resources = [r for r in rasds if + r.ResourceSubType in + [self._IDE_DISK_RES_SUB_TYPE, + self._IDE_DVD_RES_SUB_TYPE]] + volume_resources = [r for r in rasds if + r.ResourceSubType == self._PHYS_DISK_RES_SUB_TYPE] + + return (disk_resources, volume_resources) + def destroy_vm(self, vm_name): vm = self._lookup_vm_check(vm_name) diff --git a/nova/virt/hyperv/vmutilsv2.py b/nova/virt/hyperv/vmutilsv2.py index 0ddf44980144..a34553abdb65 100644 --- a/nova/virt/hyperv/vmutilsv2.py +++ b/nova/virt/hyperv/vmutilsv2.py @@ -53,9 +53,6 @@ class VMUtilsV2(vmutils.VMUtils): _SNAPSHOT_FULL = 2 _METRIC_AGGR_CPU_AVG = 'Aggregated Average CPU Utilization' - _METRIC_AGGR_DISK_R = 'Aggregated Disk Data Read' - _METRIC_AGGR_DISK_W = 'Aggregated Disk Data Written' - _METRIC_ENABLED = 2 _STORAGE_ALLOC_SETTING_DATA_CLASS = 'Msvm_StorageAllocationSettingData' @@ -231,19 +228,27 @@ class VMUtilsV2(vmutils.VMUtils): self._add_virt_resource(eth_port_data, vm.path_()) def enable_vm_metrics_collection(self, vm_name): - metric_names = [self._METRIC_AGGR_CPU_AVG, - self._METRIC_AGGR_DISK_R, - self._METRIC_AGGR_DISK_W] + metric_names = [self._METRIC_AGGR_CPU_AVG] vm = self._lookup_vm_check(vm_name) metric_svc = self._conn.Msvm_MetricService()[0] + (disks, volumes) = self._get_vm_disks(vm) + filtered_disks = [d for d in disks if + d.ResourceSubType is not self._IDE_DVD_RES_SUB_TYPE] + + # enable metrics for disk. + for disk in filtered_disks: + self._enable_metrics(metric_svc, disk) for metric_name in metric_names: metric_def = self._conn.CIM_BaseMetricDefinition(Name=metric_name) if not metric_def: LOG.debug(_("Metric not found: %s") % metric_name) else: - metric_svc.ControlMetrics( - Subject=vm.path_(), - Definition=metric_def[0].path_(), - MetricCollectionEnabled=self._METRIC_ENABLED) + self._enable_metrics(metric_svc, vm, metric_def[0].path_()) + + def _enable_metrics(self, metric_svc, element, definition_path=None): + metric_svc.ControlMetrics( + Subject=element.path_(), + Definition=definition_path, + MetricCollectionEnabled=self._METRIC_ENABLED)