From 757c333c0e55df4bcaf9d442fbe8dc8009e36989 Mon Sep 17 00:00:00 2001
From: Sylvain Bauza <sbauza@redhat.com>
Date: Thu, 18 Jul 2024 19:08:49 +0200
Subject: [PATCH] cpu: Only check governor type on online cores

Kernels don't accept to access the governor strategy on an offline core, so
we need to only validate strategies for online cores.

Change-Id: I14c9b268d0b97221216bd1a9ab9e48b48d6dcc2c
Closes-Bug: #2073528
---
 nova/tests/unit/virt/libvirt/cpu/test_api.py | 18 ++++++++++++++++++
 nova/virt/libvirt/cpu/api.py                 | 12 +++++++-----
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/nova/tests/unit/virt/libvirt/cpu/test_api.py b/nova/tests/unit/virt/libvirt/cpu/test_api.py
index 408496dbd346..4d3684817e96 100644
--- a/nova/tests/unit/virt/libvirt/cpu/test_api.py
+++ b/nova/tests/unit/virt/libvirt/cpu/test_api.py
@@ -244,3 +244,21 @@ class TestAPI(test.NoDBTestCase):
         self.api.validate_all_dedicated_cpus()
         # no assert we want to make sure the validation won't raise if
         # no dedicated cpus are configured
+
+    @mock.patch.object(core, 'get_governor')
+    @mock.patch.object(core, 'get_online')
+    def test_validate_all_dedicated_cpus_for_cpu_state_with_off_cores(
+            self, mock_get_online, mock_get_governor):
+        self.flags(cpu_power_management=True, group='libvirt')
+        self.flags(cpu_dedicated_set='1-3', group='compute')
+        self.flags(cpu_power_management_strategy='cpu_state', group='libvirt')
+        # CPU1 and CPU3 are online while CPU2 is offline
+        mock_get_online.side_effect = (True, False, True)
+        mock_get_governor.return_value = 'performance'
+        self.api.validate_all_dedicated_cpus()
+
+        mock_get_online.assert_has_calls([mock.call(1), mock.call(2),
+                                          mock.call(3)])
+        # we only have two calls as CPU2 was skipped
+        mock_get_governor.assert_has_calls([mock.call(1),
+                                            mock.call(3)])
diff --git a/nova/virt/libvirt/cpu/api.py b/nova/virt/libvirt/cpu/api.py
index 1c4bd19bee28..104a3b4d277f 100644
--- a/nova/virt/libvirt/cpu/api.py
+++ b/nova/virt/libvirt/cpu/api.py
@@ -172,8 +172,7 @@ class API(object):
         if not CONF.libvirt.cpu_power_management:
             return
         cpu_dedicated_set = hardware.get_cpu_dedicated_set() or set()
-        governors = set()
-        cpu_states = set()
+        pcpus = []
         for pcpu in cpu_dedicated_set:
             if (pcpu == 0 and
                     CONF.libvirt.cpu_power_management_strategy == 'cpu_state'):
@@ -181,11 +180,13 @@ class API(object):
                             'but it is not eligible for state management '
                             'and will be ignored')
                 continue
-            pcpu = self.core(pcpu)
             # we need to collect the governors strategy and the CPU states
-            governors.add(pcpu.governor)
-            cpu_states.add(pcpu.online)
+            pcpus.append(self.core(pcpu))
         if CONF.libvirt.cpu_power_management_strategy == 'cpu_state':
+            # NOTE(sbauza): offline cores can't have a governor, it returns a
+            # DeviceBusy exception.
+            governors = set([pcpu.governor for pcpu in pcpus
+                             if pcpu.online])
             # all the cores need to have the same governor strategy
             if len(governors) > 1:
                 msg = _("All the cores need to have the same governor strategy"
@@ -193,6 +194,7 @@ class API(object):
                         "compute node if you prefer.")
                 raise exception.InvalidConfiguration(msg)
         elif CONF.libvirt.cpu_power_management_strategy == 'governor':
+            cpu_states = set([pcpu.online for pcpu in pcpus])
             # all the cores need to be online
             if False in cpu_states:
                 msg = _("All the cores need to be online before modifying the "