From b1a0aee1abca0ed61c156dd99544adeaebaf0960 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Thu, 16 Nov 2023 18:01:29 +0100 Subject: [PATCH] Allow enabling cpu_power_management with 0 dedicated CPUs The CPU power management feature of the libvirt driver, enabled with [libvirt]cpu_power_management, only manages dedicated CPUs and does not touch share CPUs. Today nova-compute refuses to start if configured with [libvirt]cpu_power_management=true [compute]cpu_dedicated_set=None. While this is functionally not limiting it does limit the possibility to independently enable the power management and define the cpu_dedicated_set. E.g. there might be a need to enable the former in the whole cloud in a single step, while not all nodes of the cloud will have dedicated CPUs configured. This patch removes the strict config check. The implementation already handles each PCPU individually, so if there are an empty list of PCPUs then it does nothing. Closes-Bug: #2043707 Change-Id: Ib070e1042c0526f5875e34fa4f0d569590ec2514 --- .../functional/libvirt/test_power_manage.py | 7 +++---- nova/tests/unit/virt/libvirt/cpu/test_api.py | 16 +++++++++++++--- nova/virt/hardware.py | 2 +- nova/virt/libvirt/cpu/api.py | 8 -------- ...ower-management-no-pcpu-28dd7d07d0473ea2.yaml | 11 +++++++++++ 5 files changed, 28 insertions(+), 16 deletions(-) create mode 100644 releasenotes/notes/bug-2043707-power-management-no-pcpu-28dd7d07d0473ea2.yaml diff --git a/nova/tests/functional/libvirt/test_power_manage.py b/nova/tests/functional/libvirt/test_power_manage.py index 8f7ad17a21c2..f038a24dd66b 100644 --- a/nova/tests/functional/libvirt/test_power_manage.py +++ b/nova/tests/functional/libvirt/test_power_manage.py @@ -108,14 +108,13 @@ class PowerManagementTests(PowerManagementTestsBase): cpu_dedicated_set = hardware.get_cpu_dedicated_set() self._assert_cpu_set_state(cpu_dedicated_set, expected='offline') - def test_hardstop_compute_service_if_wrong_opt(self): + def test_compute_service_starts_with_power_management_and_zero_pcpu(self): self.flags(cpu_dedicated_set=None, cpu_shared_set=None, group='compute') self.flags(vcpu_pin_set=None) self.flags(cpu_power_management=True, group='libvirt') - self.assertRaises(exception.InvalidConfiguration, - self.start_compute, host_info=self.host_info, - hostname='compute2') + self.start_compute(host_info=self.host_info, hostname='compute2') + # NOTE(gibi): we test that no exception is raised by start_compute def test_create_server(self): server = self._create_server( diff --git a/nova/tests/unit/virt/libvirt/cpu/test_api.py b/nova/tests/unit/virt/libvirt/cpu/test_api.py index 5802dec5af9c..890f4566c472 100644 --- a/nova/tests/unit/virt/libvirt/cpu/test_api.py +++ b/nova/tests/unit/virt/libvirt/cpu/test_api.py @@ -182,11 +182,14 @@ class TestAPI(test.NoDBTestCase): api.power_down_all_dedicated_cpus() mock_offline.assert_not_called() - def test_power_down_all_dedicated_cpus_wrong_config(self): + @mock.patch.object(core, 'set_offline') + def test_power_down_all_dedicated_cpus_no_dedicated_cpus_configured( + self, mock_offline + ): self.flags(cpu_power_management=True, group='libvirt') self.flags(cpu_dedicated_set=None, group='compute') - self.assertRaises(exception.InvalidConfiguration, - api.power_down_all_dedicated_cpus) + api.power_down_all_dedicated_cpus() + mock_offline.assert_not_called() @mock.patch.object(core, 'get_governor') @mock.patch.object(core, 'get_online') @@ -233,3 +236,10 @@ class TestAPI(test.NoDBTestCase): mock_warning.assert_called_once_with( 'CPU0 is in cpu_dedicated_set, but it is not eligible for ' 'state management and will be ignored') + + def test_validate_all_dedicated_cpus_no_cpu(self): + self.flags(cpu_power_management=True, group='libvirt') + self.flags(cpu_dedicated_set=None, group='compute') + api.validate_all_dedicated_cpus() + # no assert we want to make sure the validation won't raise if + # no dedicated cpus are configured diff --git a/nova/virt/hardware.py b/nova/virt/hardware.py index 5ca0dda5967f..2727cffee5b1 100644 --- a/nova/virt/hardware.py +++ b/nova/virt/hardware.py @@ -78,7 +78,7 @@ def get_cpu_dedicated_set(): def get_cpu_dedicated_set_nozero(): """Return cpu_dedicated_set without CPU0, if present""" - return get_cpu_dedicated_set() - {0} + return (get_cpu_dedicated_set() or set()) - {0} def get_cpu_shared_set(): diff --git a/nova/virt/libvirt/cpu/api.py b/nova/virt/libvirt/cpu/api.py index a1f5f7a9a018..472518843998 100644 --- a/nova/virt/libvirt/cpu/api.py +++ b/nova/virt/libvirt/cpu/api.py @@ -118,14 +118,6 @@ def power_down(instance: objects.Instance) -> None: def power_down_all_dedicated_cpus() -> None: if not CONF.libvirt.cpu_power_management: return - if (CONF.libvirt.cpu_power_management and - not CONF.compute.cpu_dedicated_set - ): - msg = _("'[compute]/cpu_dedicated_set' is mandatory to be set if " - "'[libvirt]/cpu_power_management' is set." - "Please provide the CPUs that can be pinned or don't use the " - "power management if you only use shared CPUs.") - raise exception.InvalidConfiguration(msg) cpu_dedicated_set = hardware.get_cpu_dedicated_set_nozero() or set() for pcpu in cpu_dedicated_set: diff --git a/releasenotes/notes/bug-2043707-power-management-no-pcpu-28dd7d07d0473ea2.yaml b/releasenotes/notes/bug-2043707-power-management-no-pcpu-28dd7d07d0473ea2.yaml new file mode 100644 index 000000000000..54c5954d09ed --- /dev/null +++ b/releasenotes/notes/bug-2043707-power-management-no-pcpu-28dd7d07d0473ea2.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + Relaxed the config option checking of the cpu_power_management feature of + the libvirt driver. The nova-compute service will start with + [libvirt]cpu_power_management=True and an empty [compute]cpu_dedicated_set + configuration. The power management is still only applied to dedicated CPUs. + So the above configuration only allowed to ensure that cpu_power_management + can be enabled independently for configuring cpu_dedicated_set during + deployment. +