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

diff --git a/nova/tests/unit/virt/libvirt/test_guest.py b/nova/tests/unit/virt/libvirt/test_guest.py
index b1f4fefcd3eafd2412c64b94096fa76ed4dc9e2b..4bd694348eeab0ba7f1cc263e85127d51d2dff91 100644 (file)
--- a/nova/tests/unit/virt/libvirt/test_guest.py
+++ b/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)

diff --git a/nova/virt/libvirt/guest.py b/nova/virt/libvirt/guest.py
index 573d8d033a467d206f0434fa4ed1226e42be5bbf..bf07080274653e304e6b3c6d6a92dde13c5d7121 100644 (file)
--- a/nova/virt/libvirt/guest.py
+++ b/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:
-                # 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
+                try:
+                    # Device is already detached from persistent domain
+                    # and only transient domain needs update
+                    self.detach_device(config, persistent=False, live=live)
+                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)