openstack
/
nova
Code Issues Proposed changes

Merge "libvirt: Check if domain is persistent before detaching devices" into stable/newton

This commit is contained in:
Jenkins 2017-08-16 08:48:40 +00:00 committed by Gerrit Code Review
parent 5fcf538052 b2bafd09c3
commit 68a07f8b9b
5 changed files with 40 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -464,6 +464,9 @@ class FakeVirtDomain(object):
def isActive(self):
return True
def isPersistent(self):
return True
class CacheConcurrencyTestCase(test.NoDBTestCase):
def setUp(self):

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

@ -233,6 +233,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>"
@ -240,10 +255,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 |
@ -262,10 +277,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 |
@ -279,17 +295,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))
@ -299,6 +317,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(
@ -308,7 +328,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)
@ -319,6 +339,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(
@ -329,8 +351,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()

6
nova/tests/unit/virt/test_virt_drivers.py
View File

@ -146,15 +146,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

3
nova/virt/libvirt/driver.py
View File

@ -1285,7 +1285,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()
@ -1363,7 +1362,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:

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

@ -357,9 +357,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,
@ -370,8 +369,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
@ -416,6 +413,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)