Handle OPERATION_FAILED error during detach

While libvirt 4.1 started to use DEVICE_MISSING for most of the cases
there is still one[1] using OPERATION_FAILED when the device is missing
during detach. So nova needs to handle that properly.

Revert this when libvirt merges[1] and nova bumps the min version over
to that release.

[1] https://listman.redhat.com/archives/libvir-list/2021-June/msg00337.html

Change-Id: Ibc967be291853e23027d9b8210ece572adcc80c9
Closes-Bug: #1931716

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

@@ -23457,6 +23457,52 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
         mock_get_device_conf_func.assert_called_once_with()
         mock_guest.detach_device.assert_not_called()
 
+    @ddt.data(power_state.RUNNING, power_state.PAUSED)
+    def test__detach_with_retry_live_dev_not_found_operation_failed(
+        self, state
+    ):
+        """Tests that exception is raised if a live detach is requested,
+        but the device is not found in the live domain and libvirt signalled
+        that with an OPERATION_FAILED error message instead of the
+        DEVICE_MISSING. See bug 1931716.
+        """
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        mock_guest = mock.Mock(spec=libvirt_guest.Guest)
+        mock_guest.get_power_state.return_value = state
+        # for simplicity do a live only detach
+        mock_guest.has_persistent_configuration.return_value = False
+
+        mock_dev = mock.Mock(spec=vconfig.LibvirtConfigGuestDisk)
+        mock_dev.alias = 'virtio-disk1'
+
+        # there will only one device query when the code checks that device
+        # existing in the live domain before it attempts to detach it
+        mock_get_device_conf_func = mock.Mock(return_value=mock_dev)
+
+        # simulate that libvirt raises an error synchronously with
+        # OPERATION_FAILED error code
+        mock_guest.detach_device.side_effect = fakelibvirt.make_libvirtError(
+            fakelibvirt.libvirtError,
+            msg='error',
+            error_message='disk vdb not found',
+            error_code=fakelibvirt.VIR_ERR_OPERATION_FAILED)
+
+        self.assertRaises(
+            exception.DeviceNotFound,
+            drvr._detach_with_retry,
+            mock_guest,
+            uuids.instance_uuid,
+            mock_get_device_conf_func,
+            device_name='vdb',
+        )
+
+        mock_guest.has_persistent_configuration.assert_called_once_with()
+        mock_guest.detach_device.assert_called_once_with(
+            mock_dev, persistent=False, live=True)
+
+        # check that the internal event handling is cleaned up
+        self.assertEqual(set(), drvr._device_event_handler._waiters)
+
     @ddt.data(power_state.RUNNING, power_state.PAUSED)
     def test__detach_with_retry_async_fail(self, state):
         """Test that libvirt sends error event during detach"""

nova/virt/libvirt/driver.py

@@ -2504,7 +2504,15 @@ class LibvirtDriver(driver.ComputeDriver):
                 "instance %s. Libvirt error code: %d, error message: %s.",
                 device_name, instance_uuid, code, msg
             )
-            if code == libvirt.VIR_ERR_DEVICE_MISSING:
+            if (code == libvirt.VIR_ERR_DEVICE_MISSING or
+                # Libvirt 4.1 improved error code usage but OPERATION_FAILED
+                # still used in one case during detach:
+                # https://github.com/libvirt/libvirt/blob/55ea45acc99c549c7757efe954aacc33ad30a8ef/src/qemu/qemu_hotplug.c#L5324-L5328
+                # TODO(gibi): remove this when a future version of libvirt
+                # transform this error to VIR_ERR_DEVICE_MISSING too.
+                (code == libvirt.VIR_ERR_OPERATION_FAILED and
+                 'not found' in msg)
+            ):
                 LOG.debug(
                     'Libvirt failed to detach device %s from instance %s '
                     'synchronously (persistent=%s, live=%s) with error: %s.',