openstack
/
nova
Code Issues Proposed changes

libvirt: Always disconnect_volume after rebase failures 

Previously failures when rebasing onto a new volume would leave the
volume connected to the compute host. For some volume backends such as
iSCSI the subsequent call to terminate_connection would then result in
leftover devices remaining on the host.

This change simply catches any error associated with the rebase and
ensures that disconnect_volume is called for the new volume prior to
terminate_connection finally being called.

Change-Id: I5997000a0ba6341c4987405cdc0760c3b471bd59
Closes-bug: #1685185
This commit is contained in:
Lee Yarwood 2017-04-20 19:43:32 +01:00
parent 0f78442867
commit 809065485c
3 changed files with 87 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
nova/exception.py
View File

@ -1738,6 +1738,10 @@ class VolumesNotRemoved(Invalid):
msg_fmt = _("Failed to remove volume(s): (%(reason)s)")
class VolumeRebaseFailed(NovaException):
msg_fmt = _("Volume rebase failed: %(reason)s")
class InvalidVideoMode(Invalid):
msg_fmt = _("Provided video model (%(model)s) is not supported.")

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

@ -14955,6 +14955,70 @@ class LibvirtConnTestCase(test.NoDBTestCase):
def test_swap_volume_driver_source_is_snapshot(self):
self._test_swap_volume_driver(source_type='snapshot')
@mock.patch('nova.virt.libvirt.guest.BlockDevice.rebase')
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._connect_volume')
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_config')
@mock.patch('nova.virt.libvirt.guest.Guest.get_disk')
@mock.patch('nova.virt.libvirt.host.Host.get_guest')
@mock.patch('nova.virt.libvirt.host.Host.write_instance_config')
def test_swap_volume_disconnect_new_volume_on_rebase_error(self,
write_config, get_guest, get_disk, get_volume_config,
connect_volume, disconnect_volume, rebase):
"""Assert that disconnect_volume is called for the new volume if an
error is encountered while rebasing
"""
conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
instance = objects.Instance(**self.test_instance)
guest = libvirt_guest.Guest(mock.MagicMock())
get_guest.return_value = guest
exc = fakelibvirt.make_libvirtError(fakelibvirt.libvirtError,
'internal error', error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR)
rebase.side_effect = exc
self.assertRaises(exception.VolumeRebaseFailed, conn.swap_volume,
mock.sentinel.old_connection_info,
mock.sentinel.new_connection_info,
instance, '/dev/vdb', 0)
connect_volume.assert_called_once_with(
mock.sentinel.new_connection_info,
{'dev': 'vdb', 'type': 'disk', 'bus': 'virtio'})
disconnect_volume.assert_called_once_with(
mock.sentinel.new_connection_info, 'vdb')
@mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete')
@mock.patch('nova.virt.libvirt.guest.BlockDevice.abort_job')
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._connect_volume')
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_config')
@mock.patch('nova.virt.libvirt.guest.Guest.get_disk')
@mock.patch('nova.virt.libvirt.host.Host.get_guest')
@mock.patch('nova.virt.libvirt.host.Host.write_instance_config')
def test_swap_volume_disconnect_new_volume_on_pivot_error(self,
write_config, get_guest, get_disk, get_volume_config,
connect_volume, disconnect_volume, abort_job, is_job_complete):
"""Assert that disconnect_volume is called for the new volume if an
error is encountered while pivoting to the new volume
"""
conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
instance = objects.Instance(**self.test_instance)
guest = libvirt_guest.Guest(mock.MagicMock())
get_guest.return_value = guest
exc = fakelibvirt.make_libvirtError(fakelibvirt.libvirtError,
'internal error', error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR)
is_job_complete.return_value = True
abort_job.side_effect = [None, exc]
self.assertRaises(exception.VolumeRebaseFailed, conn.swap_volume,
mock.sentinel.old_connection_info,
mock.sentinel.new_connection_info,
instance, '/dev/vdb', 0)
connect_volume.assert_called_once_with(
mock.sentinel.new_connection_info,
{'dev': 'vdb', 'type': 'disk', 'bus': 'virtio'})
disconnect_volume.assert_called_once_with(
mock.sentinel.new_connection_info, 'vdb')
@mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete')
def _test_live_snapshot(self, mock_is_job_complete,
can_quiesce=False, require_quiesce=False):

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

@ -1279,14 +1279,21 @@ class LibvirtDriver(driver.ComputeDriver):
support_uefi = self._has_uefi_support()
guest.delete_configuration(support_uefi)
# Start copy with VIR_DOMAIN_REBASE_REUSE_EXT flag to
# allow writing to existing external volume file
dev.rebase(new_path, copy=True, reuse_ext=True)
try:
# Start copy with VIR_DOMAIN_REBASE_REUSE_EXT flag to
# allow writing to existing external volume file
dev.rebase(new_path, copy=True, reuse_ext=True)
while not dev.is_job_complete():
time.sleep(0.5)
while not dev.is_job_complete():
time.sleep(0.5)
dev.abort_job(pivot=True)
except Exception as exc:
LOG.exception("Failure rebasing volume %(new_path)s on "
"%(new_path)s.", {'new_path': new_path,
'old_path': disk_path})
raise exception.VolumeRebaseFailed(reason=six.text_type(exc))
dev.abort_job(pivot=True)
if resize_to:
dev.resize(resize_to * units.Gi / units.Ki)
finally:
@ -1319,7 +1326,12 @@ class LibvirtDriver(driver.ComputeDriver):
self._disconnect_volume(new_connection_info, disk_dev)
raise NotImplementedError(_("Swap only supports host devices"))
self._swap_volume(guest, disk_dev, conf.source_path, resize_to)
try:
self._swap_volume(guest, disk_dev, conf.source_path, resize_to)
except exception.VolumeRebaseFailed:
with excutils.save_and_reraise_exception():
self._disconnect_volume(new_connection_info, disk_dev)
self._disconnect_volume(old_connection_info, disk_dev)
def _get_existing_domain_xml(self, instance, network_info,