diff --git a/cinder/image/image_utils.py b/cinder/image/image_utils.py index fbf2e1192..c51179ba3 100644 --- a/cinder/image/image_utils.py +++ b/cinder/image/image_utils.py @@ -409,6 +409,20 @@ def upload_volume(context, image_service, image_meta, volume_path, image_service.update(context, image_id, {}, image_file) +def check_virtual_size(virtual_size, volume_size, image_id): + virtual_size = int(math.ceil(float(virtual_size) / units.Gi)) + + if virtual_size > volume_size: + params = {'image_size': virtual_size, + 'volume_size': volume_size} + reason = _("Image virtual size is %(image_size)dGB" + " and doesn't fit in a volume of size" + " %(volume_size)dGB.") % params + raise exception.ImageUnacceptable(image_id=image_id, + reason=reason) + return virtual_size + + def is_xenserver_image(context, image_service, image_id): image_meta = image_service.show(context, image_id) return is_xenserver_format(image_meta) diff --git a/cinder/tests/unit/test_image_utils.py b/cinder/tests/unit/test_image_utils.py index c8c4df361..95dd82bda 100644 --- a/cinder/tests/unit/test_image_utils.py +++ b/cinder/tests/unit/test_image_utils.py @@ -24,6 +24,7 @@ from oslo_utils import units from cinder import exception from cinder.image import image_utils from cinder import test +from cinder.tests.unit import fake_constants as fake from cinder.volume import throttling @@ -1390,3 +1391,24 @@ class TestTemporaryFileContextManager(test.TestCase): self.assertEqual(mock.sentinel.temporary_file, tmp_file) self.assertFalse(mock_delete.called) mock_delete.assert_called_once_with(mock.sentinel.temporary_file) + + +class TestImageUtils(test.TestCase): + def test_get_virtual_size(self): + image_id = fake.IMAGE_ID + virtual_size = 1073741824 + volume_size = 2 + virt_size = image_utils.check_virtual_size(virtual_size, + volume_size, + image_id) + self.assertEqual(1, virt_size) + + def test_get_bigger_virtual_size(self): + image_id = fake.IMAGE_ID + virtual_size = 3221225472 + volume_size = 2 + self.assertRaises(exception.ImageUnacceptable, + image_utils.check_virtual_size, + virtual_size, + volume_size, + image_id) diff --git a/cinder/tests/unit/test_rbd.py b/cinder/tests/unit/test_rbd.py index 3efcb749f..e0676fd81 100644 --- a/cinder/tests/unit/test_rbd.py +++ b/cinder/tests/unit/test_rbd.py @@ -21,6 +21,7 @@ import os import tempfile import mock +from oslo_utils import imageutils from oslo_utils import units from cinder import context @@ -1223,14 +1224,19 @@ class ManagedRBDTestCase(test_volume.DriverTestCase): @mock.patch.object(cinder.image.glance, 'get_default_image_service') @mock.patch('cinder.image.image_utils.TemporaryImages.fetch') - def test_create_vol_from_non_raw_image_status_available(self, mock_fetch, - mock_gdis): + @mock.patch('cinder.image.image_utils.qemu_img_info') + def test_create_vol_from_non_raw_image_status_available( + self, mock_qemu_info, mock_fetch, mock_gdis): """Clone non-raw image then verify volume is in available state.""" def _mock_clone_image(context, volume, image_location, image_meta, image_service): return {'provider_location': None}, False + image_info = imageutils.QemuImgInfo() + image_info.virtual_size = '1073741824' + mock_qemu_info.return_value = image_info + mock_fetch.return_value = mock.MagicMock(spec=utils.get_file_spec()) with mock.patch.object(self.volume.driver, 'clone_image') as \ mock_clone_image: diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index 8f5e1796d..baf4db95f 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -32,6 +32,7 @@ import os_brick from oslo_concurrency import processutils from oslo_config import cfg from oslo_serialization import jsonutils +from oslo_utils import imageutils from oslo_utils import importutils from oslo_utils import timeutils from oslo_utils import units @@ -3342,7 +3343,8 @@ class VolumeTestCase(BaseVolumeTestCase): snapshot_ref.destroy() db.volume_destroy(self.context, volume['id']) - def test_create_snapshot_from_bootable_volume(self): + @mock.patch('cinder.image.image_utils.qemu_img_info') + def test_create_snapshot_from_bootable_volume(self, mock_qemu_info): """Test create snapshot from bootable volume.""" # create bootable volume from image volume = self._create_volume_from_image() @@ -3350,6 +3352,10 @@ class VolumeTestCase(BaseVolumeTestCase): self.assertEqual('available', volume['status']) self.assertTrue(volume['bootable']) + image_info = imageutils.QemuImgInfo() + image_info.virtual_size = '1073741824' + mock_qemu_info.return_value = image_info + # get volume's volume_glance_metadata ctxt = context.get_admin_context() vol_glance_meta = db.volume_glance_metadata_get(ctxt, volume_id) @@ -3378,7 +3384,8 @@ class VolumeTestCase(BaseVolumeTestCase): snap.destroy() db.volume_destroy(ctxt, volume_id) - def test_create_snapshot_from_bootable_volume_fail(self): + @mock.patch('cinder.image.image_utils.qemu_img_info') + def test_create_snapshot_from_bootable_volume_fail(self, mock_qemu_info): """Test create snapshot from bootable volume. But it fails to volume_glance_metadata_copy_to_snapshot. @@ -3390,6 +3397,10 @@ class VolumeTestCase(BaseVolumeTestCase): self.assertEqual('available', volume['status']) self.assertTrue(volume['bootable']) + image_info = imageutils.QemuImgInfo() + image_info.virtual_size = '1073741824' + mock_qemu_info.return_value = image_info + # get volume's volume_glance_metadata ctxt = context.get_admin_context() vol_glance_meta = db.volume_glance_metadata_get(ctxt, volume_id) @@ -3543,23 +3554,35 @@ class VolumeTestCase(BaseVolumeTestCase): gigabytes=vol.size) mock_rollback.assert_called_once_with(self.context, ["RESERVATION"]) - def test_create_volume_from_image_cloned_status_available(self): + @mock.patch('cinder.image.image_utils.qemu_img_info') + def test_create_volume_from_image_cloned_status_available( + self, mock_qemu_info): """Test create volume from image via cloning. Verify that after cloning image to volume, it is in available state and is bootable. """ + image_info = imageutils.QemuImgInfo() + image_info.virtual_size = '1073741824' + mock_qemu_info.return_value = image_info + volume = self._create_volume_from_image() self.assertEqual('available', volume['status']) self.assertTrue(volume['bootable']) self.volume.delete_volume(self.context, volume.id, volume=volume) - def test_create_volume_from_image_not_cloned_status_available(self): + @mock.patch('cinder.image.image_utils.qemu_img_info') + def test_create_volume_from_image_not_cloned_status_available( + self, mock_qemu_info): """Test create volume from image via full copy. Verify that after copying image to volume, it is in available state and is bootable. """ + image_info = imageutils.QemuImgInfo() + image_info.virtual_size = '1073741824' + mock_qemu_info.return_value = image_info + volume = self._create_volume_from_image(fakeout_clone_image=True) self.assertEqual('available', volume['status']) self.assertTrue(volume['bootable']) @@ -3599,12 +3622,18 @@ class VolumeTestCase(BaseVolumeTestCase): volume.destroy() os.unlink(dst_path) - def test_create_volume_from_image_copy_exception_rescheduling(self): + @mock.patch('cinder.image.image_utils.qemu_img_info') + def test_create_volume_from_image_copy_exception_rescheduling( + self, mock_qemu_info): """Test create volume with ImageCopyFailure This exception should not trigger rescheduling and allocated_capacity should be incremented so we're having assert for that here. """ + image_info = imageutils.QemuImgInfo() + image_info.virtual_size = '1073741824' + mock_qemu_info.return_value = image_info + def fake_copy_image_to_volume(context, volume, image_service, image_id): raise exception.ImageCopyFailure() @@ -3622,14 +3651,18 @@ class VolumeTestCase(BaseVolumeTestCase): @mock.patch('cinder.utils.brick_get_connector') @mock.patch('cinder.volume.driver.BaseVD.secure_file_operations_enabled') @mock.patch('cinder.volume.driver.BaseVD._detach_volume') - def test_create_volume_from_image_unavailable(self, mock_detach, - mock_secure, *args): + @mock.patch('cinder.image.image_utils.qemu_img_info') + def test_create_volume_from_image_unavailable( + self, mock_qemu_info, mock_detach, mock_secure, *args): """Test create volume with ImageCopyFailure We'll raise an exception inside _connect_device after volume has already been attached to confirm that it detaches the volume. """ mock_secure.side_effect = NameError + image_info = imageutils.QemuImgInfo() + image_info.virtual_size = '1073741824' + mock_qemu_info.return_value = image_info # We want to test BaseVD copy_image_to_volume and since FakeISCSIDriver # inherits from LVM it overwrites it, so we'll mock it to use the @@ -3644,12 +3677,17 @@ class VolumeTestCase(BaseVolumeTestCase): # We must have called detach method. self.assertEqual(1, mock_detach.call_count) - def test_create_volume_from_image_clone_image_volume(self): + @mock.patch('cinder.image.image_utils.qemu_img_info') + def test_create_volume_from_image_clone_image_volume(self, mock_qemu_info): """Test create volume from image via image volume. Verify that after cloning image to volume, it is in available state and is bootable. """ + image_info = imageutils.QemuImgInfo() + image_info.virtual_size = '1073741824' + mock_qemu_info.return_value = image_info + volume = self._create_volume_from_image(clone_image_volume=True) self.assertEqual('available', volume['status']) self.assertTrue(volume['bootable']) @@ -4108,13 +4146,19 @@ class VolumeTestCase(BaseVolumeTestCase): source_volume=volume_src, availability_zone='nova') - def test_create_volume_from_sourcevol_with_glance_metadata(self): + @mock.patch('cinder.image.image_utils.qemu_img_info') + def test_create_volume_from_sourcevol_with_glance_metadata( + self, mock_qemu_info): """Test glance metadata can be correctly copied to new volume.""" def fake_create_cloned_volume(volume, src_vref): pass self.stubs.Set(self.volume.driver, 'create_cloned_volume', fake_create_cloned_volume) + image_info = imageutils.QemuImgInfo() + image_info.virtual_size = '1073741824' + mock_qemu_info.return_value = image_info + volume_src = self._create_volume_from_image() self.volume.create_volume(self.context, volume_src.id, volume=volume_src) @@ -4755,7 +4799,8 @@ class VolumeMigrationTestCase(BaseVolumeTestCase): self.assertEqual('error', volume.migration_status) self.assertEqual('available', volume.status) - def test_migrate_volume_with_glance_metadata(self): + @mock.patch('cinder.image.image_utils.qemu_img_info') + def test_migrate_volume_with_glance_metadata(self, mock_qemu_info): volume = self._create_volume_from_image(clone_image_volume=True) glance_metadata = volume.glance_metadata @@ -4765,6 +4810,10 @@ class VolumeMigrationTestCase(BaseVolumeTestCase): serialized_volume = serializer.serialize_entity(self.context, volume) volume = serializer.deserialize_entity(self.context, serialized_volume) + image_info = imageutils.QemuImgInfo() + image_info.virtual_size = '1073741824' + mock_qemu_info.return_value = image_info + host_obj = {'host': 'newhost', 'capabilities': {}} with mock.patch.object(self.volume.driver, 'migrate_volume') as mock_migrate_volume: diff --git a/cinder/tests/unit/volume/flows/test_create_volume_flow.py b/cinder/tests/unit/volume/flows/test_create_volume_flow.py index 8fc29572d..79bb91b45 100644 --- a/cinder/tests/unit/volume/flows/test_create_volume_flow.py +++ b/cinder/tests/unit/volume/flows/test_create_volume_flow.py @@ -23,6 +23,7 @@ from cinder import context from cinder import exception from cinder import test from cinder.tests.unit.consistencygroup import fake_consistencygroup +from cinder.tests.unit import fake_constants as fakes from cinder.tests.unit import fake_snapshot from cinder.tests.unit import fake_volume from cinder.tests.unit.image import fake as fake_image @@ -632,8 +633,9 @@ class CreateVolumeFlowManagerGlanceCinderBackendCase(test.TestCase): @mock.patch('cinder.volume.flows.manager.create_volume.' 'CreateVolumeFromSpecTask.' '_handle_bootable_volume_glance_meta') - def test_create_from_image_volume(self, handle_bootable, mock_fetch_img, - format='raw', owner=None, + @mock.patch('cinder.image.image_utils.qemu_img_info') + def test_create_from_image_volume(self, mock_qemu_info, handle_bootable, + mock_fetch_img, format='raw', owner=None, location=True): self.flags(allowed_direct_url_schemes=['cinder']) mock_fetch_img.return_value = mock.MagicMock( @@ -646,7 +648,11 @@ class CreateVolumeFlowManagerGlanceCinderBackendCase(test.TestCase): volume = fake_volume.fake_volume_obj(self.ctxt) image_volume = fake_volume.fake_volume_obj(self.ctxt, volume_metadata={}) - image_id = '34e54c31-3bc8-5c1d-9fff-2225bcce4b59' + image_id = fakes.IMAGE_ID + image_info = imageutils.QemuImgInfo() + image_info.virtual_size = '1073741824' + mock_qemu_info.return_value = image_info + url = 'cinder://%s' % image_volume['id'] image_location = None if location: @@ -654,7 +660,9 @@ class CreateVolumeFlowManagerGlanceCinderBackendCase(test.TestCase): image_meta = {'id': image_id, 'container_format': 'bare', 'disk_format': format, - 'owner': owner or self.ctxt.project_id} + 'owner': owner or self.ctxt.project_id, + 'virtual_size': None} + fake_driver.clone_image.return_value = (None, False) fake_db.volume_get_all_by_host.return_value = [image_volume] @@ -716,8 +724,8 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): volume = fake_volume.fake_volume_obj(self.ctxt) image_location = 'someImageLocationStr' - image_id = 'c7a8b8d4-e519-46c7-a0df-ddf1b9b9fff2' - image_meta = mock.Mock() + image_id = fakes.IMAGE_ID + image_meta = {'virtual_size': '1073741824'} manager = create_volume_manager.CreateVolumeFromSpecTask( self.mock_volume_manager, @@ -749,20 +757,21 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): image_meta=image_meta ) + @mock.patch('cinder.image.image_utils.qemu_img_info') def test_create_from_image_cannot_use_cache( - self, mock_get_internal_context, mock_create_from_img_dl, - mock_create_from_src, mock_handle_bootable, mock_fetch_img): + self, mock_qemu_info, mock_get_internal_context, + mock_create_from_img_dl, mock_create_from_src, + mock_handle_bootable, mock_fetch_img): mock_get_internal_context.return_value = None self.mock_driver.clone_image.return_value = (None, False) volume = fake_volume.fake_volume_obj(self.ctxt) + image_info = imageutils.QemuImgInfo() + image_info.virtual_size = '1073741824' + mock_qemu_info.return_value = image_info image_location = 'someImageLocationStr' - image_id = 'c7a8b8d4-e519-46c7-a0df-ddf1b9b9fff2' - image_meta = { - 'properties': { - 'virtual_size': '2147483648' - } - } + image_id = fakes.IMAGE_ID + image_meta = {'virtual_size': '1073741824'} manager = create_volume_manager.CreateVolumeFromSpecTask( self.mock_volume_manager, @@ -808,6 +817,33 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): image_meta=image_meta ) + def test_create_from_image_bigger_size( + self, mock_get_internal_context, + mock_create_from_img_dl, mock_create_from_src, + mock_handle_bootable, mock_fetch_img): + volume = fake_volume.fake_volume_obj(self.ctxt) + + image_location = 'someImageLocationStr' + image_id = fakes.IMAGE_ID + image_meta = {'virtual_size': '2147483648'} + + manager = create_volume_manager.CreateVolumeFromSpecTask( + self.mock_volume_manager, + self.mock_db, + self.mock_driver, + image_volume_cache=self.mock_cache + ) + + self.assertRaises( + exception.ImageUnacceptable, + manager._create_from_image, + self.ctxt, + volume, + image_location, + image_id, + image_meta, + self.mock_image_service) + def test_create_from_image_cache_hit( self, mock_get_internal_context, mock_create_from_img_dl, mock_create_from_src, mock_handle_bootable, mock_fetch_img): @@ -820,8 +856,8 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): volume = fake_volume.fake_volume_obj(self.ctxt) image_location = 'someImageLocationStr' - image_id = 'c7a8b8d4-e519-46c7-a0df-ddf1b9b9fff2' - image_meta = mock.Mock() + image_id = fakes.IMAGE_ID + image_meta = {'virtual_size': None} manager = create_volume_manager.CreateVolumeFromSpecTask( self.mock_volume_manager, @@ -876,7 +912,7 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): mock_volume_get.return_value = volume image_location = 'someImageLocationStr' - image_id = 'c7a8b8d4-e519-46c7-a0df-ddf1b9b9fff2' + image_id = fakes.IMAGE_ID image_meta = mock.MagicMock() manager = create_volume_manager.CreateVolumeFromSpecTask( @@ -944,7 +980,7 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): mock_create_from_img_dl.side_effect = exception.CinderException() image_location = 'someImageLocationStr' - image_id = 'c7a8b8d4-e519-46c7-a0df-ddf1b9b9fff2' + image_id = fakes.IMAGE_ID image_meta = mock.MagicMock() manager = create_volume_manager.CreateVolumeFromSpecTask( @@ -988,20 +1024,21 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): self.assertFalse(self.mock_cache.ensure_space.called) self.assertFalse(self.mock_cache.create_cache_entry.called) + @mock.patch('cinder.image.image_utils.qemu_img_info') def test_create_from_image_no_internal_context( - self, mock_get_internal_context, mock_create_from_img_dl, - mock_create_from_src, mock_handle_bootable, mock_fetch_img): + self, mock_qemu_info, mock_get_internal_context, + mock_create_from_img_dl, mock_create_from_src, + mock_handle_bootable, mock_fetch_img): self.mock_driver.clone_image.return_value = (None, False) mock_get_internal_context.return_value = None volume = fake_volume.fake_volume_obj(self.ctxt) + image_info = imageutils.QemuImgInfo() + image_info.virtual_size = '1073741824' + mock_qemu_info.return_value = image_info image_location = 'someImageLocationStr' - image_id = 'c7a8b8d4-e519-46c7-a0df-ddf1b9b9fff2' - image_meta = { - 'properties': { - 'virtual_size': '2147483648' - } - } + image_id = fakes.IMAGE_ID + image_meta = {'virtual_size': '1073741824'} manager = create_volume_manager.CreateVolumeFromSpecTask( self.mock_volume_manager, @@ -1065,7 +1102,7 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): self.mock_db.volume_create.return_value = image_volume image_location = 'someImageLocationStr' - image_id = 'c7a8b8d4-e519-46c7-a0df-ddf1b9b9fff2' + image_id = fakes.IMAGE_ID image_meta = mock.MagicMock() manager = create_volume_manager.CreateVolumeFromSpecTask( diff --git a/cinder/volume/flows/manager/create_volume.py b/cinder/volume/flows/manager/create_volume.py index 91e3a5a8f..d301bab25 100644 --- a/cinder/volume/flows/manager/create_volume.py +++ b/cinder/volume/flows/manager/create_volume.py @@ -10,14 +10,12 @@ # License for the specific language governing permissions and limitations # under the License. -import math import traceback from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log as logging from oslo_utils import timeutils -from oslo_utils import units import taskflow.engines from taskflow.patterns import linear_flow from taskflow.types import failure as ft @@ -695,6 +693,12 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask): {'volume_id': volume_ref['id'], 'image_location': image_location, 'image_id': image_id}) + virtual_size = image_meta.get('virtual_size') + if virtual_size: + virtual_size = image_utils.check_virtual_size(virtual_size, + volume_ref.size, + image_id) + # Create the volume from an image. # # First see if the driver can clone the image directly. @@ -741,21 +745,12 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask): image_service, context, image_id) as tmp_image: # Try to create the volume as the minimal size, then we can # extend once the image has been downloaded. + data = image_utils.qemu_img_info(tmp_image) + + virtual_size = image_utils.check_virtual_size( + data.virtual_size, volume_ref.size, image_id) + if should_create_cache_entry: - data = image_utils.qemu_img_info(tmp_image) - - virtual_size = int( - math.ceil(float(data.virtual_size) / units.Gi)) - - if virtual_size > volume_ref.size: - params = {'image_size': virtual_size, - 'volume_size': volume_ref.size} - reason = _("Image virtual size is %(image_size)dGB" - " and doesn't fit in a volume of size" - " %(volume_size)dGB.") % params - raise exception.ImageUnacceptable( - image_id=image_id, reason=reason) - if virtual_size and virtual_size != original_size: volume_ref.size = virtual_size volume_ref.save()