Merge "libvirt: Fix regression of listDevices() return type" into unmaintained/2023.1

Author: Zuul

nova/tests/fixtures/libvirt.py

@@ -2067,6 +2067,17 @@ def device_lookup_by_name(self, dev_name):
         return self.pci_info.get_device_by_name(dev_name)
 
     def nodeDeviceLookupByName(self, name):
+        # See bug https://bugs.launchpad.net/nova/+bug/2098892
+        # We don't test this by importing the libvirt module because the
+        # libvirt module is forbidden to be imported into our test
+        # environment. It is excluded from test-requirements.txt and we
+        # also use the ImportModulePoisonFixture in nova/test.py to prevent
+        # use of modules such as libvirt.
+        if not isinstance(name, str) and name is not None:
+            raise TypeError(
+                'virNodeDeviceLookupByName() argument 2 must be str or '
+                f'None, not {type(name)}')
+
         if name.startswith('mdev'):
             return self.mdev_info.get_device_by_name(name)
 

nova/tests/functional/regressions/test_bug_2098892.py

@@ -10,7 +10,6 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-from nova import test
 from nova.tests.fixtures import libvirt as fakelibvirt
 from nova.tests.functional.libvirt import test_vgpu
 
@@ -47,73 +46,7 @@ def setUp(self):
 
         self.start_compute_with_vgpu('host1')
 
-        def fake_nodeDeviceLookupByName(self, name):
-            # See bug https://bugs.launchpad.net/nova/+bug/2098892
-            # We don't test this by importing the libvirt module because the
-            # libvirt module is forbidden to be imported into our test
-            # environment.  It is excluded from test-requirements.txt and we
-            # also use the ImportModulePoisonFixture in nova/test.py to prevent
-            # use of modules such as libvirt.
-            if not isinstance(name, str) and name is not None:
-                raise TypeError(
-                    'virNodeDeviceLookupByName() argument 2 must be str or '
-                    f'None, not {type(name)}')
-
-        # FIXME(melwitt): We need to patch this only for this test because if
-        # we add it to the LibvirtFixture right away, it will cause the
-        # following additional tests to fail:
-        #
-        # nova.tests.functional.libvirt.test_reshape.VGPUReshapeTests
-        #   test_create_servers_with_vgpu
-        #
-        # nova.tests.functional.libvirt.test_vgpu.DifferentMdevClassesTests
-        #   test_create_servers_with_different_mdev_classes
-        #   test_resize_servers_with_mlx5
-        #
-        # nova.tests.functional.libvirt.test_vgpu.VGPULimitMultipleTypesTests
-        #   test_create_servers_with_vgpu
-        #
-        # nova.tests.functional.libvirt.test_vgpu.VGPULiveMigrationTests
-        #   test_live_migrate_server
-        #   test_live_migration_fails_on_old_source
-        #   test_live_migration_fails_due_to_non_supported_mdev_types
-        #   test_live_migration_fails_on_old_destination
-        #
-        # nova.tests.functional.libvirt.
-        #                           test_vgpu.VGPULiveMigrationTestsLMFailed
-        #   test_live_migrate_server
-        #   test_live_migration_fails_on_old_source
-        #   test_live_migration_fails_due_to_non_supported_mdev_types
-        #   test_live_migration_fails_on_old_destination
-        #
-        # nova.tests.functional.libvirt.test_vgpu.VGPUMultipleTypesTests
-        #   test_create_servers_with_specific_type
-        #   test_create_servers_with_vgpu
-        #
-        # nova.tests.functional.libvirt.test_vgpu.VGPUTests
-        #   test_multiple_instance_create
-        #   test_create_servers_with_vgpu
-        #   test_create_server_with_two_vgpus_isolated
-        #   test_resize_servers_with_vgpu
-        #
-        # nova.tests.functional.libvirt.test_vgpu.VGPUTestsLibvirt7_3
-        #   test_create_servers_with_vgpu
-        #   test_create_server_with_two_vgpus_isolated
-        #   test_resize_servers_with_vgpu
-        #   test_multiple_instance_create
-        #
-        # nova.tests.functional.regressions.
-        #                               test_bug_1951656.VGPUTestsLibvirt7_7
-        #   test_create_servers_with_vgpu
-        self.stub_out(
-            'nova.tests.fixtures.libvirt.Connection.nodeDeviceLookupByName',
-            fake_nodeDeviceLookupByName)
-
     def test_update_available_resource(self):
         # We only want to verify no errors were logged by
         # update_available_resource (logging under the 'except Exception:').
-        # FIXME(melwitt): This currently will log an error and traceback
-        # because of the bug. Update this when the bug is fixed.
-        e = self.assertRaises(
-            test.TestingException, self._run_periodics, raise_on_error=True)
-        self.assertIn('TypeError', str(e))
+        self._run_periodics(raise_on_error=True)

nova/tests/unit/virt/libvirt/test_host.py

@@ -2175,24 +2175,24 @@ def test_tpool_list_all_devices(self):
 
     def test_tpool_list_pci_devices(self):
         self._add_fake_host_devices()
-        devs = self.host.list_pci_devices()
-        self.assertEqual(8, len(devs))
-        for dev in devs:
-            self.assertIsInstance(dev, tpool.Proxy)
+        dev_names = self.host.list_pci_devices()
+        self.assertEqual(8, len(dev_names))
+        for name in dev_names:
+            self.assertIsInstance(name, str)
 
     def test_tpool_list_mdev_capable_devices(self):
         self._add_fake_host_devices()
-        devs = self.host.list_mdev_capable_devices()
-        self.assertEqual(3, len(devs))
-        for dev in devs:
-            self.assertIsInstance(dev, tpool.Proxy)
+        dev_names = self.host.list_mdev_capable_devices()
+        self.assertEqual(3, len(dev_names))
+        for name in dev_names:
+            self.assertIsInstance(name, str)
 
     def test_tpool_list_mediated_devices(self):
         self._add_fake_host_devices()
-        devs = self.host.list_mediated_devices()
-        self.assertEqual(1, len(devs))
-        for dev in devs:
-            self.assertIsInstance(dev, tpool.Proxy)
+        dev_names = self.host.list_mediated_devices()
+        self.assertEqual(1, len(dev_names))
+        for name in dev_names:
+            self.assertIsInstance(name, str)
 
 
 class LoadersTestCase(test.NoDBTestCase):

nova/virt/libvirt/host.py

@@ -1565,36 +1565,34 @@ def get_vdpa_device_path(
         nodedev = self.get_vdpa_nodedev_by_address(pci_address)
         return nodedev.vdpa_capability.dev_path
 
-    def list_pci_devices(self, flags=0):
+    def list_pci_devices(self, flags: int = 0) -> ty.List[str]:
         """Lookup pci devices.
 
-        :returns: a list of virNodeDevice instance
+        :returns: a list of strings, names of the virNodeDevice instances
         """
         return self._list_devices("pci", flags=flags)
 
-    def list_mdev_capable_devices(self, flags=0):
+    def list_mdev_capable_devices(self, flags: int = 0) -> ty.List[str]:
         """Lookup devices supporting mdev capabilities.
 
-        :returns: a list of virNodeDevice instance
+        :returns: a list of strings, names of the virNodeDevice instances
         """
         return self._list_devices("mdev_types", flags=flags)
 
-    def list_mediated_devices(self, flags=0):
+    def list_mediated_devices(self, flags: int = 0) -> ty.List[str]:
         """Lookup mediated devices.
 
-        :returns: a list of strings with the name of the instance
+        :returns: a list of strings, names of the virNodeDevice instances
         """
         return self._list_devices("mdev", flags=flags)
 
-    def _list_devices(self, cap, flags=0):
+    def _list_devices(self, cap, flags: int = 0) -> ty.List[str]:
         """Lookup devices.
 
-        :returns: a list of virNodeDevice instance
+        :returns: a list of strings, names of the virNodeDevice instances
         """
         try:
-            devs = [self._wrap_libvirt_proxy(dev)
-                    for dev in self.get_connection().listDevices(cap, flags)]
-            return devs
+            return self.get_connection().listDevices(cap, flags)
         except libvirt.libvirtError as ex:
             error_code = ex.get_error_code()
             if error_code == libvirt.VIR_ERR_NO_SUPPORT: