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
)
This commit is contained in:
parent
093d8c369c
commit
fbcf8d6733
|
@ -1790,6 +1790,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.")
|
||||
|
||||
|
|
|
@ -14829,6 +14829,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):
|
||||
|
|
|
@ -1265,20 +1265,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)
|
||||
|
@ -1310,7 +1317,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,
|
||||
|
|
Loading…
Reference in New Issue