From 8310c0ec9f5e1139b7269be316555faf296575bf Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 25 Jul 2019 11:59:23 +0100 Subject: [PATCH] libvirt: '_get_(v|p)cpu_total' to '_get_(v|p)cpu_available' We're going to need easier access to the IDs of host cores that can be used for PCPUs going forward. Rename both '_get_pcpu_total' and '_get_vcpu_total' to '_get_pcpu_available' and '_get_vcpu_available', respectively, making some minor modifications to ensure the correct value is returned.. The 'get_cpu_count' function is removed from 'nova.virt.libvirt.host' since it no longer has any callers. Change-Id: I98efdc61fd456fc7f9e1a85238c9ef9bc04a1252 Signed-off-by: Stephen Finucane --- nova/tests/unit/virt/libvirt/test_driver.py | 106 ++++++++++---------- nova/tests/unit/virt/libvirt/test_host.py | 5 - nova/virt/libvirt/driver.py | 32 +++--- nova/virt/libvirt/host.py | 12 +-- 4 files changed, 69 insertions(+), 86 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index b5598e367e04..5626d03fae50 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -8184,21 +8184,22 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_list.assert_called_with(only_guests=True, only_running=False) @mock.patch('nova.virt.libvirt.host.Host.get_online_cpus', - return_value=set([4, 5, 6])) - def test_get_pcpu_total(self, get_online_cpus): + return_value=set([0, 1, 2, 3])) + def test_get_pcpu_available(self, get_online_cpus): """Test what happens when the '[compute] cpu_dedicated_set' config option is set. """ self.flags(vcpu_pin_set=None) - self.flags(cpu_dedicated_set='4-5', cpu_shared_set=None, + self.flags(cpu_dedicated_set='2-3', cpu_shared_set=None, group='compute') drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - pcpus = drvr._get_pcpu_total() - self.assertEqual(2, pcpus) + pcpus = drvr._get_pcpu_available() + self.assertEqual(set([2, 3]), pcpus) @mock.patch('nova.virt.libvirt.host.Host.get_online_cpus', - return_value=set([4, 5, 6])) - def test_get_pcpu_total__cpu_dedicated_set_unset(self, get_online_cpus): + return_value=set([0, 1, 2, 3])) + def test_get_pcpu_available__cpu_dedicated_set_unset( + self, get_online_cpus): """Test what happens when the '[compute] cpu_dedicated_set' config option is not set. """ @@ -8206,12 +8207,13 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.flags(cpu_dedicated_set=None, cpu_shared_set=None, group='compute') drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - pcpus = drvr._get_pcpu_total() - self.assertEqual(0, pcpus) + pcpus = drvr._get_pcpu_available() + self.assertEqual(set([]), pcpus) @mock.patch('nova.virt.libvirt.host.Host.get_online_cpus', return_value=set([4, 5])) - def test_get_pcpu_total__cpu_dedicated_set_invalid(self, get_online_cpus): + def test_get_pcpu_available__cpu_dedicated_set_invalid(self, + get_online_cpus): """Test what happens when the '[compute] cpu_dedicated_set' config option is set but it's invalid. """ @@ -8219,11 +8221,11 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.flags(cpu_dedicated_set='4-6', cpu_shared_set=None, group='compute') drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - self.assertRaises(exception.Invalid, drvr._get_pcpu_total) + self.assertRaises(exception.Invalid, drvr._get_pcpu_available) - @mock.patch('nova.virt.libvirt.host.Host.get_cpu_count', - return_value=4) - def test_get_vcpu_total(self, get_cpu_count): + @mock.patch('nova.virt.libvirt.host.Host.get_online_cpus', + return_value=set([0, 1, 2, 3])) + def test_get_vcpu_available(self, get_online_cpus): """Test what happens when neither the '[compute] cpu_shared_set' or legacy 'vcpu_pin_set' config options are set. """ @@ -8231,12 +8233,12 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.flags(cpu_shared_set=None, cpu_dedicated_set=None, group='compute') drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - vcpus = drvr._get_vcpu_total() - self.assertEqual(4, vcpus) + vcpus = drvr._get_vcpu_available() + self.assertEqual(set([0, 1, 2, 3]), vcpus) @mock.patch('nova.virt.libvirt.host.Host.get_online_cpus', - return_value=set([4, 5, 6])) - def test_get_vcpu_total__with_cpu_shared_set(self, get_online_cpus): + return_value=set([4, 5, 6, 7])) + def test_get_vcpu_available__with_cpu_shared_set(self, get_online_cpus): """Test what happens when the '[compute] cpu_shared_set' config option is set. """ @@ -8244,12 +8246,12 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.flags(cpu_shared_set='4-5', cpu_dedicated_set=None, group='compute') drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - vcpus = drvr._get_vcpu_total() - self.assertEqual(2, vcpus) + vcpus = drvr._get_vcpu_available() + self.assertEqual(set([4, 5]), vcpus) @mock.patch('nova.virt.libvirt.host.Host.get_online_cpus', - return_value=set([4, 5, 6])) - def test_get_vcpu_total__with_vcpu_pin_set(self, get_online_cpus): + return_value=set([4, 5, 6, 7])) + def test_get_vcpu_available__with_vcpu_pin_set(self, get_online_cpus): """Test what happens when the legacy 'vcpu_pin_set' config option is set. """ @@ -8257,12 +8259,12 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.flags(cpu_shared_set=None, cpu_dedicated_set=None, group='compute') drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - vcpus = drvr._get_vcpu_total() - self.assertEqual(2, vcpus) + vcpus = drvr._get_vcpu_available() + self.assertEqual(set([4, 5]), vcpus) @mock.patch('nova.virt.libvirt.host.Host.get_online_cpus', - return_value=set([4, 5, 6])) - def test_get_vcpu_total__with_cpu_dedicated_set(self, get_online_cpus): + return_value=set([4, 5, 6, 7])) + def test_get_vcpu_available__with_cpu_dedicated_set(self, get_online_cpus): """Test what happens when the '[compute] cpu_dedicated_set' config option is set. """ @@ -8270,12 +8272,12 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.flags(cpu_shared_set=None, cpu_dedicated_set='4-5', group='compute') drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - vcpus = drvr._get_vcpu_total() - self.assertEqual(0, vcpus) + vcpus = drvr._get_vcpu_available() + self.assertEqual(set([]), vcpus) @mock.patch('nova.virt.libvirt.host.Host.get_online_cpus', return_value=set([4, 5])) - def test_get_vcpu_total__cpu_shared_set_invalid(self, get_online_cpus): + def test_get_vcpu_available__cpu_shared_set_invalid(self, get_online_cpus): """Test what happens when the '[compute] cpu_shared_set' config option is set but it's invalid. """ @@ -8283,11 +8285,11 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.flags(cpu_shared_set='4-6', cpu_dedicated_set=None, group='compute') drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - self.assertRaises(exception.Invalid, drvr._get_vcpu_total) + self.assertRaises(exception.Invalid, drvr._get_vcpu_available) @mock.patch('nova.virt.libvirt.host.Host.get_online_cpus', return_value=set([4, 5])) - def test_get_vcpu_total__vcpu_pin_set_invalid(self, get_online_cpus): + def test_get_vcpu_available__vcpu_pin_set_invalid(self, get_online_cpus): """Test what happens when the legacy 'vcpu_pin_set' config option is set but it's invalid. """ @@ -8295,7 +8297,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.flags(cpu_shared_set=None, cpu_dedicated_set=None, group='compute') drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - self.assertRaises(exception.Invalid, drvr._get_vcpu_total) + self.assertRaises(exception.Invalid, drvr._get_vcpu_available) @mock.patch.object(host.Host, "has_min_version", return_value=True) def test_quiesce(self, mock_has_min_version): @@ -19445,11 +19447,11 @@ class HostStateTestCase(test.NoDBTestCase): self._host.get_memory_mb_total = _get_memory_mb_total self._host.get_memory_mb_used = _get_memory_mb_used - def _get_pcpu_total(self): - return 0 + def _get_pcpu_available(self): + return set([0]) - def _get_vcpu_total(self): - return 1 + def _get_vcpu_available(self): + return set([1]) def _get_vcpu_used(self): return 0 @@ -19597,10 +19599,10 @@ class TestUpdateProviderTree(test.NoDBTestCase): new=mock.Mock(return_value={'total': disk_gb})) @mock.patch('nova.virt.libvirt.host.Host.get_memory_mb_total', new=mock.Mock(return_value=memory_mb)) - @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_pcpu_total', - new=mock.Mock(return_value=pcpus)) - @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_vcpu_total', - new=mock.Mock(return_value=vcpus)) + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_pcpu_available', + new=mock.Mock(return_value=range(pcpus))) + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_vcpu_available', + new=mock.Mock(return_value=range(vcpus))) def _test_update_provider_tree( self, mock_gpu_invs, gpu_invs=None, vpmems=None): if gpu_invs: @@ -19728,10 +19730,10 @@ class TestUpdateProviderTree(test.NoDBTestCase): new=mock.Mock(return_value={'total': disk_gb})) @mock.patch('nova.virt.libvirt.host.Host.get_memory_mb_total', new=mock.Mock(return_value=memory_mb)) - @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_pcpu_total', - new=mock.Mock(return_value=pcpus)) - @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_vcpu_total', - new=mock.Mock(return_value=vcpus)) + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_pcpu_available', + new=mock.Mock(return_value=range(pcpus))) + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_vcpu_available', + new=mock.Mock(return_value=range(vcpus))) # TODO(efried): Bug #1784020 @unittest.expectedFailure def test_update_provider_tree_for_shared_disk_gb_resource(self): @@ -19794,10 +19796,10 @@ class TestUpdateProviderTree(test.NoDBTestCase): new=mock.Mock(return_value={'total': disk_gb})) @mock.patch('nova.virt.libvirt.host.Host.get_memory_mb_total', new=mock.Mock(return_value=memory_mb)) - @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_pcpu_total', - new=mock.Mock(return_value=pcpus)) - @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_vcpu_total', - new=mock.Mock(return_value=vcpus)) + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_pcpu_available', + new=mock.Mock(return_value=range(pcpus))) + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_vcpu_available', + new=mock.Mock(return_value=range(vcpus))) def test_update_provider_tree_for_vgpu_reshape( self, mock_gpus, mock_get_devs, mock_get_mdev_info): """Tests the VGPU reshape scenario.""" @@ -19924,10 +19926,10 @@ class TestUpdateProviderTree(test.NoDBTestCase): new=mock.Mock(return_value={'total': disk_gb})) @mock.patch('nova.virt.libvirt.host.Host.get_memory_mb_total', new=mock.Mock(return_value=memory_mb)) - @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_pcpu_total', - new=mock.Mock(return_value=pcpus)) - @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_vcpu_total', - new=mock.Mock(return_value=vcpus)) + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_pcpu_available', + new=mock.Mock(return_value=range(pcpus))) + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_vcpu_available', + new=mock.Mock(return_value=range(vcpus))) def test_update_provider_tree_for_vgpu_reshape_fails(self, mock_gpus): """Tests the VGPU reshape failure scenario where VGPU allocations are not on the root compute node provider as expected. diff --git a/nova/tests/unit/virt/libvirt/test_host.py b/nova/tests/unit/virt/libvirt/test_host.py index baef122b1bac..8ec7af1c99a1 100644 --- a/nova/tests/unit/virt/libvirt/test_host.py +++ b/nova/tests/unit/virt/libvirt/test_host.py @@ -912,11 +912,6 @@ class HostTestCase(test.NoDBTestCase): mock_find_secret.return_value = None self.host.delete_secret("rbd", "rbdvol") - def test_get_cpu_count(self): - with mock.patch.object(host.Host, "get_connection") as mock_conn: - mock_conn().getInfo.return_value = ['zero', 'one', 'two'] - self.assertEqual('two', self.host.get_cpu_count()) - def test_get_memory_total(self): with mock.patch.object(host.Host, "get_connection") as mock_conn: mock_conn().getInfo.return_value = ['zero', 'one', 'two'] diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 4b6a48b9c9a3..862c362fc0a2 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -6248,13 +6248,13 @@ class LibvirtDriver(driver.ComputeDriver): guest.resume() return guest - def _get_pcpu_total(self): + def _get_pcpu_available(self): """Get number of host cores to be used for PCPUs. :returns: The number of host cores to be used for PCPUs. """ if not CONF.compute.cpu_dedicated_set: - return 0 + return set() online_cpus = self._host.get_online_cpus() dedicated_cpus = hardware.get_cpu_dedicated_set() @@ -6267,13 +6267,15 @@ class LibvirtDriver(driver.ComputeDriver): 'online': sorted(online_cpus), 'req': sorted(dedicated_cpus)}) - return len(dedicated_cpus) + return dedicated_cpus - def _get_vcpu_total(self): - """Get number of host cores to be used for VCPUs. + def _get_vcpu_available(self): + """Get host cores to be used for VCPUs. - :returns: the number of cpu core instances can be used. + :returns: A list of host CPU cores that can be used for VCPUs. """ + online_cpus = self._host.get_online_cpus() + # NOTE(stephenfin): The use of the legacy 'vcpu_pin_set' option happens # if it's defined, regardless of whether '[compute] cpu_shared_set' is # also configured. This is legacy behavior required for upgrades that @@ -6285,16 +6287,10 @@ class LibvirtDriver(driver.ComputeDriver): elif CONF.compute.cpu_shared_set: shared_cpus = hardware.get_cpu_shared_set() elif CONF.compute.cpu_dedicated_set: - return 0 + return set() else: - try: - return self._host.get_cpu_count() - except libvirt.libvirtError: - LOG.warning("Cannot get the number of host CPUs because this " - "function is not implemented for this platform.") - return 0 + return online_cpus - online_cpus = self._host.get_online_cpus() if not shared_cpus.issubset(online_cpus): msg = _("Invalid '%(config_opt)s' config: one or " "more of the configured CPUs is not online. Online " @@ -6310,7 +6306,7 @@ class LibvirtDriver(driver.ComputeDriver): 'online': sorted(online_cpus), 'req': sorted(shared_cpus)}) - return len(shared_cpus) + return shared_cpus @staticmethod def _get_local_gb_info(): @@ -7197,8 +7193,8 @@ class LibvirtDriver(driver.ComputeDriver): """ disk_gb = int(self._get_local_gb_info()['total']) memory_mb = int(self._host.get_memory_mb_total()) - vcpus = self._get_vcpu_total() - pcpus = self._get_pcpu_total() + vcpus = len(self._get_vcpu_available()) + pcpus = len(self._get_pcpu_available()) memory_enc_slots = self._get_memory_encrypted_slots() # NOTE(yikun): If the inv record does not exists, the allocation_ratio @@ -7703,7 +7699,7 @@ class LibvirtDriver(driver.ComputeDriver): # See: https://bugs.launchpad.net/nova/+bug/1215593 data["supported_instances"] = self._get_instance_capabilities() - data["vcpus"] = self._get_vcpu_total() + data["vcpus"] = len(self._get_vcpu_available()) data["memory_mb"] = self._host.get_memory_mb_total() data["local_gb"] = disk_info_dict['total'] data["vcpus_used"] = self._get_vcpu_used() diff --git a/nova/virt/libvirt/host.py b/nova/virt/libvirt/host.py index 00dbeb6da8ef..a65da0ca9ef2 100644 --- a/nova/virt/libvirt/host.py +++ b/nova/virt/libvirt/host.py @@ -647,15 +647,9 @@ class Host(object): def get_online_cpus(self): """Get the set of CPUs that are online on the host - Method is only used by NUMA code paths which check on - libvirt version >= 1.0.4. getCPUMap() was introduced in - libvirt 1.0.0. - :returns: set of online CPUs, raises libvirtError on error - """ - - (cpus, cpu_map, online) = self.get_connection().getCPUMap() + cpus, cpu_map, online = self.get_connection().getCPUMap() online_cpus = set() for cpu in range(cpus): @@ -1039,10 +1033,6 @@ class Host(object): """ return self.get_connection().getInfo() - def get_cpu_count(self): - """Returns the total numbers of cpu in the host.""" - return self._get_hardware_info()[2] - def get_memory_mb_total(self): """Get the total memory size(MB) of physical computer.