From 75d5fabff0946530a96f4066019f7cee515e4d51 Mon Sep 17 00:00:00 2001
From: whoami-rajat <rajatdhasmana@gmail.com>
Date: Thu, 23 Jun 2022 17:41:30 +0530
Subject: [PATCH] Add a check for virtual_size at API layer

When creating a bootable volume from image, we check if the volume
can fit the image which includes several checks like size, min_disk,
virtual_size etc of image should be <= volume size.
We do check the virtual_size but however that check comes at a very
later stage in the cinder-volume process which makes the check
asynchronous.
In a case where nova requests cinder to create a bootable volume
from image, it gets an OK status from cinder-api and then fails
later at the cinder-volume layer but nova isn't aware about the
failure and times out waiting for a reply from cinder.
To avoid the above scenario, we are adding a virtual_size check at
the API layer to fail fast if the volume size doesn't satisfy the
contraints to store the image.

Closes-Bug: #1980268

Change-Id: Ia5950c2dc6186b721c9c9cfc4629d72e3b731889
---
 cinder/tests/unit/test_volume_utils.py        | 30 +++++++++++++++++++
 cinder/volume/volume_utils.py                 |  6 ++++
 ...d-virtual-size-check-42a84f6b24366e5d.yaml | 10 +++++++
 3 files changed, 46 insertions(+)
 create mode 100644 releasenotes/notes/added-virtual-size-check-42a84f6b24366e5d.yaml

diff --git a/cinder/tests/unit/test_volume_utils.py b/cinder/tests/unit/test_volume_utils.py
index ca2b2cbc7f5..c7bd295fd4b 100644
--- a/cinder/tests/unit/test_volume_utils.py
+++ b/cinder/tests/unit/test_volume_utils.py
@@ -1144,6 +1144,36 @@ class VolumeUtilsTestCase(test.TestCase):
                                 vol_size)
         self.assertIn("Image 1 is not active.", str(res))
 
+    @ddt.data(None, 1, 3)
+    def test_check_image_metadata_virtual_size(self, fake_virtual_size):
+        image_meta = {'id': 1, 'min_disk': 1, 'status': 'active',
+                      'size': 1 * units.Gi,
+                      'virtual_size': fake_virtual_size * units.Gi
+                      if fake_virtual_size else None}
+        vol_size = 2
+        if fake_virtual_size and fake_virtual_size > vol_size:
+            res = self.assertRaises(
+                exception.ImageUnacceptable,
+                volume_utils.check_image_metadata,
+                image_meta,
+                vol_size)
+            self.assertIn("Image virtual size is %(image_size)dGB"
+                          " and doesn't fit in a volume of size"
+                          " %(volume_size)dGB." %
+                          {'image_size': fake_virtual_size,
+                           'volume_size': vol_size}, str(res))
+        else:
+            with mock.patch(
+                    'cinder.image.image_utils.check_virtual_size') as \
+                    mock_check_virtual_size:
+                volume_utils.check_image_metadata(image_meta, vol_size)
+                if fake_virtual_size:
+                    mock_check_virtual_size.assert_called_once_with(
+                        image_meta['virtual_size'], vol_size,
+                        image_meta['id'])
+                else:
+                    mock_check_virtual_size.assert_not_called()
+
     def test_enable_volume_bootable(self):
         ctxt = context.get_admin_context()
         volume = test_utils.create_volume(ctxt, bootable=False)
diff --git a/cinder/volume/volume_utils.py b/cinder/volume/volume_utils.py
index b58640ca6f6..6f5333511be 100644
--- a/cinder/volume/volume_utils.py
+++ b/cinder/volume/volume_utils.py
@@ -1125,6 +1125,12 @@ def check_image_metadata(image_meta: dict[str, Union[str, int]],
         msg = msg % {'volume_size': vol_size, 'min_disk': min_disk}
         raise exception.InvalidInput(reason=msg)
 
+    # Check if virtual size is not greater than volume size
+    virtual_size = utils.as_int(image_meta.get('virtual_size'))
+    if virtual_size:
+        image_utils.check_virtual_size(virtual_size, vol_size,
+                                       image_meta['id'])
+
 
 def enable_bootable_flag(volume: 'objects.Volume') -> None:
     try:
diff --git a/releasenotes/notes/added-virtual-size-check-42a84f6b24366e5d.yaml b/releasenotes/notes/added-virtual-size-check-42a84f6b24366e5d.yaml
new file mode 100644
index 00000000000..b82b44d7706
--- /dev/null
+++ b/releasenotes/notes/added-virtual-size-check-42a84f6b24366e5d.yaml
@@ -0,0 +1,10 @@
+---
+fixes:
+  - |
+    `Bug #1980268 <https://bugs.launchpad.net/cinder/+bug/1980268>`_:
+    When creating a volume from an image, a check has been added to compare
+    the requested volume size to the image's ``virtual_size`` property and
+    fail the request if the volume will be too small to contain the image.
+    If the image record does not contain this property, the request is
+    accepted but the volume will go to ``error`` status if the image does
+    not fit (which is the current behavior).