From 82528c83acec494926214e86c7bd56d63c388d81 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 5 Oct 2020 15:59:10 +0100 Subject: [PATCH] hardware: Check inventory of shared CPUs for 'mixed' policy If using the 'mixed' CPU policy then, by design, the instance is consuming both "shared" and "dedicated" host CPUs. However, we were only checking the latter. Fix this. Change-Id: Ic21c918736e7046ad32a2b8a3330496ce42950b0 Signed-off-by: Stephen Finucane Closes-Bug: #1898272 --- .../functional/libvirt/test_numa_servers.py | 7 ++---- nova/tests/unit/virt/test_hardware.py | 23 +++++++++++++++++++ nova/virt/hardware.py | 20 ++++++++++++---- 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/nova/tests/functional/libvirt/test_numa_servers.py b/nova/tests/functional/libvirt/test_numa_servers.py index ae57478cb6dd..31a68ae42272 100644 --- a/nova/tests/functional/libvirt/test_numa_servers.py +++ b/nova/tests/functional/libvirt/test_numa_servers.py @@ -323,11 +323,8 @@ class NUMAServersTest(NUMAServersTestBase): } flavor_id = self._create_flavor(vcpu=4, extra_spec=extra_spec) - # FIXME(stephenfin): This is bug #1898272. In real life, this - # configuration will result in a libvirt traceback due to attempting to - # pin the single shared instance vCPU to an empty set - # self._run_build_test(flavor_id, end_status='ERROR') - self._run_build_test(flavor_id) + # There shouldn't be any hosts available to satisfy this request + self._run_build_test(flavor_id, end_status='ERROR') def test_create_server_with_dedicated_policy_old_configuration(self): """Create a server using the legacy extra spec and configuration. diff --git a/nova/tests/unit/virt/test_hardware.py b/nova/tests/unit/virt/test_hardware.py index c9eff4d81c79..f1b47548cdc5 100644 --- a/nova/tests/unit/virt/test_hardware.py +++ b/nova/tests/unit/virt/test_hardware.py @@ -3926,6 +3926,29 @@ class CPUPinningCellTestCase(test.NoDBTestCase, _CPUPinningTestCaseBase): mock_log.warning.call_args[0][0], ) + def test_get_pinning_mixed_policy_bug_1898272(self): + """A host cell must have sufficient dedicated *and* shared CPUs when + using the mixed policy. + """ + host_pin = objects.NUMACell( + id=0, + cpuset=set(), + pcpuset=set(range(4)), + memory=4096, + memory_usage=0, + pinned_cpus=set(), + siblings=[{0, 2}, {1, 3}], + mempages=[], + ) + inst_pin = objects.InstanceNUMACell( + cpuset={0}, + pcpuset={1, 2, 3}, + memory=2048, + cpu_policy=fields.CPUAllocationPolicy.MIXED, + ) + inst_topo = hw._numa_fit_instance_cell(host_pin, inst_pin) + self.assertIsNone(inst_topo) + class CPUPinningTestCase(test.NoDBTestCase, _CPUPinningTestCaseBase): def test_host_numa_fit_instance_to_host_single_cell(self): diff --git a/nova/virt/hardware.py b/nova/virt/hardware.py index be64162959bd..1163222f653a 100644 --- a/nova/virt/hardware.py +++ b/nova/virt/hardware.py @@ -1121,7 +1121,12 @@ def _numa_fit_instance_cell( 'cpuset_reserved': cpuset_reserved }) return None - else: + + if instance_cell.cpu_policy in ( + fields.CPUAllocationPolicy.SHARED, + fields.CPUAllocationPolicy.MIXED, + None, + ): required_cpus = len(instance_cell.cpuset) if required_cpus > len(host_cell.cpuset): LOG.debug('Not enough host cell CPUs to fit instance cell; ' @@ -1135,7 +1140,7 @@ def _numa_fit_instance_cell( fields.CPUAllocationPolicy.DEDICATED, fields.CPUAllocationPolicy.MIXED, ): - LOG.debug('Pinning has been requested') + LOG.debug('Instance has requested pinned CPUs') required_cpus = len(instance_cell.pcpuset) + cpuset_reserved if required_cpus > host_cell.avail_pcpus: LOG.debug('Not enough available CPUs to schedule instance. ' @@ -1166,9 +1171,14 @@ def _numa_fit_instance_cell( LOG.debug('Failed to map instance cell CPUs to host cell CPUs') return None - elif limits: - LOG.debug('No pinning requested, considering limitations on usable cpu' - ' and memory') + if instance_cell.cpu_policy in ( + fields.CPUAllocationPolicy.SHARED, + fields.CPUAllocationPolicy.MIXED, + None, + ) and limits: + LOG.debug( + 'Instance has requested shared CPUs; considering limitations ' + 'on usable CPU and memory.') cpu_usage = host_cell.cpu_usage + len(instance_cell.cpuset) cpu_limit = len(host_cell.cpuset) * limits.cpu_allocation_ratio if cpu_usage > cpu_limit: