Fix wait for detach code to handle 'disk not found error'

Currently, the code does an initial detach from the persistent and
transient domains in one call, then follows up with a call of the
retryable function, which first checks the domain xml before retrying
the transient domain detach. If the transient domain detach (which is
asynchronous) completes after we checked the domain xml, trying to
detach it again will raise a LibvirtError exception. Then, our loop
code in `_do_wait_and_retry_detach` doesn't catch it and will respond
with error. We should be handling the `disk not found error` from
libvirt and consider the job complete.

Closes-Bug: #1642689

Change-Id: I131aaf28d2f5d5d964d4045e3d7d62207079cfb0

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

@@ -270,6 +270,26 @@ class GuestTestCase(test.NoDBTestCase):
             exception.DeviceNotFound, self.guest.detach_device_with_retry,
             get_config, "/dev/vdb", persistent=True, live=True)
 
+    @mock.patch.object(libvirt_guest.Guest, "detach_device")
+    def test_detach_device_with_retry_operation_failed(self, mock_detach):
+        conf = mock.Mock(spec=vconfig.LibvirtConfigGuestDevice)
+        conf.to_xml.return_value = "</xml>"
+        get_config = mock.Mock(return_value=conf)
+        fake_device = "vdb"
+        fake_exc = fakelibvirt.make_libvirtError(
+            fakelibvirt.libvirtError,
+            msg="invalid argument: no target device vdb",
+            error_code=fakelibvirt.VIR_ERR_OPERATION_FAILED,
+            error_message="disk vdb not found",
+            error_domain=fakelibvirt.VIR_FROM_DOMAIN)
+        mock_detach.side_effect = [None, fake_exc]
+        retry_detach = self.guest.detach_device_with_retry(
+            get_config, fake_device, persistent=True, live=True,
+            inc_sleep_time=.01, max_retry_count=3)
+        # Some time later, we can do the wait/retry to ensure detach
+        self.domain.detachDeviceFlags.reset_mock()
+        self.assertRaises(exception.DeviceNotFound, retry_detach)
+
     def test_get_xml_desc(self):
         self.guest.get_xml_desc()
         self.domain.XMLDesc.assert_called_once_with(flags=0)

nova/virt/libvirt/guest.py

@@ -375,10 +375,18 @@ class Guest(object):
         def _do_wait_and_retry_detach():
             config = get_device_conf_func(device)
             if config is not None:
+                try:
                     # Device is already detached from persistent domain
                     # and only transient domain needs update
                     self.detach_device(config, persistent=False, live=live)
-                # Raise error since the device still existed on the guest
+                except libvirt.libvirtError as ex:
+                    with excutils.save_and_reraise_exception():
+                        errcode = ex.get_error_code()
+                        if errcode == libvirt.VIR_ERR_OPERATION_FAILED:
+                            errmsg = ex.get_error_message()
+                            if 'not found' in errmsg:
+                                raise exception.DeviceNotFound(device=device)
+
                 reason = _("Unable to detach from guest transient domain.")
                 raise exception.DeviceDetachFailed(device=device,
                                                    reason=reason)