[stable/zed] fix: fix CPU zero-pinned issue (#1326)
This won't be able to merged to stable/zed for OpenStack/nova and it's already fixed in stable/2023.1, so let's directly put patch to stable/zed.
reference: https://review.opendev.org/c/openstack/nova/+/882531
Reviewed-by: Mohammed Naser <mnaser@vexxhost.com>
diff --git a/images/nova/Dockerfile b/images/nova/Dockerfile
index 66c19d3..cff050c 100644
--- a/images/nova/Dockerfile
+++ b/images/nova/Dockerfile
@@ -7,6 +7,8 @@
ARG NOVA_GIT_REF=b4a69447e2a176fd3821abc427019339ec700f0c
ADD --keep-git-dir=true https://opendev.org/openstack/nova.git#${NOVA_GIT_REF} /src/nova
RUN git -C /src/nova fetch --unshallow
+COPY patches/nova /patches/nova
+RUN git -C /src/nova apply --verbose /patches/nova/*
RUN --mount=type=cache,mode=0755,target=/root/.cache/pip,sharing=private <<EOF bash -xe
pip3 install \
--constraint /upper-constraints.txt \
diff --git a/images/nova/patches/nova/0000-Reproduce-asym-NUMA-mixed-CPU-policy-bug.patch b/images/nova/patches/nova/0000-Reproduce-asym-NUMA-mixed-CPU-policy-bug.patch
new file mode 100644
index 0000000..1421311
--- /dev/null
+++ b/images/nova/patches/nova/0000-Reproduce-asym-NUMA-mixed-CPU-policy-bug.patch
@@ -0,0 +1,101 @@
+From 5f26bfbc8c65d723eb9c22304f0598c587f7d789 Mon Sep 17 00:00:00 2001
+From: Balazs Gibizer <gibi@redhat.com>
+Date: Wed, 26 Oct 2022 13:23:37 +0200
+Subject: [PATCH 1/2] Reproduce asym NUMA mixed CPU policy bug
+
+Related-Bug: #1994526
+Change-Id: I52ee068377cc48ef4b4cdcb4b05fdc8d926faddf
+(cherry picked from commit 182c5be122868d4fe910309a868b044b0e71316b)
+(cherry picked from commit 2cf835be82e11505ae8609deba83d61a34ecaa8d)
+---
+ .../functional/libvirt/test_numa_servers.py | 74 +++++++++++++++++++
+ 1 file changed, 74 insertions(+)
+
+diff --git a/nova/tests/functional/libvirt/test_numa_servers.py b/nova/tests/functional/libvirt/test_numa_servers.py
+index bb428159ad..f021869722 100644
+--- a/nova/tests/functional/libvirt/test_numa_servers.py
++++ b/nova/tests/functional/libvirt/test_numa_servers.py
+@@ -346,6 +346,80 @@ class NUMAServersTest(NUMAServersTestBase):
+ # 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_mixed_policy_asymmetric_multi_numa(self):
++ """Boot an instance stretched to two NUMA nodes requesting only
++ shared CPUs in one NUMA and only dedicated in the other NUMA node.
++ """
++ # shared dedicated
++ # NUMA0 pCPU | 0 | 2 3
++ # NUMA1 pCPU | | 6 7
++ self.flags(
++ cpu_shared_set='0',
++ cpu_dedicated_set='2,3,6,7',
++ group='compute',
++ )
++ self.flags(vcpu_pin_set=None)
++
++ host_info = fakelibvirt.HostInfo(
++ cpu_nodes=2, cpu_sockets=1, cpu_cores=4, cpu_threads=1)
++ self.start_compute(host_info=host_info, hostname='compute1')
++
++ # sanity check the created host topology object; this is really just a
++ # test of the fakelibvirt module
++ host_numa = objects.NUMATopology.obj_from_db_obj(
++ objects.ComputeNode.get_by_nodename(
++ self.ctxt, 'compute1',
++ ).numa_topology
++ )
++ self.assertEqual(2, len(host_numa.cells))
++ self.assertEqual({0}, host_numa.cells[0].cpuset)
++ self.assertEqual({2, 3}, host_numa.cells[0].pcpuset)
++
++ self.assertEqual(set(), host_numa.cells[1].cpuset)
++ self.assertEqual({6, 7}, host_numa.cells[1].pcpuset)
++
++ # create a flavor with 1 shared and 2 dedicated CPUs stretched to
++ # different NUMA nodes
++ extra_spec = {
++ 'hw:cpu_policy': 'mixed',
++ 'hw:cpu_dedicated_mask': '^0',
++ 'hw:numa_nodes': '2',
++ 'hw:numa_cpus.0': '0',
++ 'hw:numa_cpus.1': '1,2',
++ 'hw:numa_mem.0': '256',
++ 'hw:numa_mem.1': '768',
++ }
++ flavor_id = self._create_flavor(
++ vcpu=3, memory_mb=1024, extra_spec=extra_spec)
++ # The only possible solution (ignoring the order of vCPU1,2):
++ # vCPU 0 => pCPU 0, NUMA0, shared
++ # vCPU 1 => pCPU 6, NUMA1, dedicated
++ # vCPU 2 => pCPU 7, NUMA1, dedicated
++ # This is bug 1994526 as the scheduling fails
++ self._run_build_test(flavor_id, end_status='ERROR')
++
++ # # After bug 1994526 is fixed, this should pass
++ # expected_usage = {
++ # 'DISK_GB': 20, 'MEMORY_MB': 1024, 'PCPU': 2, 'VCPU': 1,
++ # }
++ # server = self._run_build_test(
++ # flavor_id, expected_usage=expected_usage)
++ #
++ # # sanity check the instance topology
++ # inst = objects.Instance.get_by_uuid(self.ctxt, server['id'])
++ # self.assertEqual(2, len(inst.numa_topology.cells))
++ #
++ # self.assertEqual({0}, inst.numa_topology.cells[0].cpuset)
++ # self.assertEqual(set(), inst.numa_topology.cells[0].pcpuset)
++ # self.assertEqual(None, inst.numa_topology.cells[0].cpu_pinning)
++ #
++ # self.assertEqual(set(), inst.numa_topology.cells[1].cpuset)
++ # self.assertEqual({1, 2}, inst.numa_topology.cells[1].pcpuset)
++ # self.assertEqual(
++ # {6, 7},
++ # set(inst.numa_topology.cells[1].cpu_pinning.values())
++ # )
++
+ def test_create_server_with_dedicated_policy_old_configuration(self):
+ """Create a server using the legacy extra spec and configuration.
+
+--
+2.25.1
+
diff --git a/images/nova/patches/nova/0001-Handle-zero-pinned-CPU-in-a-cell-with-mixed-policy.patch b/images/nova/patches/nova/0001-Handle-zero-pinned-CPU-in-a-cell-with-mixed-policy.patch
new file mode 100644
index 0000000..28e6fb1
--- /dev/null
+++ b/images/nova/patches/nova/0001-Handle-zero-pinned-CPU-in-a-cell-with-mixed-policy.patch
@@ -0,0 +1,122 @@
+From 9d11b6ae82b9dc709f3b856e50587d9713a70495 Mon Sep 17 00:00:00 2001
+From: Balazs Gibizer <gibi@redhat.com>
+Date: Wed, 26 Oct 2022 13:28:47 +0200
+Subject: [PATCH] Handle zero pinned CPU in a cell with mixed policy
+
+When cpu_policy is mixed the scheduler tries to find a valid CPU pinning
+for each instance NUMA cell. However if there is an instance NUMA cell
+that does not request any pinned CPUs then such logic will calculate
+empty pinning information for that cell. Then the scheduler logic
+wrongly assumes that an empty pinning result means there was no valid
+pinning. However there is difference between a None result when no valid
+pinning found, from an empty result [] which means there was nothing to
+pin.
+
+This patch makes sure that pinning == None is differentiated from
+pinning == [].
+
+Closes-Bug: #1994526
+Change-Id: I5a35a45abfcfbbb858a94927853777f112e73e5b
+(cherry picked from commit cffe3971ce585a1ddc374a3ed067347857338831)
+(cherry picked from commit fa2ba3ab2c566337f5ad6ebd8fbcecc6aa930a40)
+---
+ .../functional/libvirt/test_numa_servers.py | 42 +++++++++----------
+ nova/virt/hardware.py | 10 +++--
+ 2 files changed, 25 insertions(+), 27 deletions(-)
+
+diff --git a/nova/tests/functional/libvirt/test_numa_servers.py b/nova/tests/functional/libvirt/test_numa_servers.py
+index f021869722..5b73e1b965 100644
+--- a/nova/tests/functional/libvirt/test_numa_servers.py
++++ b/nova/tests/functional/libvirt/test_numa_servers.py
+@@ -391,34 +391,30 @@ class NUMAServersTest(NUMAServersTestBase):
+ }
+ flavor_id = self._create_flavor(
+ vcpu=3, memory_mb=1024, extra_spec=extra_spec)
++ expected_usage = {
++ 'DISK_GB': 20, 'MEMORY_MB': 1024, 'PCPU': 2, 'VCPU': 1,
++ }
+ # The only possible solution (ignoring the order of vCPU1,2):
+ # vCPU 0 => pCPU 0, NUMA0, shared
+ # vCPU 1 => pCPU 6, NUMA1, dedicated
+ # vCPU 2 => pCPU 7, NUMA1, dedicated
+- # This is bug 1994526 as the scheduling fails
+- self._run_build_test(flavor_id, end_status='ERROR')
++ server = self._run_build_test(
++ flavor_id, expected_usage=expected_usage)
+
+- # # After bug 1994526 is fixed, this should pass
+- # expected_usage = {
+- # 'DISK_GB': 20, 'MEMORY_MB': 1024, 'PCPU': 2, 'VCPU': 1,
+- # }
+- # server = self._run_build_test(
+- # flavor_id, expected_usage=expected_usage)
+- #
+- # # sanity check the instance topology
+- # inst = objects.Instance.get_by_uuid(self.ctxt, server['id'])
+- # self.assertEqual(2, len(inst.numa_topology.cells))
+- #
+- # self.assertEqual({0}, inst.numa_topology.cells[0].cpuset)
+- # self.assertEqual(set(), inst.numa_topology.cells[0].pcpuset)
+- # self.assertEqual(None, inst.numa_topology.cells[0].cpu_pinning)
+- #
+- # self.assertEqual(set(), inst.numa_topology.cells[1].cpuset)
+- # self.assertEqual({1, 2}, inst.numa_topology.cells[1].pcpuset)
+- # self.assertEqual(
+- # {6, 7},
+- # set(inst.numa_topology.cells[1].cpu_pinning.values())
+- # )
++ # sanity check the instance topology
++ inst = objects.Instance.get_by_uuid(self.ctxt, server['id'])
++ self.assertEqual(2, len(inst.numa_topology.cells))
++
++ self.assertEqual({0}, inst.numa_topology.cells[0].cpuset)
++ self.assertEqual(set(), inst.numa_topology.cells[0].pcpuset)
++ self.assertIsNone(inst.numa_topology.cells[0].cpu_pinning)
++
++ self.assertEqual(set(), inst.numa_topology.cells[1].cpuset)
++ self.assertEqual({1, 2}, inst.numa_topology.cells[1].pcpuset)
++ self.assertEqual(
++ {6, 7},
++ set(inst.numa_topology.cells[1].cpu_pinning.values())
++ )
+
+ def test_create_server_with_dedicated_policy_old_configuration(self):
+ """Create a server using the legacy extra spec and configuration.
+diff --git a/nova/virt/hardware.py b/nova/virt/hardware.py
+index 271a719aa2..def72b97d1 100644
+--- a/nova/virt/hardware.py
++++ b/nova/virt/hardware.py
+@@ -869,7 +869,7 @@ def _pack_instance_onto_cores(host_cell, instance_cell,
+ instance_cell.pcpuset)
+ cpuset_reserved = _get_reserved(
+ sibling_sets[1], pinning, num_cpu_reserved=num_cpu_reserved)
+- if not pinning or (num_cpu_reserved and not cpuset_reserved):
++ if pinning is None or (num_cpu_reserved and not cpuset_reserved):
+ continue
+ break
+
+@@ -895,7 +895,7 @@ def _pack_instance_onto_cores(host_cell, instance_cell,
+ cpuset_reserved = _get_reserved(
+ sibling_set, pinning, num_cpu_reserved=num_cpu_reserved)
+
+- if not pinning or (num_cpu_reserved and not cpuset_reserved):
++ if pinning is None or (num_cpu_reserved and not cpuset_reserved):
+ return
+ LOG.debug('Selected cores for pinning: %s, in cell %s', pinning,
+ host_cell.id)
+@@ -2590,8 +2590,10 @@ def numa_usage_from_instance_numa(host_topology, instance_topology,
+ None, fields.CPUAllocationPolicy.SHARED,
+ ):
+ continue
+-
+- pinned_cpus = set(instance_cell.cpu_pinning.values())
++ if instance_cell.cpu_pinning:
++ pinned_cpus = set(instance_cell.cpu_pinning.values())
++ else:
++ pinned_cpus = set()
+ if instance_cell.cpuset_reserved:
+ pinned_cpus |= instance_cell.cpuset_reserved
+
+--
+2.25.1
+