This change fixes a bug in the KVM hypervisor where VCPU threads weren't being pinned if an instance was pinned to a single core, and where worker threads weren't being pinned if the VCPU threads were being pinned individually.
This adds a new parameter, worker_cpu_mask, that allows an instance to pin its worker threads to a set of CPU; the mask parameter matches the current cpu_mask, which now applies to the VCPU threads. Tests are added to cover these new paths. There is an additional slight change to ParseCpuMask to avoid code duplication when handling a mask value of "all". CPU Pinning on KVM with tests Working version of CPU pinning on KVM. slight modification to the ParseCpuMask function to reuse code debugging left in old rebased commit removing debugging fixed overly long line Signed-off-by: Emily Bragg <[email protected]> --- lib/hypervisor/hv_kvm/__init__.py | 34 +++++++----- lib/utils/__init__.py | 9 ++-- src/Ganeti/Constants.hs | 5 ++ test/py/ganeti.hypervisor.hv_kvm_unittest.py | 81 ++++++++++++++++++++++++++-- 4 files changed, 107 insertions(+), 22 deletions(-) diff --git a/lib/hypervisor/hv_kvm/__init__.py b/lib/hypervisor/hv_kvm/__init__.py index 1e89c82..4a03bd8 100644 --- a/lib/hypervisor/hv_kvm/__init__.py +++ b/lib/hypervisor/hv_kvm/__init__.py @@ -505,6 +505,7 @@ class KVMHypervisor(hv_base.BaseHypervisor): constants.HV_REBOOT_BEHAVIOR: hv_base.ParamInSet(True, constants.REBOOT_BEHAVIORS), constants.HV_CPU_MASK: hv_base.OPT_MULTI_CPU_MASK_CHECK, + constants.HV_WORKER_CPU_MASK: hv_base.OPT_MULTI_CPU_MASK_CHECK, constants.HV_CPU_TYPE: hv_base.NO_CHECK, constants.HV_CPU_CORES: hv_base.OPT_NONNEGATIVE_INT_CHECK, constants.HV_CPU_THREADS: hv_base.OPT_NONNEGATIVE_INT_CHECK, @@ -893,9 +894,12 @@ class KVMHypervisor(hv_base.BaseHypervisor): target_process.set_cpu_affinity(range(psutil.cpu_count())) else: target_process.set_cpu_affinity(cpus) + for p in target_process.get_children(recursive=True): + p.set_cpu_affinity(cpus) @classmethod - def _AssignCpuAffinity(cls, cpu_mask, process_id, thread_dict): + def _AssignCpuAffinity(cls, cpu_mask, process_id, thread_dict, + worker_cpu_mask): """Change CPU affinity for running VM according to given CPU mask. @param cpu_mask: CPU mask as given by the user. e.g. "0-2,4:all:1,3" @@ -905,20 +909,23 @@ class KVMHypervisor(hv_base.BaseHypervisor): @type process_id: int @param thread_dict: map of virtual CPUs to KVM thread IDs @type thread_dict: dict int:int + @param worker_cpu_mask: CPU mask as given by the user for the worker + threads. e.g. "0-2,4" + @type worker_cpu_mask: string """ - # Convert the string CPU mask to a list of list of int's + worker_cpu_list = utils.ParseCpuMask(worker_cpu_mask) + cls._SetProcessAffinity(process_id, worker_cpu_list) + + # Convert the string CPU mask to a list of list of ints cpu_list = utils.ParseMultiCpuMask(cpu_mask) - if len(cpu_list) == 1: all_cpu_mapping = cpu_list[0] - if all_cpu_mapping == constants.CPU_PINNING_OFF: - # If CPU pinning has 1 entry that's "all", then do nothing - pass - else: - # If CPU pinning has one non-all entry, map the entire VM to - # one set of physical CPUs - cls._SetProcessAffinity(process_id, all_cpu_mapping) + if all_cpu_mapping != constants.CPU_PINNING_OFF: + # The vcpus do not inherit the affinity of the parent process so they + # also must be pinned. + for vcpu in thread_dict: + cls._SetProcessAffinity(thread_dict[vcpu], all_cpu_mapping) else: # The number of vCPUs mapped should match the number of vCPUs # reported by KVM. This was already verified earlier, so @@ -949,7 +956,7 @@ class KVMHypervisor(hv_base.BaseHypervisor): return result - def _ExecuteCpuAffinity(self, instance_name, cpu_mask): + def _ExecuteCpuAffinity(self, instance_name, cpu_mask, worker_cpu_mask): """Complete CPU pinning. @type instance_name: string @@ -963,7 +970,7 @@ class KVMHypervisor(hv_base.BaseHypervisor): # Get vCPU thread IDs, to be used if need to pin vCPUs separately thread_dict = self._GetVcpuThreadIds(instance_name) # Run CPU pinning, based on configured mask - self._AssignCpuAffinity(cpu_mask, pid, thread_dict) + self._AssignCpuAffinity(cpu_mask, pid, thread_dict, worker_cpu_mask) def ListInstances(self, hvparams=None): """Get the list of running instances. @@ -1922,7 +1929,8 @@ class KVMHypervisor(hv_base.BaseHypervisor): # If requested, set CPU affinity and resume instance execution if cpu_pinning: - self._ExecuteCpuAffinity(instance.name, up_hvp[constants.HV_CPU_MASK]) + self._ExecuteCpuAffinity(instance.name, up_hvp[constants.HV_CPU_MASK], + up_hvp[constants.HV_WORKER_CPU_MASK]) start_memory = self._InstanceStartupMemory(instance) if start_memory < instance.beparams[constants.BE_MAXMEM]: diff --git a/lib/utils/__init__.py b/lib/utils/__init__.py index ce89869..d51ad50 100644 --- a/lib/utils/__init__.py +++ b/lib/utils/__init__.py @@ -296,6 +296,8 @@ def ParseCpuMask(cpu_mask): return [] cpu_list = [] for range_def in cpu_mask.split(","): + if range_def == constants.CPU_PINNING_ALL: + return [constants.CPU_PINNING_ALL_VAL] boundaries = range_def.split("-") n_elements = len(boundaries) if n_elements > 2: @@ -335,11 +337,8 @@ def ParseMultiCpuMask(cpu_mask): return [] cpu_list = [] for range_def in cpu_mask.split(constants.CPU_PINNING_SEP): - if range_def == constants.CPU_PINNING_ALL: - cpu_list.append([constants.CPU_PINNING_ALL_VAL, ]) - else: - # Uniquify and sort the list before adding - cpu_list.append(sorted(set(ParseCpuMask(range_def)))) + # Uniquify and sort the list before adding + cpu_list.append(sorted(set(ParseCpuMask(range_def)))) return cpu_list diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs index 77030c3..09ca78d 100644 --- a/src/Ganeti/Constants.hs +++ b/src/Ganeti/Constants.hs @@ -1661,6 +1661,9 @@ hvCpuCores = "cpu_cores" hvCpuMask :: String hvCpuMask = "cpu_mask" +hvWorkerCpuMask :: String +hvWorkerCpuMask = "worker_cpu_mask" + hvCpuSockets :: String hvCpuSockets = "cpu_sockets" @@ -1915,6 +1918,7 @@ hvsParameterTypes = Map.fromList , (hvCpuCap, VTypeInt) , (hvCpuCores, VTypeInt) , (hvCpuMask, VTypeString) + , (hvWorkerCpuMask, VTypeString) , (hvCpuSockets, VTypeInt) , (hvCpuThreads, VTypeInt) , (hvCpuType, VTypeString) @@ -4143,6 +4147,7 @@ hvcDefaults = , (hvMemPath, PyValueEx "") , (hvRebootBehavior, PyValueEx instanceRebootAllowed) , (hvCpuMask, PyValueEx cpuPinningAll) + , (hvWorkerCpuMask, PyValueEx cpuPinningAll) , (hvCpuType, PyValueEx "") , (hvCpuCores, PyValueEx (0 :: Int)) , (hvCpuThreads, PyValueEx (0 :: Int)) diff --git a/test/py/ganeti.hypervisor.hv_kvm_unittest.py b/test/py/ganeti.hypervisor.hv_kvm_unittest.py index 16b1e0a..c8bd838 100755 --- a/test/py/ganeti.hypervisor.hv_kvm_unittest.py +++ b/test/py/ganeti.hypervisor.hv_kvm_unittest.py @@ -578,12 +578,12 @@ class TestKvmRuntime(testutils.GanetiTestCase): self.MockOut(mock.patch(kvm_class + '._CallMonitorCommand')) self.cfg = ConfigMock() - params = constants.HVC_DEFAULTS[constants.HT_KVM].copy() - beparams = constants.BEC_DEFAULTS.copy() + self.params = constants.HVC_DEFAULTS[constants.HT_KVM].copy() + self.beparams = constants.BEC_DEFAULTS.copy() self.instance = self.cfg.AddNewInstance(name='name.example.com', hypervisor='kvm', - hvparams=params, - beparams=beparams) + hvparams=self.params, + beparams=self.beparams) def testDirectoriesCreated(self): hypervisor = hv_kvm.KVMHypervisor() @@ -616,5 +616,78 @@ class TestKvmRuntime(testutils.GanetiTestCase): self.mocks['run_cmd'].side_effect = RunCmd hypervisor.StartInstance(self.instance, [], False) + +class TestKvmCpuPinning(testutils.GanetiTestCase): + def setUp(self): + super(TestKvmCpuPinning, self).setUp() + kvm_class = 'ganeti.hypervisor.hv_kvm.KVMHypervisor' + self.MockOut('qmp', mock.patch('ganeti.hypervisor.hv_kvm.QmpConnection')) + self.MockOut('run_cmd', mock.patch('ganeti.utils.RunCmd')) + self.MockOut('ensure_dirs', mock.patch('ganeti.utils.EnsureDirs')) + self.MockOut('write_file', mock.patch('ganeti.utils.WriteFile')) + self.MockOut(mock.patch(kvm_class + '._InstancePidAlive', + return_value=(True, 1371, True))) + self.MockOut(mock.patch(kvm_class + '._GetVcpuThreadIds', + return_value=[1, 3, 5, 2, 4, 0 ])) + self.params = constants.HVC_DEFAULTS[constants.HT_KVM].copy() + + def testCpuPinningDefault(self): + mock_process = mock.MagicMock() + cpu_mask = self.params['cpu_mask'] + worker_cpu_mask = self.params['worker_cpu_mask'] + hypervisor = hv_kvm.KVMHypervisor() + with mock.patch('psutil.Process', return_value=mock_process): + hypervisor._ExecuteCpuAffinity('test_instance', cpu_mask, worker_cpu_mask) + + self.assertEqual(mock_process.set_cpu_affinity.call_count, 1) + self.assertEqual(mock_process.set_cpu_affinity.call_args_list[0], + mock.call(range(0,12))) + + def testCpuPinningPerVcpu(self): + mock_process = mock.MagicMock() + mock_process.set_cpu_affinity = mock.MagicMock() + mock_process.set_cpu_affinity().return_value = True + mock_process.get_children.return_value = [] + mock_process.reset_mock() + + cpu_mask = "1:2:4:5:10:15-17" + worker_cpu_mask = self.params['worker_cpu_mask'] + hypervisor = hv_kvm.KVMHypervisor() + + # This is necessary so that it provides the same object each time instead of + # overwriting it each time. + def get_mock_process(unused_pid): + return mock_process + + with mock.patch('psutil.Process', get_mock_process): + hypervisor._ExecuteCpuAffinity('test_instance', cpu_mask, worker_cpu_mask) + self.assertEqual(mock_process.set_cpu_affinity.call_count, 7) + self.assertEqual(mock_process.set_cpu_affinity.call_args_list[0], + mock.call(range(0,12))) + self.assertEqual(mock_process.set_cpu_affinity.call_args_list[6], + mock.call([15, 16, 17])) + + def testCpuPinningEntireInstance(self): + mock_process = mock.MagicMock() + mock_process.set_cpu_affinity = mock.MagicMock() + mock_process.set_cpu_affinity().return_value = True + mock_process.get_children.return_value = [] + mock_process.reset_mock() + + cpu_mask = "4" + worker_cpu_mask = "5" + hypervisor = hv_kvm.KVMHypervisor() + + def get_mock_process(unused_pid): + return mock_process + + with mock.patch('psutil.Process', get_mock_process): + hypervisor._ExecuteCpuAffinity('test_instance', cpu_mask, worker_cpu_mask) + self.assertEqual(mock_process.set_cpu_affinity.call_count, 7) + self.assertEqual(mock_process.set_cpu_affinity.call_args_list[0], + mock.call([5])) + self.assertEqual(mock_process.set_cpu_affinity.call_args_list[1], + mock.call([4])) + if __name__ == "__main__": testutils.GanetiTestProgram() -- 2.8.0.rc3.226.g39d4020
