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
(cherry picked from commit 563c0927d1
)
This commit is contained in:
parent
527834dea8
commit
55e2c851f1
|
@ -461,6 +461,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
|
||||
|
||||
|
|
|
@ -1359,7 +1359,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()
|
||||
|
||||
|
@ -1437,7 +1436,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:
|
||||
|
|
|
@ -361,9 +361,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,
|
||||
|
@ -374,8 +373,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
|
||||
|
@ -420,6 +417,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