diff --git a/nova/tests/unit/virt/libvirt/fakelibvirt.py b/nova/tests/unit/virt/libvirt/fakelibvirt.py index d73e90c4751a..d10a44c73862 100644 --- a/nova/tests/unit/virt/libvirt/fakelibvirt.py +++ b/nova/tests/unit/virt/libvirt/fakelibvirt.py @@ -141,6 +141,7 @@ VIR_ERR_NO_SECRET = 66 VIR_ERR_AGENT_UNRESPONSIVE = 86 VIR_ERR_ARGUMENT_UNSUPPORTED = 74 VIR_ERR_OPERATION_UNSUPPORTED = 84 +VIR_ERR_DEVICE_MISSING = 99 # Readonly VIR_CONNECT_RO = 1 diff --git a/nova/tests/unit/virt/libvirt/test_guest.py b/nova/tests/unit/virt/libvirt/test_guest.py index b11a5f46a13b..d89eefc7d65b 100644 --- a/nova/tests/unit/virt/libvirt/test_guest.py +++ b/nova/tests/unit/virt/libvirt/test_guest.py @@ -307,7 +307,9 @@ class GuestTestCase(test.NoDBTestCase): self.assertIn('foo', six.text_type(ex)) @mock.patch.object(libvirt_guest.Guest, "detach_device") - def test_detach_device_with_retry_operation_failed(self, mock_detach): + def _test_detach_device_with_retry_second_detach_failure( + self, mock_detach, error_code=fakelibvirt.VIR_ERR_DEVICE_MISSING, + error_message="device not found: disk vdb not found"): # This simulates a retry of the transient/live domain detach # failing because the device is not found conf = mock.Mock(spec=vconfig.LibvirtConfigGuestDevice) @@ -318,8 +320,8 @@ class GuestTestCase(test.NoDBTestCase): fake_device = "vdb" fake_exc = fakelibvirt.make_libvirtError( fakelibvirt.libvirtError, "", - error_message="operation failed: disk vdb not found", - error_code=fakelibvirt.VIR_ERR_OPERATION_FAILED, + error_message=error_message, + error_code=error_code, error_domain=fakelibvirt.VIR_FROM_DOMAIN) mock_detach.side_effect = [None, fake_exc] retry_detach = self.guest.detach_device_with_retry( @@ -333,46 +335,52 @@ class GuestTestCase(test.NoDBTestCase): self.assertNotIn('Original exception being dropped', self.stdlog.logger.output) - @mock.patch.object(libvirt_guest.Guest, "detach_device") - def test_detach_device_with_retry_operation_internal(self, mock_detach): - # This simulates a retry of the transient/live domain detach - # failing because the device is not found - conf = mock.Mock(spec=vconfig.LibvirtConfigGuestDevice) - conf.to_xml.return_value = "" - self.domain.isPersistent.return_value = True + # TODO(lyarwood): Remove this test once MIN_LIBVIRT_VERSION is >= 4.1.0 + def test_detach_device_with_retry_second_detach_operation_failed(self): + self._test_detach_device_with_retry_second_detach_failure( + error_code=fakelibvirt.VIR_ERR_OPERATION_FAILED, + error_message="operation failed: disk vdb not found") - get_config = mock.Mock(return_value=conf) - fake_device = "vdb" - fake_exc = fakelibvirt.make_libvirtError( - fakelibvirt.libvirtError, "", - error_message="operation failed: disk vdb not found", + # TODO(lyarwood): Remove this test once MIN_LIBVIRT_VERSION is >= 4.1.0 + def test_detach_device_with_retry_second_detach_internal_error(self): + self._test_detach_device_with_retry_second_detach_failure( error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR, - 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, live=True, - inc_sleep_time=.01, max_retry_count=3) - # Some time later, we can do the wait/retry to ensure detach - self.assertRaises(exception.DeviceNotFound, retry_detach) + error_message="operation failed: disk vdb not found") - def test_detach_device_with_retry_invalid_argument(self): - # This simulates a persistent domain detach failing because - # the device is not found + def test_detach_device_with_retry_second_detach_device_missing(self): + self._test_detach_device_with_retry_second_detach_failure() + + def _test_detach_device_with_retry_first_detach_failure( + self, error_code=fakelibvirt.VIR_ERR_DEVICE_MISSING, + error_message="device not found: disk vdb not found"): + # This simulates a persistent or live domain detach failing because the + # device is not found during the first attempt to detach the device. + # We should still attempt to detach the device from the live config if + # the detach from persistent failed OR we should retry the detach from + # the live config if the first detach from live config failed. + # Note that the side effects in this test case [fake_exc, None] could + # not happen in real life if the first detach failed because the detach + # from live raised not found. In real life, the second attempt to + # detach from live would raise not found again because the device is + # not present. The purpose of this test is to verify that we try to + # detach a second time if the first detach fails, so we are OK with the + # unrealistic side effects for detach from live failing the first time. conf = mock.Mock(spec=vconfig.LibvirtConfigGuestDevice) conf.to_xml.return_value = "" self.domain.isPersistent.return_value = True get_config = mock.Mock() - # Simulate the persistent domain attach attempt followed by the live - # domain attach attempt and success + # Simulate an inactive or live detach attempt which fails (not found) + # followed by a live config detach attempt that is successful get_config.side_effect = [conf, conf, conf, None, None] fake_device = "vdb" fake_exc = fakelibvirt.make_libvirtError( fakelibvirt.libvirtError, "", - error_message="invalid argument: no target device vdb", - error_code=fakelibvirt.VIR_ERR_INVALID_ARG, + error_message=error_message, + error_code=error_code, error_domain=fakelibvirt.VIR_FROM_DOMAIN) - # Detach from persistent raises not found, detach from live succeeds + # Detach from persistent or live raises not found, detach from live + # succeeds afterward self.domain.detachDeviceFlags.side_effect = [fake_exc, None] retry_detach = self.guest.detach_device_with_retry(get_config, fake_device, live=True, inc_sleep_time=.01, max_retry_count=3) @@ -387,30 +395,26 @@ class GuestTestCase(test.NoDBTestCase): self.domain.detachDeviceFlags.assert_called_once_with( "", flags=fakelibvirt.VIR_DOMAIN_AFFECT_LIVE) - def test_detach_device_with_retry_invalid_argument_no_live(self): - # This simulates a persistent domain detach failing because - # the device is not found - conf = mock.Mock(spec=vconfig.LibvirtConfigGuestDevice) - conf.to_xml.return_value = "" - self.domain.isPersistent.return_value = True + # TODO(lyarwood): Remove this test once MIN_LIBVIRT_VERSION is >= 4.1.0 + def test_detach_device_with_retry_first_detach_operation_failed(self): + self._test_detach_device_with_retry_first_detach_failure( + error_code=fakelibvirt.VIR_ERR_OPERATION_FAILED, + error_message="operation failed: disk vdb not found") - get_config = mock.Mock() - # Simulate the persistent domain attach attempt - get_config.return_value = conf - fake_device = "vdb" - fake_exc = fakelibvirt.make_libvirtError( - fakelibvirt.libvirtError, "", - error_message="invalid argument: no target device vdb", + # TODO(lyarwood): Remove this test once MIN_LIBVIRT_VERSION is >= 4.1.0 + def test_detach_device_with_retry_first_detach_internal_error(self): + self._test_detach_device_with_retry_first_detach_failure( + error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR, + error_message="operation failed: disk vdb not found") + + # TODO(lyarwood): Remove this test once MIN_LIBVIRT_VERSION is >= 4.1.0 + def test_detach_device_with_retry_first_detach_invalid_arg(self): + self._test_detach_device_with_retry_first_detach_failure( error_code=fakelibvirt.VIR_ERR_INVALID_ARG, - error_domain=fakelibvirt.VIR_FROM_DOMAIN) - # Detach from persistent raises not found - self.domain.detachDeviceFlags.side_effect = fake_exc - self.assertRaises(exception.DeviceNotFound, - self.guest.detach_device_with_retry, get_config, - fake_device, live=False, inc_sleep_time=.01, max_retry_count=3) - # We should have tried to detach from the persistent domain - self.domain.detachDeviceFlags.assert_called_once_with( - "", flags=fakelibvirt.VIR_DOMAIN_AFFECT_CONFIG) + error_message="invalid argument: no target device vdb") + + def test_detach_device_with_retry_first_detach_device_missing(self): + self._test_detach_device_with_retry_first_detach_failure() def test_get_xml_desc(self): self.guest.get_xml_desc() diff --git a/nova/virt/libvirt/guest.py b/nova/virt/libvirt/guest.py index 976bca2717c2..c20b7fc60ec8 100644 --- a/nova/virt/libvirt/guest.py +++ b/nova/virt/libvirt/guest.py @@ -405,14 +405,22 @@ class Guest(object): except libvirt.libvirtError as ex: with excutils.save_and_reraise_exception(reraise=False) as ctx: errcode = ex.get_error_code() + # TODO(lyarwood): Remove libvirt.VIR_ERR_OPERATION_FAILED + # and libvirt.VIR_ERR_INTERNAL_ERROR once + # MIN_LIBVIRT_VERSION is >= 4.1.0 if errcode in (libvirt.VIR_ERR_OPERATION_FAILED, - libvirt.VIR_ERR_INTERNAL_ERROR): + libvirt.VIR_ERR_INTERNAL_ERROR, + libvirt.VIR_ERR_DEVICE_MISSING): + # TODO(lyarwood): Remove the following error message + # check once we only care about VIR_ERR_DEVICE_MISSING errmsg = ex.get_error_message() if 'not found' in errmsg: # This will be raised if the live domain # detach fails because the device is not found raise exception.DeviceNotFound( device=alternative_device_name) + # TODO(lyarwood): Remove libvirt.VIR_ERR_INVALID_ARG once + # MIN_LIBVIRT_VERSION is >= 4.1.0 elif errcode == libvirt.VIR_ERR_INVALID_ARG: errmsg = ex.get_error_message() if 'no target device' in errmsg: