libvirt: Check if domain is persistent before detaching devices
Previously the libvirt driver would always assume that it was only detaching devices (volumes or virtual interfaces) from a persistent domain however that is not always the case. For example when rolling back from a live migration an attempt is made to detach volumes from the transient destination domain that is being cleaned up. This attempt would fail with the previous assumption of the domain being persistent in place. This change introduces a simple call to has_persistent_configuration within detach_device_with_retry to confirm the state of the domain before attempting to detach. Closes-Bug: #1669857 Closes-Bug: #1696125 Change-Id: I95948721a0119f5f54dbe50d4455fd47d422164b
This commit is contained in:
parent
0fc64f0ab6
commit
563c0927d1
|
@ -524,6 +524,9 @@ class FakeVirtDomain(object):
|
|||
def isActive(self):
|
||||
return True
|
||||
|
||||
def isPersistent(self):
|
||||
return True
|
||||
|
||||
|
||||
class CacheConcurrencyTestCase(test.NoDBTestCase):
|
||||
def setUp(self):
|
||||
|
|
|
@ -227,6 +227,21 @@ class GuestTestCase(test.NoDBTestCase):
|
|||
"</xml>", flags=(fakelibvirt.VIR_DOMAIN_AFFECT_CONFIG |
|
||||
fakelibvirt.VIR_DOMAIN_AFFECT_LIVE))
|
||||
|
||||
def test_detach_device_with_retry_from_transient_domain(self):
|
||||
conf = mock.Mock(spec=vconfig.LibvirtConfigGuestDevice)
|
||||
conf.to_xml.return_value = "</xml>"
|
||||
get_config = mock.Mock()
|
||||
get_config.side_effect = [conf, conf, None]
|
||||
dev_path = "/dev/vdb"
|
||||
self.domain.isPersistent.return_value = False
|
||||
retry_detach = self.guest.detach_device_with_retry(
|
||||
get_config, dev_path, live=True, inc_sleep_time=.01)
|
||||
self.domain.detachDeviceFlags.assert_called_once_with(
|
||||
"</xml>", flags=fakelibvirt.VIR_DOMAIN_AFFECT_LIVE)
|
||||
self.domain.detachDeviceFlags.reset_mock()
|
||||
retry_detach()
|
||||
self.assertEqual(1, self.domain.detachDeviceFlags.call_count)
|
||||
|
||||
def test_detach_device_with_retry_detach_success(self):
|
||||
conf = mock.Mock(spec=vconfig.LibvirtConfigGuestDevice)
|
||||
conf.to_xml.return_value = "</xml>"
|
||||
|
@ -234,10 +249,10 @@ class GuestTestCase(test.NoDBTestCase):
|
|||
# Force multiple retries of detach
|
||||
get_config.side_effect = [conf, conf, conf, None]
|
||||
dev_path = "/dev/vdb"
|
||||
self.domain.isPersistent.return_value = True
|
||||
|
||||
retry_detach = self.guest.detach_device_with_retry(
|
||||
get_config, dev_path, persistent=True, live=True,
|
||||
inc_sleep_time=.01)
|
||||
get_config, dev_path, live=True, inc_sleep_time=.01)
|
||||
# Ensure we've only done the initial detach call
|
||||
self.domain.detachDeviceFlags.assert_called_once_with(
|
||||
"</xml>", flags=(fakelibvirt.VIR_DOMAIN_AFFECT_CONFIG |
|
||||
|
@ -256,10 +271,11 @@ class GuestTestCase(test.NoDBTestCase):
|
|||
conf.to_xml.return_value = "</xml>"
|
||||
# Continue to return some value for the disk config
|
||||
get_config = mock.Mock(return_value=conf)
|
||||
self.domain.isPersistent.return_value = True
|
||||
|
||||
retry_detach = self.guest.detach_device_with_retry(
|
||||
get_config, "/dev/vdb", persistent=True, live=True,
|
||||
inc_sleep_time=.01, max_retry_count=3)
|
||||
get_config, "/dev/vdb", live=True, inc_sleep_time=.01,
|
||||
max_retry_count=3)
|
||||
# Ensure we've only done the initial detach call
|
||||
self.domain.detachDeviceFlags.assert_called_once_with(
|
||||
"</xml>", flags=(fakelibvirt.VIR_DOMAIN_AFFECT_CONFIG |
|
||||
|
@ -273,17 +289,19 @@ class GuestTestCase(test.NoDBTestCase):
|
|||
|
||||
def test_detach_device_with_retry_device_not_found(self):
|
||||
get_config = mock.Mock(return_value=None)
|
||||
self.domain.isPersistent.return_value = True
|
||||
ex = self.assertRaises(
|
||||
exception.DeviceNotFound, self.guest.detach_device_with_retry,
|
||||
get_config, "/dev/vdb", persistent=True, live=True)
|
||||
get_config, "/dev/vdb", live=True)
|
||||
self.assertIn("/dev/vdb", six.text_type(ex))
|
||||
|
||||
def test_detach_device_with_retry_device_not_found_alt_name(self):
|
||||
"""Tests to make sure we use the alternative name in errors."""
|
||||
get_config = mock.Mock(return_value=None)
|
||||
self.domain.isPersistent.return_value = True
|
||||
ex = self.assertRaises(
|
||||
exception.DeviceNotFound, self.guest.detach_device_with_retry,
|
||||
get_config, mock.sentinel.device, persistent=True, live=True,
|
||||
get_config, mock.sentinel.device, live=True,
|
||||
alternative_device_name='foo')
|
||||
self.assertIn('foo', six.text_type(ex))
|
||||
|
||||
|
@ -293,6 +311,8 @@ class GuestTestCase(test.NoDBTestCase):
|
|||
# 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
|
||||
|
||||
get_config = mock.Mock(return_value=conf)
|
||||
fake_device = "vdb"
|
||||
fake_exc = fakelibvirt.make_libvirtError(
|
||||
|
@ -302,7 +322,7 @@ class GuestTestCase(test.NoDBTestCase):
|
|||
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,
|
||||
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)
|
||||
|
@ -313,6 +333,8 @@ class GuestTestCase(test.NoDBTestCase):
|
|||
# the device is not found
|
||||
conf = mock.Mock(spec=vconfig.LibvirtConfigGuestDevice)
|
||||
conf.to_xml.return_value = "</xml>"
|
||||
self.domain.isPersistent.return_value = True
|
||||
|
||||
get_config = mock.Mock(return_value=conf)
|
||||
fake_device = "vdb"
|
||||
fake_exc = fakelibvirt.make_libvirtError(
|
||||
|
@ -323,8 +345,8 @@ class GuestTestCase(test.NoDBTestCase):
|
|||
mock_detach.side_effect = fake_exc
|
||||
self.assertRaises(exception.DeviceNotFound,
|
||||
self.guest.detach_device_with_retry,
|
||||
get_config, fake_device, persistent=True, live=True,
|
||||
inc_sleep_time=.01, max_retry_count=3)
|
||||
get_config, fake_device, live=True, inc_sleep_time=.01,
|
||||
max_retry_count=3)
|
||||
|
||||
def test_get_xml_desc(self):
|
||||
self.guest.get_xml_desc()
|
||||
|
|
|
@ -136,15 +136,11 @@ class _FakeDriverBackendTestCase(object):
|
|||
pass
|
||||
|
||||
def fake_detach_device_with_retry(_self, get_device_conf_func, device,
|
||||
persistent, live,
|
||||
max_retry_count=7,
|
||||
inc_sleep_time=2,
|
||||
max_sleep_time=30):
|
||||
live, *args, **kwargs):
|
||||
# Still calling detach, but instead of returning function
|
||||
# that actually checks if device is gone from XML, just continue
|
||||
# because XML never gets updated in these tests
|
||||
_self.detach_device(get_device_conf_func(device),
|
||||
persistent=persistent,
|
||||
live=live)
|
||||
return fake_wait
|
||||
|
||||
|
|
|
@ -1365,7 +1365,6 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
# volume is still in use.
|
||||
wait_for_detach = guest.detach_device_with_retry(guest.get_disk,
|
||||
disk_dev,
|
||||
persistent=True,
|
||||
live=live)
|
||||
wait_for_detach()
|
||||
|
||||
|
@ -1467,7 +1466,7 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
# Now we are going to loop until the interface is detached or we
|
||||
# timeout.
|
||||
wait_for_detach = guest.detach_device_with_retry(
|
||||
guest.get_interface_by_cfg, cfg, persistent=True, live=live,
|
||||
guest.get_interface_by_cfg, cfg, live=live,
|
||||
alternative_device_name=vif.get('address'))
|
||||
wait_for_detach()
|
||||
except exception.DeviceNotFound:
|
||||
|
|
|
@ -371,9 +371,8 @@ class Guest(object):
|
|||
devs.append(dev)
|
||||
return devs
|
||||
|
||||
def detach_device_with_retry(self, get_device_conf_func, device,
|
||||
persistent, live, max_retry_count=7,
|
||||
inc_sleep_time=2,
|
||||
def detach_device_with_retry(self, get_device_conf_func, device, live,
|
||||
max_retry_count=7, inc_sleep_time=2,
|
||||
max_sleep_time=30,
|
||||
alternative_device_name=None):
|
||||
"""Detaches a device from the guest. After the initial detach request,
|
||||
|
@ -384,8 +383,6 @@ class Guest(object):
|
|||
:param get_device_conf_func: function which takes device as a parameter
|
||||
and returns the configuration for device
|
||||
:param device: device to detach
|
||||
:param persistent: bool to indicate whether the change is
|
||||
persistent or not
|
||||
:param live: bool to indicate whether it affects the guest in running
|
||||
state
|
||||
:param max_retry_count: number of times the returned function will
|
||||
|
@ -430,6 +427,8 @@ class Guest(object):
|
|||
if conf is None:
|
||||
raise exception.DeviceNotFound(device=alternative_device_name)
|
||||
|
||||
persistent = self.has_persistent_configuration()
|
||||
|
||||
LOG.debug('Attempting initial detach for device %s', device)
|
||||
_try_detach_device(conf, persistent, live)
|
||||
LOG.debug('Start retrying detach until device %s is gone.', device)
|
||||
|
|
Loading…
Reference in New Issue