openstack
/
nova
Code Issues Proposed changes

libvirt: Handle VIR_ERR_DEVICE_MISSING when detaching devices 

Introduced in libvirt v4.1.0 [1] this error code replaces the previously
raised VIR_ERR_INVALID_ARG, VIR_ERR_OPERATION_FAILED and
VIR_ERR_INVALID_ARG codes [2][3].

VIR_ERR_OPERATION_FAILED was introduced and tested as an
active/live/hot unplug config device detach error code in
I131aaf28d2f5d5d964d4045e3d7d62207079cfb0.

VIR_ERR_INTERNAL_ERROR was introduced and tested as an
active/live/hot unplug config device detach error code in
I3055cd7641de92ab188de73733ca9288a9ca730a.

VIR_ERR_INVALID_ARG was introduced and tested as an
inactive/persistent/cold unplug config device detach error code in
I09230fc47b0950aa5a3db839a070613c9c817576.

This change introduces support for the new VIR_ERR_DEVICE_MISSING error
code while also retaining coverage for these codes until
MIN_LIBVIRT_VERSION is bumped past v4.1.0.

The majority of this change is test code motion with the existing tests
being modified to run against either the active or inactive versions of
the above  error codes for the time being.

test_detach_device_with_retry_operation_internal and
test_detach_device_with_retry_invalid_argument_no_live have been removed
as they duplicate the logic within the now refactored
_test_detach_device_with_retry_second_detach_failure.

[1] https://libvirt.org/git/?p=libvirt.git;a=commit;h=bb189c8e8c93f115c13fa3bfffdf64498f3f0ce1
[2] https://libvirt.org/git/?p=libvirt.git;a=commit;h=126db34a81bc9f9f9710408f88cceaa1e34bbbd7
[3] https://libvirt.org/git/?p=libvirt.git;a=commit;h=2f54eab7c7c618811de23c60a51e910274cf30de

Closes-Bug: #1887946
Change-Id: I7eb86edc130d186a66c04b229d46347ec5c0b625
(cherry picked from commit 902f09af25)
(cherry picked from commit 93058ae1b8)
This commit is contained in:
Lee Yarwood 2020-07-17 00:45:10 +01:00
parent 7e91134c23
commit 863d6ef760
3 changed files with 66 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
nova/tests/unit/virt/libvirt/fakelibvirt.py
View File

@ -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

108
nova/tests/unit/virt/libvirt/test_guest.py
View File

@ -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 = "</xml>"
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 = "</xml>"
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(
"</xml>", 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 = "</xml>"
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(
"</xml>", 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()

10
nova/virt/libvirt/guest.py
View File

@ -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: