From d079f37f7ccb8da9e74416cedce4f194b66ee1d0 Mon Sep 17 00:00:00 2001 From: int32bit Date: Wed, 23 Nov 2016 18:21:16 +0800 Subject: [PATCH] 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 | 20 ++++++++++++++++++++ nova/virt/libvirt/guest.py | 16 ++++++++++++---- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_guest.py b/nova/tests/unit/virt/libvirt/test_guest.py index b1f4fefcd3..4bd694348e 100644 --- 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 = "" + 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 573d8d033a..bf07080274 100644 --- 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) -- 2.39.5