diff --git a/nova/exception.py b/nova/exception.py index 5006175ef7d0..1d9d9b551ade 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -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.") diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index a7508a046636..ba15f5a526af 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -14737,6 +14737,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): diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 5663d5f85299..ba01199858d1 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -1208,20 +1208,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) @@ -1253,7 +1260,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,