diff --git a/nova/tests/unit/virt/libvirt/cpu/test_api.py b/nova/tests/unit/virt/libvirt/cpu/test_api.py index 79664d7f8e47..5802dec5af9c 100644 --- a/nova/tests/unit/virt/libvirt/cpu/test_api.py +++ b/nova/tests/unit/virt/libvirt/cpu/test_api.py @@ -80,23 +80,23 @@ class TestAPI(test.NoDBTestCase): @mock.patch.object(core, 'set_online') def test_power_up_online(self, mock_online): self.flags(cpu_power_management=True, group='libvirt') - self.flags(cpu_dedicated_set='0-1', group='compute') + self.flags(cpu_dedicated_set='1-2', group='compute') api.power_up(self.fake_inst) - # only core #0 can be set as core #2 is not on the dedicated set + # only core #2 can be set as core #0 is not on the dedicated set # As a reminder, core(i).online calls set_online(i) - mock_online.assert_called_once_with(0) + mock_online.assert_called_once_with(2) @mock.patch.object(core, 'set_governor') def test_power_up_governor(self, mock_set_governor): self.flags(cpu_power_management=True, group='libvirt') self.flags(cpu_power_management_strategy='governor', group='libvirt') - self.flags(cpu_dedicated_set='0-1', group='compute') + self.flags(cpu_dedicated_set='1-2', group='compute') api.power_up(self.fake_inst) - # only core #0 can be set as core #2 is not on the dedicated set + # only core #2 can be set as core #1 is not on the dedicated set # As a reminder, core(i).set_high_governor calls set_governor(i) - mock_set_governor.assert_called_once_with(0, 'performance') + mock_set_governor.assert_called_once_with(2, 'performance') @mock.patch.object(core, 'set_online') def test_power_up_skipped(self, mock_online): @@ -113,23 +113,36 @@ class TestAPI(test.NoDBTestCase): @mock.patch.object(core, 'set_offline') def test_power_down_offline(self, mock_offline): self.flags(cpu_power_management=True, group='libvirt') - self.flags(cpu_dedicated_set='0-1', group='compute') + self.flags(cpu_dedicated_set='1-2', group='compute') api.power_down(self.fake_inst) - # only core #0 can be set as core #2 is not on the dedicated set + # only core #2 can be set as core #1 is not on the dedicated set # As a reminder, core(i).online calls set_online(i) - mock_offline.assert_called_once_with(0) + mock_offline.assert_called_once_with(2) @mock.patch.object(core, 'set_governor') - def test_power_down_governor(self, mock_set_governor): + def test_power_down_governor_cpu0_ignored(self, mock_set_governor): self.flags(cpu_power_management=True, group='libvirt') self.flags(cpu_power_management_strategy='governor', group='libvirt') self.flags(cpu_dedicated_set='0-1', group='compute') api.power_down(self.fake_inst) - # only core #0 can be set as core #2 is not on the dedicated set + + # Make sure that core #0 is ignored, since it is special and cannot + # be powered down. + mock_set_governor.assert_not_called() + + @mock.patch.object(core, 'set_governor') + def test_power_down_governor(self, mock_set_governor): + self.flags(cpu_power_management=True, group='libvirt') + self.flags(cpu_power_management_strategy='governor', group='libvirt') + self.flags(cpu_dedicated_set='1-2', group='compute') + + api.power_down(self.fake_inst) + + # only core #2 can be set as core #0 is not on the dedicated set # As a reminder, core(i).set_high_governor calls set_governor(i) - mock_set_governor.assert_called_once_with(0, 'powersave') + mock_set_governor.assert_called_once_with(2, 'powersave') @mock.patch.object(core, 'set_offline') def test_power_down_skipped(self, mock_offline): @@ -146,22 +159,22 @@ class TestAPI(test.NoDBTestCase): @mock.patch.object(core, 'set_offline') def test_power_down_all_dedicated_cpus_offline(self, mock_offline): self.flags(cpu_power_management=True, group='libvirt') - self.flags(cpu_dedicated_set='0-1', group='compute') + self.flags(cpu_dedicated_set='0-2', group='compute') api.power_down_all_dedicated_cpus() - # All dedicated CPUs are turned offline - mock_offline.assert_has_calls([mock.call(0), mock.call(1)]) + # All dedicated CPUs are turned offline, except CPU0 + mock_offline.assert_has_calls([mock.call(1), mock.call(2)]) @mock.patch.object(core, 'set_governor') def test_power_down_all_dedicated_cpus_governor(self, mock_set_governor): self.flags(cpu_power_management=True, group='libvirt') self.flags(cpu_power_management_strategy='governor', group='libvirt') - self.flags(cpu_dedicated_set='0-1', group='compute') + self.flags(cpu_dedicated_set='0-2', group='compute') api.power_down_all_dedicated_cpus() - # All dedicated CPUs are turned offline - mock_set_governor.assert_has_calls([mock.call(0, 'powersave'), - mock.call(1, 'powersave')]) + # All dedicated CPUs are turned offline, except CPU0 + mock_set_governor.assert_has_calls([mock.call(1, 'powersave'), + mock.call(2, 'powersave')]) @mock.patch.object(core, 'set_offline') def test_power_down_all_dedicated_cpus_skipped(self, mock_offline): @@ -192,9 +205,31 @@ class TestAPI(test.NoDBTestCase): def test_validate_all_dedicated_cpus_for_cpu_state(self, mock_get_online, mock_get_governor): self.flags(cpu_power_management=True, group='libvirt') - self.flags(cpu_dedicated_set='0-1', group='compute') + self.flags(cpu_dedicated_set='1-2', group='compute') self.flags(cpu_power_management_strategy='cpu_state', group='libvirt') mock_get_online.return_value = True mock_get_governor.side_effect = ('powersave', 'performance') self.assertRaises(exception.InvalidConfiguration, api.validate_all_dedicated_cpus) + + @mock.patch.object(core, 'get_governor') + @mock.patch.object(core, 'get_online') + @mock.patch.object(api.LOG, 'warning') + def test_validate_all_dedicated_cpus_for_cpu_state_warning( + self, mock_warning, mock_get_online, mock_get_governor): + self.flags(cpu_power_management=True, group='libvirt') + self.flags(cpu_dedicated_set='0-2', group='compute') + self.flags(cpu_power_management_strategy='cpu_state', group='libvirt') + + mock_get_online.return_value = True + mock_get_governor.return_value = 'performance' + + api.validate_all_dedicated_cpus() + + # Make sure we skipped CPU0 + mock_get_online.assert_has_calls([mock.call(1), mock.call(2)]) + + # Make sure we logged a warning about CPU0 + mock_warning.assert_called_once_with( + 'CPU0 is in cpu_dedicated_set, but it is not eligible for ' + 'state management and will be ignored') diff --git a/nova/tests/unit/virt/test_hardware.py b/nova/tests/unit/virt/test_hardware.py index ab51a3e26ca5..381213f9720d 100644 --- a/nova/tests/unit/virt/test_hardware.py +++ b/nova/tests/unit/virt/test_hardware.py @@ -90,6 +90,11 @@ class CPUSetTestCase(test.NoDBTestCase): cpuset_ids = hw.get_cpu_dedicated_set() self.assertEqual(set([0, 1, 3, 4, 5, 6]), cpuset_ids) + def test_get_cpu_dedicated_set_nozero(self): + self.flags(cpu_dedicated_set='0-5,6,^2', group='compute') + cpuset_ids = hw.get_cpu_dedicated_set_nozero() + self.assertEqual(set([1, 3, 4, 5, 6]), cpuset_ids) + def test_get_cpu_dedicated_set__unset(self): self.flags(cpu_dedicated_set=None, group='compute') cpuset_ids = hw.get_cpu_dedicated_set() diff --git a/nova/virt/hardware.py b/nova/virt/hardware.py index 2960ba98d0a8..5ca0dda5967f 100644 --- a/nova/virt/hardware.py +++ b/nova/virt/hardware.py @@ -76,6 +76,11 @@ def get_cpu_dedicated_set(): return cpu_ids +def get_cpu_dedicated_set_nozero(): + """Return cpu_dedicated_set without CPU0, if present""" + return get_cpu_dedicated_set() - {0} + + def get_cpu_shared_set(): """Parse ``[compute] cpu_shared_set`` config. diff --git a/nova/virt/libvirt/cpu/api.py b/nova/virt/libvirt/cpu/api.py index 51ab26dce259..a1f5f7a9a018 100644 --- a/nova/virt/libvirt/cpu/api.py +++ b/nova/virt/libvirt/cpu/api.py @@ -81,7 +81,7 @@ def power_up(instance: objects.Instance) -> None: if instance.numa_topology is None: return - cpu_dedicated_set = hardware.get_cpu_dedicated_set() or set() + cpu_dedicated_set = hardware.get_cpu_dedicated_set_nozero() or set() pcpus = instance.numa_topology.cpu_pinning powered_up = set() for pcpu in pcpus: @@ -101,7 +101,7 @@ def power_down(instance: objects.Instance) -> None: if instance.numa_topology is None: return - cpu_dedicated_set = hardware.get_cpu_dedicated_set() or set() + cpu_dedicated_set = hardware.get_cpu_dedicated_set_nozero() or set() pcpus = instance.numa_topology.cpu_pinning powered_down = set() for pcpu in pcpus: @@ -127,7 +127,7 @@ def power_down_all_dedicated_cpus() -> None: "power management if you only use shared CPUs.") raise exception.InvalidConfiguration(msg) - cpu_dedicated_set = hardware.get_cpu_dedicated_set() or set() + cpu_dedicated_set = hardware.get_cpu_dedicated_set_nozero() or set() for pcpu in cpu_dedicated_set: pcpu = Core(pcpu) if CONF.libvirt.cpu_power_management_strategy == 'cpu_state': @@ -144,6 +144,11 @@ def validate_all_dedicated_cpus() -> None: governors = set() cpu_states = set() for pcpu in cpu_dedicated_set: + if (pcpu == 0 and + CONF.libvirt.cpu_power_management_strategy == 'cpu_state'): + LOG.warning('CPU0 is in cpu_dedicated_set, but it is not eligible ' + 'for state management and will be ignored') + continue pcpu = Core(pcpu) # we need to collect the governors strategy and the CPU states governors.add(pcpu.governor)