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.

NOTE(lyarwood): Conflict caused by MIN_LIBVIRT_VERSION being 1.2.1 in
stable/ocata making I81c32bbea0f04ca876f2078ef2ae0e1975473584
unsuitable. The is_job_complete polling loop removed by that change in
master is now moved into the try block of this change ensuring we catch
any errors that might be thrown while waiting for the async pivot. The
log.exception message also requires translation in Ocata.

Conflicts:
        nova/virt/libvirt/driver.py

Change-Id: I5997000a0ba6341c4987405cdc0760c3b471bd59
Closes-bug: #1685185
(cherry picked from commit 809065485c)
(cherry picked from commit fbcf8d6733)
This commit is contained in:
Lee Yarwood 2017-04-20 19:43:32 +01:00
parent ae45c4a72a
commit 8fb411c9b2
3 changed files with 89 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
nova/exception.py
View File

@ -1802,6 +1802,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

@ -14734,6 +14734,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):

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

@ -1207,20 +1207,27 @@ 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)
if resize_to:
dev.abort_job(pivot=True)
# NOTE(alex_xu): domain.blockJobAbort isn't sync call. This
# is bug in libvirt. So we need waiting for the pivot is
# finished. libvirt bug #1119173
while not dev.is_job_complete():
time.sleep(0.5)
except Exception as exc:
LOG.exception(_LE("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))
if resize_to:
dev.resize(resize_to * units.Gi / units.Ki)
finally:
self._host.write_instance_config(xml)
@ -1252,7 +1259,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,