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)
(cherry picked from commit 863d6ef760)
(cherry picked from commit 76428c1a6a)
This commit is contained in:
Lee Yarwood 2020-07-17 00:45:10 +01:00
parent b18e93a2a4
commit 74b053f47a
3 changed files with 66 additions and 53 deletions

View File

@ -130,6 +130,7 @@ VIR_ERR_NO_SECRET = 66
VIR_ERR_AGENT_UNRESPONSIVE = 86 VIR_ERR_AGENT_UNRESPONSIVE = 86
VIR_ERR_ARGUMENT_UNSUPPORTED = 74 VIR_ERR_ARGUMENT_UNSUPPORTED = 74
VIR_ERR_OPERATION_UNSUPPORTED = 84 VIR_ERR_OPERATION_UNSUPPORTED = 84
VIR_ERR_DEVICE_MISSING = 99
# Readonly # Readonly
VIR_CONNECT_RO = 1 VIR_CONNECT_RO = 1

View File

@ -305,7 +305,9 @@ class GuestTestCase(test.NoDBTestCase):
self.assertIn('foo', six.text_type(ex)) self.assertIn('foo', six.text_type(ex))
@mock.patch.object(libvirt_guest.Guest, "detach_device") @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 # This simulates a retry of the transient/live domain detach
# failing because the device is not found # failing because the device is not found
conf = mock.Mock(spec=vconfig.LibvirtConfigGuestDevice) conf = mock.Mock(spec=vconfig.LibvirtConfigGuestDevice)
@ -316,8 +318,8 @@ class GuestTestCase(test.NoDBTestCase):
fake_device = "vdb" fake_device = "vdb"
fake_exc = fakelibvirt.make_libvirtError( fake_exc = fakelibvirt.make_libvirtError(
fakelibvirt.libvirtError, "", fakelibvirt.libvirtError, "",
error_message="operation failed: disk vdb not found", error_message=error_message,
error_code=fakelibvirt.VIR_ERR_OPERATION_FAILED, error_code=error_code,
error_domain=fakelibvirt.VIR_FROM_DOMAIN) error_domain=fakelibvirt.VIR_FROM_DOMAIN)
mock_detach.side_effect = [None, fake_exc] mock_detach.side_effect = [None, fake_exc]
retry_detach = self.guest.detach_device_with_retry( retry_detach = self.guest.detach_device_with_retry(
@ -331,46 +333,52 @@ class GuestTestCase(test.NoDBTestCase):
self.assertNotIn('Original exception being dropped', self.assertNotIn('Original exception being dropped',
self.stdlog.logger.output) self.stdlog.logger.output)
@mock.patch.object(libvirt_guest.Guest, "detach_device") # TODO(lyarwood): Remove this test once MIN_LIBVIRT_VERSION is >= 4.1.0
def test_detach_device_with_retry_operation_internal(self, mock_detach): def test_detach_device_with_retry_second_detach_operation_failed(self):
# This simulates a retry of the transient/live domain detach self._test_detach_device_with_retry_second_detach_failure(
# failing because the device is not found error_code=fakelibvirt.VIR_ERR_OPERATION_FAILED,
conf = mock.Mock(spec=vconfig.LibvirtConfigGuestDevice) error_message="operation failed: disk vdb not found")
conf.to_xml.return_value = "</xml>"
self.domain.isPersistent.return_value = True
get_config = mock.Mock(return_value=conf) # TODO(lyarwood): Remove this test once MIN_LIBVIRT_VERSION is >= 4.1.0
fake_device = "vdb" def test_detach_device_with_retry_second_detach_internal_error(self):
fake_exc = fakelibvirt.make_libvirtError( self._test_detach_device_with_retry_second_detach_failure(
fakelibvirt.libvirtError, "",
error_message="operation failed: disk vdb not found",
error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR, error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR,
error_domain=fakelibvirt.VIR_FROM_DOMAIN) error_message="operation failed: disk vdb not found")
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)
def test_detach_device_with_retry_invalid_argument(self): def test_detach_device_with_retry_second_detach_device_missing(self):
# This simulates a persistent domain detach failing because self._test_detach_device_with_retry_second_detach_failure()
# the device is not found
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 = mock.Mock(spec=vconfig.LibvirtConfigGuestDevice)
conf.to_xml.return_value = "</xml>" conf.to_xml.return_value = "</xml>"
self.domain.isPersistent.return_value = True self.domain.isPersistent.return_value = True
get_config = mock.Mock() get_config = mock.Mock()
# Simulate the persistent domain attach attempt followed by the live # Simulate an inactive or live detach attempt which fails (not found)
# domain attach attempt and success # followed by a live config detach attempt that is successful
get_config.side_effect = [conf, conf, conf, None, None] get_config.side_effect = [conf, conf, conf, None, None]
fake_device = "vdb" fake_device = "vdb"
fake_exc = fakelibvirt.make_libvirtError( fake_exc = fakelibvirt.make_libvirtError(
fakelibvirt.libvirtError, "", fakelibvirt.libvirtError, "",
error_message="invalid argument: no target device vdb", error_message=error_message,
error_code=fakelibvirt.VIR_ERR_INVALID_ARG, error_code=error_code,
error_domain=fakelibvirt.VIR_FROM_DOMAIN) 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] self.domain.detachDeviceFlags.side_effect = [fake_exc, None]
retry_detach = self.guest.detach_device_with_retry(get_config, retry_detach = self.guest.detach_device_with_retry(get_config,
fake_device, live=True, inc_sleep_time=.01, max_retry_count=3) fake_device, live=True, inc_sleep_time=.01, max_retry_count=3)
@ -385,30 +393,26 @@ class GuestTestCase(test.NoDBTestCase):
self.domain.detachDeviceFlags.assert_called_once_with( self.domain.detachDeviceFlags.assert_called_once_with(
"</xml>", flags=fakelibvirt.VIR_DOMAIN_AFFECT_LIVE) "</xml>", flags=fakelibvirt.VIR_DOMAIN_AFFECT_LIVE)
def test_detach_device_with_retry_invalid_argument_no_live(self): # TODO(lyarwood): Remove this test once MIN_LIBVIRT_VERSION is >= 4.1.0
# This simulates a persistent domain detach failing because def test_detach_device_with_retry_first_detach_operation_failed(self):
# the device is not found self._test_detach_device_with_retry_first_detach_failure(
conf = mock.Mock(spec=vconfig.LibvirtConfigGuestDevice) error_code=fakelibvirt.VIR_ERR_OPERATION_FAILED,
conf.to_xml.return_value = "</xml>" error_message="operation failed: disk vdb not found")
self.domain.isPersistent.return_value = True
get_config = mock.Mock() # TODO(lyarwood): Remove this test once MIN_LIBVIRT_VERSION is >= 4.1.0
# Simulate the persistent domain attach attempt def test_detach_device_with_retry_first_detach_internal_error(self):
get_config.return_value = conf self._test_detach_device_with_retry_first_detach_failure(
fake_device = "vdb" error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR,
fake_exc = fakelibvirt.make_libvirtError( error_message="operation failed: disk vdb not found")
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_invalid_arg(self):
self._test_detach_device_with_retry_first_detach_failure(
error_code=fakelibvirt.VIR_ERR_INVALID_ARG, error_code=fakelibvirt.VIR_ERR_INVALID_ARG,
error_domain=fakelibvirt.VIR_FROM_DOMAIN) error_message="invalid argument: no target device vdb")
# Detach from persistent raises not found
self.domain.detachDeviceFlags.side_effect = fake_exc def test_detach_device_with_retry_first_detach_device_missing(self):
self.assertRaises(exception.DeviceNotFound, self._test_detach_device_with_retry_first_detach_failure()
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)
def test_get_xml_desc(self): def test_get_xml_desc(self):
self.guest.get_xml_desc() self.guest.get_xml_desc()

View File

@ -406,14 +406,22 @@ class Guest(object):
except libvirt.libvirtError as ex: except libvirt.libvirtError as ex:
with excutils.save_and_reraise_exception(reraise=False) as ctx: with excutils.save_and_reraise_exception(reraise=False) as ctx:
errcode = ex.get_error_code() 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, 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() errmsg = ex.get_error_message()
if 'not found' in errmsg: if 'not found' in errmsg:
# This will be raised if the live domain # This will be raised if the live domain
# detach fails because the device is not found # detach fails because the device is not found
raise exception.DeviceNotFound( raise exception.DeviceNotFound(
device=alternative_device_name) 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: elif errcode == libvirt.VIR_ERR_INVALID_ARG:
errmsg = ex.get_error_message() errmsg = ex.get_error_message()
if 'no target device' in errmsg: if 'no target device' in errmsg: