Image virtual size doesn't fit to volume size

Glance gives images' sizes in bytes. But virtual size of image can have
different value when image created in another format (e.g. qcow2) then
raw. Raise exception when trying create volume from an image bigger size
then volume size.

This commit uses fake value image_id from cinder/tests/unit/fake_constants.py
to clean up this test module consistent with similar cleanups in the codebase.

Closes-Bug: #1599147
Change-Id: Ia8f644ac59c4ee05742ec8ce5affa682e9a90a78

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)

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)

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.patch('cinder.image.image_utils.qemu_img_info')
-                                                            mock_gdis):
+    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:

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.patch('cinder.image.image_utils.qemu_img_info')
-                                                  mock_secure, *args):
+    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:

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,
+    @mock.patch('cinder.image.image_utils.qemu_img_info')
-                                      format='raw', owner=None,
+    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_id = fakes.IMAGE_ID
-        image_meta = mock.Mock()
+        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,
+            self, mock_qemu_info, mock_get_internal_context,
-            mock_create_from_src, mock_handle_bootable, mock_fetch_img):
+            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_id = fakes.IMAGE_ID
-        image_meta = {
+        image_meta = {'virtual_size': '1073741824'}
-            'properties': {
-                'virtual_size': '2147483648'
-            }
-        }
 
         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_id = fakes.IMAGE_ID
-        image_meta = mock.Mock()
+        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,
+            self, mock_qemu_info, mock_get_internal_context,
-            mock_create_from_src, mock_handle_bootable, mock_fetch_img):
+            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_id = fakes.IMAGE_ID
-        image_meta = {
+        image_meta = {'virtual_size': '1073741824'}
-            'properties': {
-                'virtual_size': '2147483648'
-            }
-        }
 
         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(

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()