diff --git a/nova/tests/unit/virt/libvirt/storage/test_rbd.py b/nova/tests/unit/virt/libvirt/storage/test_rbd.py index 7e0dcc3eaf7f..6c25c22b6eb5 100644 --- a/nova/tests/unit/virt/libvirt/storage/test_rbd.py +++ b/nova/tests/unit/virt/libvirt/storage/test_rbd.py @@ -103,7 +103,7 @@ class RbdTestCase(test.NoDBTestCase): self.assertFalse(self.driver.is_cloneable({'url': loc}, image_meta)) - @mock.patch.object(rbd_utils.RBDDriver, '_get_fsid') + @mock.patch.object(rbd_utils.RBDDriver, 'get_fsid') @mock.patch.object(rbd_utils, 'rbd') @mock.patch.object(rbd_utils, 'rados') def test_cloneable(self, mock_rados, mock_rbd, mock_get_fsid): @@ -113,7 +113,7 @@ class RbdTestCase(test.NoDBTestCase): self.assertTrue(self.driver.is_cloneable(location, image_meta)) self.assertTrue(mock_get_fsid.called) - @mock.patch.object(rbd_utils.RBDDriver, '_get_fsid') + @mock.patch.object(rbd_utils.RBDDriver, 'get_fsid') def test_uncloneable_different_fsid(self, mock_get_fsid): mock_get_fsid.return_value = 'abc' location = {'url': 'rbd://def/pool/image/snap'} @@ -122,7 +122,7 @@ class RbdTestCase(test.NoDBTestCase): self.driver.is_cloneable(location, image_meta)) self.assertTrue(mock_get_fsid.called) - @mock.patch.object(rbd_utils.RBDDriver, '_get_fsid') + @mock.patch.object(rbd_utils.RBDDriver, 'get_fsid') @mock.patch.object(rbd_utils, 'RBDVolumeProxy') @mock.patch.object(rbd_utils, 'rbd') @mock.patch.object(rbd_utils, 'rados') @@ -140,7 +140,7 @@ class RbdTestCase(test.NoDBTestCase): snapshot='snap', read_only=True) self.assertTrue(mock_get_fsid.called) - @mock.patch.object(rbd_utils.RBDDriver, '_get_fsid') + @mock.patch.object(rbd_utils.RBDDriver, 'get_fsid') def test_uncloneable_bad_format(self, mock_get_fsid): mock_get_fsid.return_value = 'abc' location = {'url': 'rbd://abc/pool/image/snap'} @@ -151,7 +151,7 @@ class RbdTestCase(test.NoDBTestCase): self.driver.is_cloneable(location, image_meta)) self.assertTrue(mock_get_fsid.called) - @mock.patch.object(rbd_utils.RBDDriver, '_get_fsid') + @mock.patch.object(rbd_utils.RBDDriver, 'get_fsid') def test_uncloneable_missing_format(self, mock_get_fsid): mock_get_fsid.return_value = 'abc' location = {'url': 'rbd://abc/pool/image/snap'} @@ -212,6 +212,33 @@ class RbdTestCase(test.NoDBTestCase): rbd.clone.assert_called_once_with(*args, **kwargs) self.assertEqual(2, client.__enter__.call_count) + @mock.patch.object(rbd_utils, 'RADOSClient') + @mock.patch.object(rbd_utils, 'rbd') + @mock.patch.object(rbd_utils, 'rados') + def test_clone_eperm(self, mock_rados, mock_rbd, mock_client): + pool = u'images' + image = u'image-name' + snap = u'snapshot-name' + location = {'url': u'rbd://fsid/%s/%s/%s' % (pool, image, snap)} + + client_stack = [] + + def mock__enter__(inst): + def _inner(): + client_stack.append(inst) + return inst + return _inner + + client = mock_client.return_value + # capture both rados client used to perform the clone + client.__enter__.side_effect = mock__enter__(client) + + setattr(mock_rbd, 'PermissionError', test.TestingException) + rbd = mock_rbd.RBD.return_value + rbd.clone.side_effect = test.TestingException + self.assertRaises(exception.Forbidden, + self.driver.clone, location, self.volume_name) + @mock.patch.object(rbd_utils, 'RBDVolumeProxy') def test_resize(self, mock_proxy): size = 1024 @@ -384,6 +411,18 @@ class RbdTestCase(test.NoDBTestCase): client.__enter__.assert_called_once_with() client.__exit__.assert_called_once_with(None, None, None) + @mock.patch.object(rbd_utils, 'rbd') + @mock.patch.object(rbd_utils, 'rados') + @mock.patch.object(rbd_utils, 'RADOSClient') + def test_destroy_volume(self, mock_client, mock_rados, mock_rbd): + rbd = mock_rbd.RBD.return_value + vol = '12345_test' + client = mock_client.return_value + self.driver.destroy_volume(vol) + rbd.remove.assert_called_once_with(client.ioctx, vol) + client.__enter__.assert_called_once_with() + client.__exit__.assert_called_once_with(None, None, None) + @mock.patch.object(rbd_utils, 'rbd') @mock.patch.object(rbd_utils, 'rados') @mock.patch.object(rbd_utils, 'RADOSClient') @@ -406,17 +445,87 @@ class RbdTestCase(test.NoDBTestCase): self.driver.create_snap(self.volume_name, self.snap_name) proxy.create_snap.assert_called_once_with(self.snap_name) + @mock.patch.object(rbd_utils, 'RBDVolumeProxy') + def test_create_protected_snap(self, mock_proxy): + proxy = mock_proxy.return_value + proxy.__enter__.return_value = proxy + proxy.is_protected_snap.return_value = False + self.driver.create_snap(self.volume_name, self.snap_name, protect=True) + proxy.create_snap.assert_called_once_with(self.snap_name) + proxy.is_protected_snap.assert_called_once_with(self.snap_name) + proxy.protect_snap.assert_called_once_with(self.snap_name) + @mock.patch.object(rbd_utils, 'RBDVolumeProxy') def test_remove_snap(self, mock_proxy): proxy = mock_proxy.return_value proxy.__enter__.return_value = proxy + proxy.list_snaps.return_value = [{'name': self.snap_name}] + proxy.is_protected_snap.return_value = False + self.driver.remove_snap(self.volume_name, self.snap_name) + proxy.remove_snap.assert_called_once_with(self.snap_name) + + @mock.patch.object(rbd_utils, 'RBDVolumeProxy') + def test_remove_snap_force(self, mock_proxy): + proxy = mock_proxy.return_value + proxy.__enter__.return_value = proxy + proxy.is_protected_snap.return_value = True + proxy.list_snaps.return_value = [{'name': self.snap_name}] + self.driver.remove_snap(self.volume_name, self.snap_name, force=True) + proxy.is_protected_snap.assert_called_once_with(self.snap_name) + proxy.unprotect_snap.assert_called_once_with(self.snap_name) + proxy.remove_snap.assert_called_once_with(self.snap_name) + + @mock.patch.object(rbd_utils, 'RBDVolumeProxy') + def test_remove_snap_does_nothing_when_no_snapshot(self, mock_proxy): + proxy = mock_proxy.return_value + proxy.__enter__.return_value = proxy + proxy.list_snaps.return_value = [{'name': 'some-other-snaphot'}] self.driver.remove_snap(self.volume_name, self.snap_name) self.assertFalse(proxy.remove_snap.called) - proxy.list_snaps.return_value = [{'name': self.snap_name}, ] + @mock.patch.object(rbd_utils, 'RBDVolumeProxy') + def test_remove_snap_does_nothing_when_protected(self, mock_proxy): + proxy = mock_proxy.return_value + proxy.__enter__.return_value = proxy + proxy.is_protected_snap.return_value = True + proxy.list_snaps.return_value = [{'name': self.snap_name}] self.driver.remove_snap(self.volume_name, self.snap_name) + self.assertFalse(proxy.remove_snap.called) + + @mock.patch.object(rbd_utils, 'RBDVolumeProxy') + def test_remove_snap_protected_ignore_errors(self, mock_proxy): + proxy = mock_proxy.return_value + proxy.__enter__.return_value = proxy + proxy.is_protected_snap.return_value = True + proxy.list_snaps.return_value = [{'name': self.snap_name}] + self.driver.remove_snap(self.volume_name, self.snap_name, + ignore_errors=True) proxy.remove_snap.assert_called_once_with(self.snap_name) + @mock.patch.object(rbd_utils, 'RBDVolumeProxy') + def test_parent_info(self, mock_proxy): + proxy = mock_proxy.return_value + proxy.__enter__.return_value = proxy + self.driver.parent_info(self.volume_name) + proxy.parent_info.assert_called_once_with() + + @mock.patch.object(rbd_utils, 'rbd') + @mock.patch.object(rbd_utils, 'RBDVolumeProxy') + def test_parent_info_throws_exception_on_error(self, mock_proxy, mock_rbd): + setattr(mock_rbd, 'ImageNotFound', test.TestingException) + proxy = mock_proxy.return_value + proxy.__enter__.return_value = proxy + proxy.parent_info.side_effect = test.TestingException + self.assertRaises(exception.ImageUnacceptable, + self.driver.parent_info, self.volume_name) + + @mock.patch.object(rbd_utils, 'RBDVolumeProxy') + def test_flatten(self, mock_proxy): + proxy = mock_proxy.return_value + proxy.__enter__.return_value = proxy + self.driver.flatten(self.volume_name) + proxy.flatten.assert_called_once_with() + @mock.patch.object(rbd_utils, 'RBDVolumeProxy') def test_rollback_to_snap(self, mock_proxy): proxy = mock_proxy.return_value diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 79145f12220c..03d5f7fff3fa 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -15981,6 +15981,84 @@ class LibvirtSnapshotTests(_BaseSnapshotTests): self._test_snapshot(disk_format='qcow2', extra_properties=extra_properties) + @mock.patch.object(rbd_utils, 'RBDDriver') + @mock.patch.object(rbd_utils, 'rbd') + def test_raw_with_rbd_clone(self, mock_rbd, mock_driver): + self.flags(images_type='rbd', group='libvirt') + rbd = mock_driver.return_value + rbd.parent_info = mock.Mock(return_value=['test-pool', '', '']) + rbd.parse_url = mock.Mock(return_value=['a', 'b', 'c', 'd']) + with mock.patch.object(fake_libvirt_utils, 'find_disk', + return_value=('rbd://some/fake/rbd/image', + 'raw')): + with mock.patch.object(fake_libvirt_utils, 'disk_type', new='rbd'): + self._test_snapshot(disk_format='raw') + rbd.clone.assert_called_with(mock.ANY, mock.ANY, dest_pool='test-pool') + rbd.flatten.assert_called_with(mock.ANY, pool='test-pool') + + @mock.patch.object(rbd_utils, 'RBDDriver') + @mock.patch.object(rbd_utils, 'rbd') + def test_raw_with_rbd_clone_graceful_fallback(self, mock_rbd, mock_driver): + self.flags(images_type='rbd', group='libvirt') + rbd = mock_driver.return_value + rbd.parent_info = mock.Mock(side_effect=exception.ImageUnacceptable( + image_id='fake_id', reason='rbd testing')) + with test.nested( + mock.patch.object(libvirt_driver.imagebackend.images, + 'convert_image', + side_effect=_fake_convert_image), + mock.patch.object(fake_libvirt_utils, 'find_disk', + return_value=('rbd://some/fake/rbd/image', + 'raw')), + mock.patch.object(fake_libvirt_utils, 'disk_type', new='rbd')): + self._test_snapshot(disk_format='raw') + self.assertFalse(rbd.clone.called) + + @mock.patch.object(rbd_utils, 'RBDDriver') + @mock.patch.object(rbd_utils, 'rbd') + def test_raw_with_rbd_clone_eperm(self, mock_rbd, mock_driver): + self.flags(images_type='rbd', group='libvirt') + rbd = mock_driver.return_value + rbd.parent_info = mock.Mock(return_value=['test-pool', '', '']) + rbd.parse_url = mock.Mock(return_value=['a', 'b', 'c', 'd']) + rbd.clone = mock.Mock(side_effect=exception.Forbidden( + image_id='fake_id', reason='rbd testing')) + with test.nested( + mock.patch.object(libvirt_driver.imagebackend.images, + 'convert_image', + side_effect=_fake_convert_image), + mock.patch.object(fake_libvirt_utils, 'find_disk', + return_value=('rbd://some/fake/rbd/image', + 'raw')), + mock.patch.object(fake_libvirt_utils, 'disk_type', new='rbd')): + self._test_snapshot(disk_format='raw') + # Ensure that the direct_snapshot attempt was cleaned up + rbd.remove_snap.assert_called_with('c', 'd', ignore_errors=False, + pool='b', force=True) + + @mock.patch.object(rbd_utils, 'RBDDriver') + @mock.patch.object(rbd_utils, 'rbd') + def test_raw_with_rbd_clone_post_process_fails(self, mock_rbd, + mock_driver): + self.flags(images_type='rbd', group='libvirt') + rbd = mock_driver.return_value + rbd.parent_info = mock.Mock(return_value=['test-pool', '', '']) + rbd.parse_url = mock.Mock(return_value=['a', 'b', 'c', 'd']) + with test.nested( + mock.patch.object(fake_libvirt_utils, 'find_disk', + return_value=('rbd://some/fake/rbd/image', + 'raw')), + mock.patch.object(fake_libvirt_utils, 'disk_type', new='rbd'), + mock.patch.object(self.image_service, 'update', + side_effect=test.TestingException)): + self.assertRaises(test.TestingException, self._test_snapshot, + disk_format='raw') + rbd.clone.assert_called_with(mock.ANY, mock.ANY, dest_pool='test-pool') + rbd.flatten.assert_called_with(mock.ANY, pool='test-pool') + # Ensure that the direct_snapshot attempt was cleaned up + rbd.remove_snap.assert_called_with('c', 'd', ignore_errors=True, + pool='b', force=True) + class LXCSnapshotTests(LibvirtSnapshotTests): """Repeat all of the Libvirt snapshot tests, but with LXC enabled""" diff --git a/nova/tests/unit/virt/libvirt/test_imagebackend.py b/nova/tests/unit/virt/libvirt/test_imagebackend.py index 0746652cb923..8abe3e7e1399 100644 --- a/nova/tests/unit/virt/libvirt/test_imagebackend.py +++ b/nova/tests/unit/virt/libvirt/test_imagebackend.py @@ -1154,6 +1154,7 @@ class EncryptedLvmTestCase(_ImageTestCase, test.NoDBTestCase): class RbdTestCase(_ImageTestCase, test.NoDBTestCase): + FSID = "FakeFsID" POOL = "FakePool" USER = "FakeUser" CONF = "FakeConf" @@ -1441,6 +1442,137 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): mock_import.assert_called_once_with(mock.sentinel.file, name) _test() + def test_get_parent_pool(self): + image = self.image_class(self.INSTANCE, self.NAME) + with mock.patch.object(rbd_utils.RBDDriver, 'parent_info') as mock_pi: + mock_pi.return_value = [self.POOL, 'fake-image', 'fake-snap'] + parent_pool = image._get_parent_pool(self.CONTEXT, 'fake-image', + self.FSID) + self.assertEqual(self.POOL, parent_pool) + + def test_get_parent_pool_no_parent_info(self): + image = self.image_class(self.INSTANCE, self.NAME) + rbd_uri = 'rbd://%s/%s/fake-image/fake-snap' % (self.FSID, self.POOL) + with test.nested(mock.patch.object(rbd_utils.RBDDriver, 'parent_info'), + mock.patch.object(imagebackend.IMAGE_API, 'get'), + ) as (mock_pi, mock_get): + mock_pi.side_effect = exception.ImageUnacceptable(image_id='test', + reason='test') + mock_get.return_value = {'locations': [{'url': rbd_uri}]} + parent_pool = image._get_parent_pool(self.CONTEXT, 'fake-image', + self.FSID) + self.assertEqual(self.POOL, parent_pool) + + def test_get_parent_pool_non_local_image(self): + image = self.image_class(self.INSTANCE, self.NAME) + rbd_uri = 'rbd://remote-cluster/remote-pool/fake-image/fake-snap' + with test.nested( + mock.patch.object(rbd_utils.RBDDriver, 'parent_info'), + mock.patch.object(imagebackend.IMAGE_API, 'get') + ) as (mock_pi, mock_get): + mock_pi.side_effect = exception.ImageUnacceptable(image_id='test', + reason='test') + mock_get.return_value = {'locations': [{'url': rbd_uri}]} + self.assertRaises(exception.ImageUnacceptable, + image._get_parent_pool, self.CONTEXT, + 'fake-image', self.FSID) + + def test_direct_snapshot(self): + image = self.image_class(self.INSTANCE, self.NAME) + test_snap = 'rbd://%s/%s/fake-image-id/snap' % (self.FSID, self.POOL) + with test.nested( + mock.patch.object(rbd_utils.RBDDriver, 'get_fsid', + return_value=self.FSID), + mock.patch.object(image, '_get_parent_pool', + return_value=self.POOL), + mock.patch.object(rbd_utils.RBDDriver, 'create_snap'), + mock.patch.object(rbd_utils.RBDDriver, 'clone'), + mock.patch.object(rbd_utils.RBDDriver, 'flatten'), + mock.patch.object(image, 'cleanup_direct_snapshot') + ) as (mock_fsid, mock_parent, mock_create_snap, mock_clone, + mock_flatten, mock_cleanup): + location = image.direct_snapshot(self.CONTEXT, 'fake-snapshot', + 'fake-format', 'fake-image-id', + 'fake-base-image') + mock_fsid.assert_called_once_with() + mock_parent.assert_called_once_with(self.CONTEXT, + 'fake-base-image', + self.FSID) + mock_create_snap.assert_has_calls([mock.call(image.rbd_name, + 'fake-snapshot', + protect=True), + mock.call('fake-image-id', + 'snap', + pool=self.POOL, + protect=True)]) + mock_clone.assert_called_once_with(mock.ANY, 'fake-image-id', + dest_pool=self.POOL) + mock_flatten.assert_called_once_with('fake-image-id', + pool=self.POOL) + mock_cleanup.assert_called_once_with(mock.ANY) + self.assertEqual(test_snap, location) + + def test_direct_snapshot_cleans_up_on_failures(self): + image = self.image_class(self.INSTANCE, self.NAME) + test_snap = 'rbd://%s/%s/%s/snap' % (self.FSID, image.pool, + image.rbd_name) + with test.nested( + mock.patch.object(rbd_utils.RBDDriver, 'get_fsid', + return_value=self.FSID), + mock.patch.object(image, '_get_parent_pool', + return_value=self.POOL), + mock.patch.object(rbd_utils.RBDDriver, 'create_snap'), + mock.patch.object(rbd_utils.RBDDriver, 'clone', + side_effect=exception.Forbidden('testing')), + mock.patch.object(rbd_utils.RBDDriver, 'flatten'), + mock.patch.object(image, 'cleanup_direct_snapshot')) as ( + mock_fsid, mock_parent, mock_create_snap, mock_clone, + mock_flatten, mock_cleanup): + self.assertRaises(exception.Forbidden, image.direct_snapshot, + self.CONTEXT, 'snap', 'fake-format', + 'fake-image-id', 'fake-base-image') + mock_create_snap.assert_called_once_with(image.rbd_name, 'snap', + protect=True) + self.assertFalse(mock_flatten.called) + mock_cleanup.assert_called_once_with(dict(url=test_snap)) + + def test_cleanup_direct_snapshot(self): + image = self.image_class(self.INSTANCE, self.NAME) + test_snap = 'rbd://%s/%s/%s/snap' % (self.FSID, image.pool, + image.rbd_name) + with test.nested( + mock.patch.object(rbd_utils.RBDDriver, 'remove_snap'), + mock.patch.object(rbd_utils.RBDDriver, 'destroy_volume') + ) as (mock_rm, mock_destroy): + # Ensure that the method does nothing when no location is provided + image.cleanup_direct_snapshot(None) + self.assertFalse(mock_rm.called) + + # Ensure that destroy_volume is not called + image.cleanup_direct_snapshot(dict(url=test_snap)) + mock_rm.assert_called_once_with(image.rbd_name, 'snap', force=True, + ignore_errors=False, + pool=image.pool) + self.assertFalse(mock_destroy.called) + + def test_cleanup_direct_snapshot_destroy_volume(self): + image = self.image_class(self.INSTANCE, self.NAME) + test_snap = 'rbd://%s/%s/%s/snap' % (self.FSID, image.pool, + image.rbd_name) + with test.nested( + mock.patch.object(rbd_utils.RBDDriver, 'remove_snap'), + mock.patch.object(rbd_utils.RBDDriver, 'destroy_volume') + ) as (mock_rm, mock_destroy): + # Ensure that destroy_volume is called + image.cleanup_direct_snapshot(dict(url=test_snap), + also_destroy_volume=True) + mock_rm.assert_called_once_with(image.rbd_name, 'snap', + force=True, + ignore_errors=False, + pool=image.pool) + mock_destroy.assert_called_once_with(image.rbd_name, + pool=image.pool) + class PloopTestCase(_ImageTestCase, test.NoDBTestCase): SIZE = 1024 diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 51e47609d753..6f3483adabe2 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -1510,48 +1510,86 @@ class LibvirtDriver(driver.ComputeDriver): instance=instance) update_task_state(task_state=task_states.IMAGE_PENDING_UPLOAD) - snapshot_directory = CONF.libvirt.snapshots_directory - fileutils.ensure_tree(snapshot_directory) - with utils.tempdir(dir=snapshot_directory) as tmpdir: - try: - out_path = os.path.join(tmpdir, snapshot_name) - if live_snapshot: - # NOTE(xqueralt): libvirt needs o+x in the temp directory - os.chmod(tmpdir, 0o701) - self._live_snapshot(context, instance, guest, disk_path, - out_path, source_format, image_format, - image_meta) - else: - snapshot_backend.snapshot_extract(out_path, image_format) - finally: - guest = None - # NOTE(dkang): because previous managedSave is not called - # for LXC, _create_domain must not be called. - if CONF.libvirt.virt_type != 'lxc' and not live_snapshot: - if state == power_state.RUNNING: - guest = self._create_domain(domain=virt_dom) - elif state == power_state.PAUSED: - guest = self._create_domain( - domain=virt_dom, pause=True) - - if guest is not None: - self._attach_pci_devices(guest, - pci_manager.get_instance_pci_devs(instance)) - self._attach_sriov_ports(context, instance, guest) - LOG.info(_LI("Snapshot extracted, beginning image upload"), - instance=instance) - - # Upload that image to the image service + try: update_task_state(task_state=task_states.IMAGE_UPLOADING, - expected_state=task_states.IMAGE_PENDING_UPLOAD) - with libvirt_utils.file_open(out_path) as image_file: - self._image_api.update(context, - image_id, - metadata, - image_file) - LOG.info(_LI("Snapshot image upload complete"), - instance=instance) + expected_state=task_states.IMAGE_PENDING_UPLOAD) + metadata['location'] = snapshot_backend.direct_snapshot( + context, snapshot_name, image_format, image_id, + instance.image_ref) + self._snapshot_domain(context, live_snapshot, virt_dom, state, + instance) + self._image_api.update(context, image_id, metadata, + purge_props=False) + except (NotImplementedError, exception.ImageUnacceptable, + exception.Forbidden) as e: + if type(e) != NotImplementedError: + LOG.warning(_LW('Performing standard snapshot because direct ' + 'snapshot failed: %(error)s'), {'error': e}) + failed_snap = metadata.pop('location', None) + if failed_snap: + failed_snap = {'url': str(failed_snap)} + snapshot_backend.cleanup_direct_snapshot(failed_snap, + also_destroy_volume=True, + ignore_errors=True) + update_task_state(task_state=task_states.IMAGE_PENDING_UPLOAD, + expected_state=task_states.IMAGE_UPLOADING) + + snapshot_directory = CONF.libvirt.snapshots_directory + fileutils.ensure_tree(snapshot_directory) + with utils.tempdir(dir=snapshot_directory) as tmpdir: + try: + out_path = os.path.join(tmpdir, snapshot_name) + if live_snapshot: + # NOTE(xqueralt): libvirt needs o+x in the tempdir + os.chmod(tmpdir, 0o701) + self._live_snapshot(context, instance, guest, + disk_path, out_path, source_format, + image_format, image_meta) + else: + snapshot_backend.snapshot_extract(out_path, + image_format) + finally: + self._snapshot_domain(context, live_snapshot, virt_dom, + state, instance) + LOG.info(_LI("Snapshot extracted, beginning image upload"), + instance=instance) + + # Upload that image to the image service + update_task_state(task_state=task_states.IMAGE_UPLOADING, + expected_state=task_states.IMAGE_PENDING_UPLOAD) + with libvirt_utils.file_open(out_path) as image_file: + self._image_api.update(context, + image_id, + metadata, + image_file) + except Exception: + with excutils.save_and_reraise_exception(): + LOG.exception(_LE("Failed to snapshot image")) + failed_snap = metadata.pop('location', None) + if failed_snap: + failed_snap = {'url': str(failed_snap)} + snapshot_backend.cleanup_direct_snapshot( + failed_snap, also_destroy_volume=True, + ignore_errors=True) + + LOG.info(_LI("Snapshot image upload complete"), instance=instance) + + def _snapshot_domain(self, context, live_snapshot, virt_dom, state, + instance): + guest = None + # NOTE(dkang): because previous managedSave is not called + # for LXC, _create_domain must not be called. + if CONF.libvirt.virt_type != 'lxc' and not live_snapshot: + if state == power_state.RUNNING: + guest = self._create_domain(domain=virt_dom) + elif state == power_state.PAUSED: + guest = self._create_domain(domain=virt_dom, pause=True) + + if guest is not None: + self._attach_pci_devices( + guest, pci_manager.get_instance_pci_devs(instance)) + self._attach_sriov_ports(context, instance, guest) def _can_set_admin_password(self, image_meta): if (CONF.libvirt.virt_type not in ('kvm', 'qemu') or diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py index 5372a2b1bc05..47336fe7d828 100644 --- a/nova/virt/libvirt/imagebackend.py +++ b/nova/virt/libvirt/imagebackend.py @@ -393,6 +393,25 @@ class Image(object): raise exception.ImageUnacceptable(image_id=image_id_or_uri, reason=reason) + def direct_snapshot(self, context, snapshot_name, image_format, image_id, + base_image_id): + """Prepare a snapshot for direct reference from glance + + :raises: exception.ImageUnacceptable if it cannot be + referenced directly in the specified image format + :returns: URL to be given to glance + """ + raise NotImplementedError(_('direct_snapshot() is not implemented')) + + def cleanup_direct_snapshot(self, location, also_destroy_volume=False, + ignore_errors=False): + """Performs any cleanup actions required after calling + direct_snapshot(), for graceful exception handling and the like. + + This should be a no-op on any backend where it is not implemented. + """ + pass + def _get_lock_name(self, base): """Get an image's name of a base file.""" return os.path.split(base)[-1] @@ -906,6 +925,96 @@ class Rbd(Image): def rollback_to_snap(self, name): return self.driver.rollback_to_snap(self.rbd_name, name) + def _get_parent_pool(self, context, base_image_id, fsid): + parent_pool = None + try: + # The easy way -- the image is an RBD clone, so use the parent + # images' storage pool + parent_pool, _im, _snap = self.driver.parent_info(self.rbd_name) + except exception.ImageUnacceptable: + # The hard way -- the image is itself a parent, so ask Glance + # where it came from + LOG.debug('No parent info for %s; asking the Image API where its ' + 'store is', base_image_id) + try: + image_meta = IMAGE_API.get(context, base_image_id, + include_locations=True) + except Exception as e: + LOG.debug('Unable to get image %(image_id)s; error: %(error)s', + {'image_id': base_image_id, 'error': e}) + image_meta = {} + + # Find the first location that is in the same RBD cluster + for location in image_meta.get('locations', []): + try: + parent_fsid, parent_pool, _im, _snap = \ + self.driver.parse_url(location['url']) + if parent_fsid == fsid: + break + else: + parent_pool = None + except exception.ImageUnacceptable: + continue + + if not parent_pool: + raise exception.ImageUnacceptable( + _('Cannot determine the parent storage pool for %s; ' + 'cannot determine where to store images') % + base_image_id) + + return parent_pool + + def direct_snapshot(self, context, snapshot_name, image_format, + image_id, base_image_id): + """Creates an RBD snapshot directly. + """ + fsid = self.driver.get_fsid() + # NOTE(nic): Nova has zero comprehension of how Glance's image store + # is configured, but we can infer what storage pool Glance is using + # by looking at the parent image. If using authx, write access should + # be enabled on that pool for the Nova user + parent_pool = self._get_parent_pool(context, base_image_id, fsid) + + # Snapshot the disk and clone it into Glance's storage pool. librbd + # requires that snapshots be set to "protected" in order to clone them + self.driver.create_snap(self.rbd_name, snapshot_name, protect=True) + location = {'url': 'rbd://%(fsid)s/%(pool)s/%(image)s/%(snap)s' % + dict(fsid=fsid, + pool=self.pool, + image=self.rbd_name, + snap=snapshot_name)} + try: + self.driver.clone(location, image_id, dest_pool=parent_pool) + # Flatten the image, which detaches it from the source snapshot + self.driver.flatten(image_id, pool=parent_pool) + finally: + # all done with the source snapshot, clean it up + self.cleanup_direct_snapshot(location) + + # Glance makes a protected snapshot called 'snap' on uploaded + # images and hands it out, so we'll do that too. The name of + # the snapshot doesn't really matter, this just uses what the + # glance-store rbd backend sets (which is not configurable). + self.driver.create_snap(image_id, 'snap', pool=parent_pool, + protect=True) + return ('rbd://%(fsid)s/%(pool)s/%(image)s/snap' % + dict(fsid=fsid, pool=parent_pool, image=image_id)) + + def cleanup_direct_snapshot(self, location, also_destroy_volume=False, + ignore_errors=False): + """Unprotects and destroys the name snapshot. + + With also_destroy_volume=True, it will also cleanup/destroy the parent + volume. This is useful for cleaning up when the target volume fails + to snapshot properly. + """ + if location: + _fsid, _pool, _im, _snap = self.driver.parse_url(location['url']) + self.driver.remove_snap(_im, _snap, pool=_pool, force=True, + ignore_errors=ignore_errors) + if also_destroy_volume: + self.driver.destroy_volume(_im, pool=_pool) + class Ploop(Image): def __init__(self, instance=None, disk_name=None, path=None): diff --git a/nova/virt/libvirt/storage/rbd_utils.py b/nova/virt/libvirt/storage/rbd_utils.py index ddb19f91907e..bf3a33486acf 100644 --- a/nova/virt/libvirt/storage/rbd_utils.py +++ b/nova/virt/libvirt/storage/rbd_utils.py @@ -177,7 +177,7 @@ class RBDDriver(object): raise exception.ImageUnacceptable(image_id=url, reason=reason) return pieces - def _get_fsid(self): + def get_fsid(self): with RADOSClient(self) as client: return client.cluster.get_fsid() @@ -189,7 +189,7 @@ class RBDDriver(object): LOG.debug('not cloneable: %s', e) return False - if self._get_fsid() != fsid: + if self.get_fsid() != fsid: reason = '%s is in a different ceph cluster' % url LOG.debug(reason) return False @@ -209,19 +209,25 @@ class RBDDriver(object): dict(loc=url, err=e)) return False - def clone(self, image_location, dest_name): + def clone(self, image_location, dest_name, dest_pool=None): _fsid, pool, image, snapshot = self.parse_url( image_location['url']) - LOG.debug('cloning %(pool)s/%(img)s@%(snap)s' % - dict(pool=pool, img=image, snap=snapshot)) + LOG.debug('cloning %(pool)s/%(img)s@%(snap)s to ' + '%(dest_pool)s/%(dest_name)s', + dict(pool=pool, img=image, snap=snapshot, + dest_pool=dest_pool, dest_name=dest_name)) with RADOSClient(self, str(pool)) as src_client: - with RADOSClient(self) as dest_client: - rbd.RBD().clone(src_client.ioctx, - image.encode('utf-8'), - snapshot.encode('utf-8'), - dest_client.ioctx, - dest_name, - features=src_client.features) + with RADOSClient(self, dest_pool) as dest_client: + try: + rbd.RBD().clone(src_client.ioctx, + image.encode('utf-8'), + snapshot.encode('utf-8'), + dest_client.ioctx, + str(dest_name), + features=src_client.features) + except rbd.PermissionError: + raise exception.Forbidden(_('no write permission on ' + 'storage pool %s') % dest_pool) def size(self, name): with RBDVolumeProxy(self, name) as vol: @@ -237,6 +243,31 @@ class RBDDriver(object): with RBDVolumeProxy(self, name) as vol: vol.resize(size) + def parent_info(self, volume, pool=None): + """Returns the pool, image and snapshot name for the parent of an + RBD volume. + + :volume: Name of RBD object + :pool: Name of pool + """ + try: + with RBDVolumeProxy(self, str(volume), pool=pool) as vol: + return vol.parent_info() + except rbd.ImageNotFound: + raise exception.ImageUnacceptable(_("no usable parent snapshot " + "for volume %s") % volume) + + def flatten(self, volume, pool=None): + """"Flattens" a snapshotted image with the parents' data, + effectively detaching it from the parent. + + :volume: Name of RBD object + :pool: Name of pool + """ + LOG.debug('flattening %(pool)s/%(vol)s', dict(pool=pool, vol=volume)) + with RBDVolumeProxy(self, str(volume), pool=pool) as vol: + tpool.execute(vol.flatten) + def exists(self, name, pool=None, snapshot=None): try: with RBDVolumeProxy(self, name, @@ -281,10 +312,12 @@ class RBDDriver(object): args += self.ceph_args() utils.execute('rbd', 'import', *args) - def cleanup_volumes(self, instance): + def _destroy_volume(self, client, volume, pool=None): + """Destroy an RBD volume, retrying as needed. + """ def _cleanup_vol(ioctx, volume, retryctx): try: - rbd.RBD().remove(client.ioctx, volume) + rbd.RBD().remove(ioctx, volume) raise loopingcall.LoopingCallDone(retvalue=False) except rbd.ImageHasSnapshots: self.remove_snap(volume, libvirt_utils.RESIZE_SNAPSHOT_NAME, @@ -297,6 +330,20 @@ class RBDDriver(object): if retryctx['retries'] <= 0: raise loopingcall.LoopingCallDone() + # NOTE(danms): We let it go for ten seconds + retryctx = {'retries': 10} + timer = loopingcall.FixedIntervalLoopingCall( + _cleanup_vol, client.ioctx, volume, retryctx) + timed_out = timer.start(interval=1).wait() + if timed_out: + # NOTE(danms): Run this again to propagate the error, but + # if it succeeds, don't raise the loopingcall exception + try: + _cleanup_vol(client.ioctx, volume, retryctx) + except loopingcall.LoopingCallDone: + pass + + def cleanup_volumes(self, instance): with RADOSClient(self, self.pool) as client: def belongs_to_instance(disk): @@ -312,18 +359,7 @@ class RBDDriver(object): volumes = rbd.RBD().list(client.ioctx) for volume in filter(belongs_to_instance, volumes): - # NOTE(danms): We let it go for ten seconds - retryctx = {'retries': 10} - timer = loopingcall.FixedIntervalLoopingCall( - _cleanup_vol, client.ioctx, volume, retryctx) - timed_out = timer.start(interval=1).wait() - if timed_out: - # NOTE(danms): Run this again to propagate the error, but - # if it succeeds, don't raise the loopingcall exception - try: - _cleanup_vol(client.ioctx, volume, retryctx) - except loopingcall.LoopingCallDone: - pass + self._destroy_volume(client, volume) def get_pool_info(self): with RADOSClient(self) as client: @@ -332,34 +368,49 @@ class RBDDriver(object): 'free': stats['kb_avail'] * units.Ki, 'used': stats['kb_used'] * units.Ki} - def create_snap(self, volume, name): - """Create a snapshot on an RBD object. + def create_snap(self, volume, name, pool=None, protect=False): + """Create a snapshot of an RBD volume. :volume: Name of RBD object :name: Name of snapshot + :pool: Name of pool + :protect: Set the snapshot to "protected" """ LOG.debug('creating snapshot(%(snap)s) on rbd image(%(img)s)', {'snap': name, 'img': volume}) - with RBDVolumeProxy(self, volume) as vol: + with RBDVolumeProxy(self, str(volume), pool=pool) as vol: tpool.execute(vol.create_snap, name) + if protect and not vol.is_protected_snap(name): + tpool.execute(vol.protect_snap, name) - def remove_snap(self, volume, name, ignore_errors=False): - """Remove a snapshot from an RBD volume. + def remove_snap(self, volume, name, ignore_errors=False, pool=None, + force=False): + """Removes a snapshot from an RBD volume. :volume: Name of RBD object :name: Name of snapshot :ignore_errors: whether or not to log warnings on failures + :pool: Name of pool + :force: Remove snapshot even if it is protected """ - with RBDVolumeProxy(self, volume) as vol: + with RBDVolumeProxy(self, str(volume), pool=pool) as vol: if name in [snap.get('name', '') for snap in vol.list_snaps()]: - LOG.debug('removing snapshot(%(snap)s) on rbd image(%(img)s)', - {'snap': name, 'img': volume}) + if vol.is_protected_snap(name): + if force: + tpool.execute(vol.unprotect_snap, name) + elif not ignore_errors: + LOG.warning(_LW('snapshot(%(name)s) on rbd ' + 'image(%(img)s) is protected, ' + 'skipping'), + {'name': name, 'img': volume}) + return + LOG.debug('removing snapshot(%(name)s) on rbd image(%(img)s)', + {'name': name, 'img': volume}) tpool.execute(vol.remove_snap, name) - else: - if not ignore_errors: - LOG.warning(_LW('no snapshot(%(snap)s) found on ' - 'image(%(img)s)'), {'snap': name, - 'img': volume}) + elif not ignore_errors: + LOG.warning(_LW('no snapshot(%(name)s) found on rbd ' + 'image(%(img)s)'), + {'name': name, 'img': volume}) def rollback_to_snap(self, volume, name): """Revert an RBD volume to its contents at a snapshot. @@ -374,3 +425,9 @@ class RBDDriver(object): tpool.execute(vol.rollback_to_snap, name) else: raise exception.SnapshotNotFound(snapshot_id=name) + + def destroy_volume(self, volume, pool=None): + """A one-shot version of cleanup_volumes() + """ + with RADOSClient(self, pool) as client: + self._destroy_volume(client, volume) diff --git a/releasenotes/notes/bp-rbd-instance-snapshots-130e860b726ddc16.yaml b/releasenotes/notes/bp-rbd-instance-snapshots-130e860b726ddc16.yaml new file mode 100644 index 000000000000..100ccfa7557c --- /dev/null +++ b/releasenotes/notes/bp-rbd-instance-snapshots-130e860b726ddc16.yaml @@ -0,0 +1,12 @@ +--- +features: + - When RBD is used for ephemeral disks and image storage, make + snapshot use Ceph directly, and update Glance with the new location. + In case of failure, it will gracefully fallback to the "generic" + snapshot method. This requires changing the typical permissions + for the Nova Ceph user (if using authx) to allow writing to + the pool where vm images are stored, and it also requires + configuring Glance to provide a v2 endpoint with direct_url + support enabled (there are security implications to doing this). + See http://docs.ceph.com/docs/master/rbd/rbd-openstack/ for more + information on configuring OpenStack with RBD.