From 11751e94f1ae53ad1fcafc02cf864694adb842a0 Mon Sep 17 00:00:00 2001
From: Stephen Finucane <sfinucan@redhat.com>
Date: Wed, 3 Jul 2019 16:12:51 +0100
Subject: [PATCH] libvirt: Remove unnecessary try-catch around 'getCPUMap'

This was added in the original review [1] but seems to be totally
unnecessary: the only reason this can fail is if libvirt version is less
than 1.2.5 [2] or if the ACL checks fail [3]. The former isn't an issue
since our minimum is greater than this and the latter isn't an issue
since that feature is off by default and there's no reason it would be
turned on for a nova host, since nova is a trusted management
application that needs full access to libvirt.

Remove the exception handling and fix a bug with a related exception,
which was attempting to do delayed string interpolation which is only a
thing with the 'logging' library, not exceptions.

[1] https://review.opendev.org/#/c/173187/8/nova/virt/libvirt/driver.py@4480
[2] https://github.com/libvirt/libvirt/search?l=C&q=.nodeGetCPUMap
[3] https://github.com/libvirt/libvirt/blob/76be4f5ddac/src/vz/vz_driver.c#L1003

Change-Id: Ia7bfd3e9c8b542beb59a7a01da45a60a6027ea37
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
---
 nova/tests/unit/virt/libvirt/test_driver.py | 26 -----------------
 nova/virt/libvirt/driver.py                 | 32 ++++++---------------
 2 files changed, 9 insertions(+), 49 deletions(-)

diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index 2dd38d958b..e2ac295eb9 100644
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -7366,32 +7366,6 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         get_online_cpus.return_value = set([4, 5])
         self.assertRaises(exception.Invalid, drvr._get_vcpu_total)
 
-    @mock.patch('nova.virt.libvirt.host.Host.get_online_cpus')
-    def test_get_host_vcpus_libvirt_error(self, get_online_cpus):
-        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
-        not_supported_exc = fakelibvirt.make_libvirtError(
-            fakelibvirt.libvirtError,
-            'this function is not supported by the connection driver:'
-            ' virNodeNumOfDevices',
-            error_code=fakelibvirt.VIR_ERR_NO_SUPPORT)
-        self.flags(vcpu_pin_set="4-6")
-        get_online_cpus.side_effect = not_supported_exc
-        self.assertRaises(exception.Invalid, drvr._get_vcpu_total)
-
-    @mock.patch('nova.virt.libvirt.host.Host.get_online_cpus')
-    def test_get_host_vcpus_libvirt_error_success(self, get_online_cpus):
-        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
-        not_supported_exc = fakelibvirt.make_libvirtError(
-            fakelibvirt.libvirtError,
-            'this function is not supported by the connection driver:'
-            ' virNodeNumOfDevices',
-            error_code=fakelibvirt.VIR_ERR_NO_SUPPORT)
-        self.flags(vcpu_pin_set="1")
-        get_online_cpus.side_effect = not_supported_exc
-        expected_vcpus = 1
-        vcpus = drvr._get_vcpu_total()
-        self.assertEqual(expected_vcpus, vcpus)
-
     @mock.patch('nova.virt.libvirt.host.Host.get_cpu_count')
     def test_get_host_vcpus_after_hotplug(self, get_cpu_count):
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 76b2e2ddfd..8fbbd1e7f3 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -5844,29 +5844,15 @@ class LibvirtDriver(driver.ComputeDriver):
             return total_pcpus
 
         available_ids = hardware.get_vcpu_pin_set()
-        # We get the list of online CPUs on the host and see if the requested
-        # set falls under these. If not, we retain the old behavior.
-        online_pcpus = None
-        try:
-            online_pcpus = self._host.get_online_cpus()
-        except libvirt.libvirtError as ex:
-            error_code = ex.get_error_code()
-            err_msg = encodeutils.exception_to_unicode(ex)
-            LOG.warning(
-                "Couldn't retrieve the online CPUs due to a Libvirt "
-                "error: %(error)s with error code: %(error_code)s",
-                {'error': err_msg, 'error_code': error_code})
-        if online_pcpus:
-            if not (available_ids <= online_pcpus):
-                msg = (_("Invalid vcpu_pin_set config, one or more of the "
-                         "specified cpuset is not online. Online cpuset(s): "
-                         "%(online)s, requested cpuset(s): %(req)s"),
-                       {'online': sorted(online_pcpus),
-                        'req': sorted(available_ids)})
-                raise exception.Invalid(msg)
-        elif sorted(available_ids)[-1] >= total_pcpus:
-            raise exception.Invalid(_("Invalid vcpu_pin_set config, "
-                                      "out of hypervisor cpu range."))
+        online_pcpus = self._host.get_online_cpus()
+        if not (available_ids <= online_pcpus):
+            msg = _("Invalid 'vcpu_pin_set' config: one or more of the "
+                    "requested CPUs is not online. Online cpuset(s): "
+                    "%(online)s, requested cpuset(s): %(req)s")
+            raise exception.Invalid(msg % {
+                'online': sorted(online_pcpus),
+                'req': sorted(available_ids)})
+
         return len(available_ids)
 
     @staticmethod