From 943124ed6c0f2e8caf9ceb296174e71b47fc6f4f Mon Sep 17 00:00:00 2001
From: Dan Smith <dansmith@redhat.com>
Date: Wed, 9 Nov 2022 07:47:46 -0800
Subject: Further robustification of format_inspector

This adds a little more caution around the format_inspector,
specifically when calling virtual_size in case something very
unexpected goes wrong to ensure we don't interrupt the upload
process.

Change-Id: I14648e7b724b771755cd10fc5f5362a826780dd7
Related-Bug: #1983279
---
 glance/location.py                    | 16 ++++++++++++----
 glance/tests/unit/test_store_image.py | 31 +++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/glance/location.py b/glance/location.py
index 13e73e21a..cf05fcdb2 100644
--- a/glance/location.py
+++ b/glance/location.py
@@ -584,15 +584,23 @@ class ImageProxy(glance.domain.proxy.Image):
 
         self._upload_to_store(data, verifier, backend, size)
 
-        if fmt and fmt.format_match and fmt.virtual_size:
-            self.image.virtual_size = fmt.virtual_size
-            LOG.info('Image format matched and virtual size computed: %i',
-                     self.image.virtual_size)
+        virtual_size = 0
+        if fmt and fmt.format_match:
+            try:
+                virtual_size = fmt.virtual_size
+                LOG.info('Image format matched and virtual size computed: %i',
+                         virtual_size)
+            except Exception as e:
+                LOG.error(_LE('Unable to determine virtual_size because: %s'),
+                          e)
         elif fmt:
             LOG.warning('Image format %s did not match; '
                         'unable to calculate virtual size',
                         self.image.disk_format)
 
+        if virtual_size:
+            self.image.virtual_size = fmt.virtual_size
+
         if set_active and self.image.status != 'active':
             self.image.status = 'active'
 
diff --git a/glance/tests/unit/test_store_image.py b/glance/tests/unit/test_store_image.py
index 50bda54a2..01b1fc4ff 100644
--- a/glance/tests/unit/test_store_image.py
+++ b/glance/tests/unit/test_store_image.py
@@ -283,6 +283,37 @@ class TestStoreImage(utils.BaseTestCase):
         self.assertEqual('active', image.status)
         self.assertEqual(0, image.virtual_size)
 
+    @mock.patch('glance.common.format_inspector.QcowInspector.virtual_size',
+                new_callable=mock.PropertyMock)
+    @mock.patch('glance.common.format_inspector.QcowInspector.format_match',
+                new_callable=mock.PropertyMock)
+    def test_image_set_data_inspector_virtual_size_failure(self, mock_fm,
+                                                           mock_vs):
+        # Force our format to match
+        mock_fm.return_value = True
+
+        # Make virtual_size fail in some unexpected way
+        mock_vs.side_effect = ValueError('some error')
+
+        context = glance.context.RequestContext(user=USER1)
+        image_stub = ImageStub(UUID2, status='queued', locations=[])
+        image_stub.disk_format = 'qcow2'
+        # We are going to pass an iterable data source, so use the
+        # FakeStoreAPIReader that actually reads from that data
+        store_api = unit_test_utils.FakeStoreAPIReader()
+        image = glance.location.ImageProxy(image_stub, context,
+                                           store_api, self.store_utils)
+
+        # Make sure set_data proceeds even though the format clearly
+        # does not match
+        image.set_data(iter(['YYYY']), 4)
+        self.assertEqual(4, image.size)
+        # NOTE(markwash): FakeStore returns image_id for location
+        self.assertEqual(UUID2, image.locations[0]['url'])
+        self.assertEqual('Z', image.checksum)
+        self.assertEqual('active', image.status)
+        self.assertEqual(0, image.virtual_size)
+
     @mock.patch('glance.common.format_inspector.get_inspector')
     def test_image_set_data_inspector_not_needed(self, mock_gi):
         context = glance.context.RequestContext(user=USER1)
-- 
cgit v1.2.1