Forbid booting of QCOW2 images with virtual_size > root_gb

Currently, it's possible to boot an instance from a QCOW2 image,
which has virtual_size bigger than one allowed by the given flavor
(root_gb).

The issue is caused by two different problems in the code:

1) typo in get_disk_size() has made it always return None and
   effectively disabled verify_base_size() checks

2) Rbd image backend skips the verify_base_size() step for
   'cached' images (the one with base files), so it is possible to
   boot an instance using a larger flavor once and then use smaller
   flavors to boot the same image, even if allowed root_gb size is
   smaller than the image virtual size

Closes-Bug: #1429093

Change-Id: I383130e5f8cc288f4b428ed43fe4d3aba7169473
(cherry picked from commit c1f9ed27af64e6893d9d0153a964df5aba99b8f0)

3 files changed, 53 insertions, 9 deletions

diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index 0905b5d5e9..861ab177c3 100644
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -6293,9 +6293,10 @@ class LibvirtConnTestCase(test.NoDBTestCase):
         with contextlib.nested(
             mock.patch.object(drvr, '_fetch_instance_kernel_ramdisk'),
             mock.patch.object(libvirt_driver.libvirt_utils, 'fetch_image'),
-            mock.patch.object(drvr, '_create_ephemeral')
+            mock.patch.object(drvr, '_create_ephemeral'),
+            mock.patch.object(imagebackend.Image, 'verify_base_size')
         ) as (fetch_kernel_ramdisk_mock, fetch_image_mock,
-                create_ephemeral_mock):
+                create_ephemeral_mock, verify_base_size_mock):
             drvr._create_images_and_backing(self.context, instance,
                                             "/fake/instance/dir",
                                             disk_info_json)
@@ -6309,6 +6310,12 @@ class LibvirtConnTestCase(test.NoDBTestCase):
             self.assertEqual(
                     os.path.join(base_dir, 'fake_image_backing_file'),
                     m_kwargs['target'])
+            verify_base_size_mock.assert_has_calls([
+                mock.call(os.path.join(base_dir, 'fake_image_backing_file'),
+                          25165824),
+                mock.call(os.path.join(base_dir, 'ephemeral_1_default'),
+                          1073741824)
+            ])
 
     def test_create_images_and_backing_disk_info_none(self):
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@@ -7320,7 +7327,8 @@ class LibvirtConnTestCase(test.NoDBTestCase):
         with contextlib.nested(
             mock.patch.object(utils, 'execute'),
             mock.patch.object(drvr, 'get_info'),
-            mock.patch.object(drvr, '_create_domain_and_network')):
+            mock.patch.object(drvr, '_create_domain_and_network'),
+            mock.patch.object(imagebackend.Image, 'verify_base_size')):
             self.assertRaises(exception.InvalidBDMFormat, drvr._create_image,
                               context, instance, disk_info['mapping'],
                               block_device_info=block_device_info)
diff --git a/nova/tests/unit/virt/libvirt/test_imagebackend.py b/nova/tests/unit/virt/libvirt/test_imagebackend.py
index ba47d07b11..33629fa1b0 100644
--- a/nova/tests/unit/virt/libvirt/test_imagebackend.py
+++ b/nova/tests/unit/virt/libvirt/test_imagebackend.py
@@ -139,6 +139,14 @@ class _ImageTestCase(object):
             self.assertEqual(fs.source_type, "file")
             self.assertEqual(fs.source_file, image.path)
 
+    @mock.patch('nova.virt.disk.api.get_disk_size')
+    def test_get_disk_size(self, get_disk_size):
+        get_disk_size.return_value = 2361393152
+
+        image = self.image_class(self.INSTANCE, self.NAME)
+        self.assertEqual(2361393152, image.get_disk_size(image.path))
+        get_disk_size.assert_called_once_with(image.path)
+
 
 class RawTestCase(_ImageTestCase, test.NoDBTestCase):
 
@@ -393,6 +401,8 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
         fn = self.prepare_mocks()
         fn(max_size=self.SIZE, target=self.TEMPLATE_PATH)
         self.mox.StubOutWithMock(os.path, 'exists')
+        self.mox.StubOutWithMock(imagebackend.Image,
+                                 'verify_base_size')
         if self.OLD_STYLE_INSTANCE_PATH:
             os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False)
         os.path.exists(self.DISK_INFO_PATH).AndReturn(False)
@@ -400,6 +410,7 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
         os.path.exists(self.TEMPLATE_PATH).AndReturn(False)
         os.path.exists(self.PATH).AndReturn(False)
         os.path.exists(self.PATH).AndReturn(False)
+        imagebackend.Image.verify_base_size(self.TEMPLATE_PATH, self.SIZE)
         imagebackend.libvirt_utils.create_cow_image(self.TEMPLATE_PATH,
                                                     self.PATH)
         imagebackend.disk.extend(self.PATH, self.SIZE, use_cow=True)
@@ -434,6 +445,8 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
         self.mox.StubOutWithMock(os.path, 'exists')
         self.mox.StubOutWithMock(imagebackend.libvirt_utils,
                                  'get_disk_backing_file')
+        self.mox.StubOutWithMock(imagebackend.Image,
+                                 'verify_base_size')
         if self.OLD_STYLE_INSTANCE_PATH:
             os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False)
         os.path.exists(self.DISK_INFO_PATH).AndReturn(False)
@@ -444,6 +457,7 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
         imagebackend.libvirt_utils.get_disk_backing_file(self.PATH)\
             .AndReturn(self.QCOW2_BASE)
         os.path.exists(self.QCOW2_BASE).AndReturn(False)
+        imagebackend.Image.verify_base_size(self.TEMPLATE_PATH, self.SIZE)
         imagebackend.libvirt_utils.copy_image(self.TEMPLATE_PATH,
                                               self.QCOW2_BASE)
         imagebackend.disk.extend(self.QCOW2_BASE, self.SIZE, use_cow=True)
@@ -462,6 +476,8 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
         self.mox.StubOutWithMock(os.path, 'exists')
         self.mox.StubOutWithMock(imagebackend.libvirt_utils,
                                  'get_disk_backing_file')
+        self.mox.StubOutWithMock(imagebackend.Image,
+                                 'verify_base_size')
         if self.OLD_STYLE_INSTANCE_PATH:
             os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False)
         os.path.exists(self.DISK_INFO_PATH).AndReturn(False)
@@ -472,6 +488,7 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
 
         imagebackend.libvirt_utils.get_disk_backing_file(self.PATH)\
             .AndReturn(None)
+        imagebackend.Image.verify_base_size(self.TEMPLATE_PATH, self.SIZE)
         os.path.exists(self.PATH).AndReturn(True)
         self.mox.ReplayAll()
 
@@ -1196,6 +1213,8 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase):
         image.get_disk_size(rbd_name).AndReturn(self.SIZE)
         self.mox.StubOutWithMock(image.driver, 'resize')
         image.driver.resize(rbd_name, full_size)
+        self.mox.StubOutWithMock(image, 'verify_base_size')
+        image.verify_base_size(self.TEMPLATE_PATH, full_size)
 
         self.mox.ReplayAll()
 
@@ -1263,6 +1282,25 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase):
 
         self.assertEqual(image.path, rbd_path)
 
+    def test_get_disk_size(self):
+        image = self.image_class(self.INSTANCE, self.NAME)
+        with mock.patch.object(image.driver, 'size') as size_mock:
+            size_mock.return_value = 2361393152
+
+            self.assertEqual(2361393152, image.get_disk_size(image.path))
+            size_mock.assert_called_once_with(image.rbd_name)
+
+    def test_create_image_too_small(self):
+        image = self.image_class(self.INSTANCE, self.NAME)
+        with mock.patch.object(image, 'driver') as driver_mock:
+            driver_mock.exists.return_value = True
+            driver_mock.size.return_value = 2
+
+            self.assertRaises(exception.FlavorDiskTooSmall,
+                              image.create_image, mock.MagicMock(),
+                              self.TEMPLATE_PATH, 1)
+            driver_mock.size.assert_called_once_with(image.rbd_name)
+
 
 class PloopTestCase(_ImageTestCase, test.NoDBTestCase):
     SIZE = 1024
@@ -1271,7 +1309,6 @@ class PloopTestCase(_ImageTestCase, test.NoDBTestCase):
         self.image_class = imagebackend.Ploop
         super(PloopTestCase, self).setUp()
         self.utils = imagebackend.utils
-        self.stubs.Set(imagebackend.Ploop, 'get_disk_size', lambda a, b: 2048)
 
     def prepare_mocks(self):
         fn = self.mox.CreateMockAnything()
@@ -1301,6 +1338,7 @@ class PloopTestCase(_ImageTestCase, test.NoDBTestCase):
         self.mox.VerifyAll()
 
     def test_create_image(self):
+        self.stubs.Set(imagebackend.Ploop, 'get_disk_size', lambda a, b: 2048)
         fn = self.prepare_mocks()
         fn(target=self.TEMPLATE_PATH, max_size=2048, image_id=None)
         img_path = os.path.join(self.PATH, "root.hds")
diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py
index 803429bf75..8655b48f52 100644
--- a/nova/virt/libvirt/imagebackend.py
+++ b/nova/virt/libvirt/imagebackend.py
@@ -281,7 +281,7 @@ class Image(object):
             raise exception.FlavorDiskTooSmall()
 
     def get_disk_size(self, name):
-        disk.get_disk_size(name)
+        return disk.get_disk_size(name)
 
     def snapshot_extract(self, target, out_format):
         raise NotImplementedError()
@@ -478,8 +478,7 @@ class Qcow2(Image):
         # Download the unmodified base image unless we already have a copy.
         if not os.path.exists(base):
             prepare_template(target=base, max_size=size, *args, **kwargs)
-        else:
-            self.verify_base_size(base, size)
+        self.verify_base_size(base, size)
 
         legacy_backing_size = None
         legacy_base = base
@@ -726,13 +725,12 @@ class Rbd(Image):
 
         if not self.check_image_exists():
             prepare_template(target=base, max_size=size, *args, **kwargs)
-        else:
-            self.verify_base_size(base, size)
 
         # prepare_template() may have cloned the image into a new rbd
         # image already instead of downloading it locally
         if not self.check_image_exists():
             self.driver.import_image(base, self.rbd_name)
+        self.verify_base_size(base, size)
 
         if size and size > self.get_disk_size(self.rbd_name):
             self.driver.resize(self.rbd_name, size)