diff --git a/nova/tests/virt/libvirt/fake_libvirt_utils.py b/nova/tests/virt/libvirt/fake_libvirt_utils.py index bb4174eaf17..964d7484f4d 100644 --- a/nova/tests/virt/libvirt/fake_libvirt_utils.py +++ b/nova/tests/virt/libvirt/fake_libvirt_utils.py @@ -197,7 +197,7 @@ def get_fs_info(path): 'free': 84 * (1024 ** 3)} -def fetch_image(context, target, image_id, user_id, project_id): +def fetch_image(context, target, image_id, user_id, project_id, max_size=0): pass diff --git a/nova/tests/virt/libvirt/test_imagebackend.py b/nova/tests/virt/libvirt/test_imagebackend.py index 8d159fd7841..2ead87cf027 100644 --- a/nova/tests/virt/libvirt/test_imagebackend.py +++ b/nova/tests/virt/libvirt/test_imagebackend.py @@ -193,7 +193,7 @@ def prepare_mocks(self): def test_create_image(self): fn = self.prepare_mocks() - fn(target=self.TEMPLATE_PATH, image_id=None) + fn(target=self.TEMPLATE_PATH, max_size=None, image_id=None) imagebackend.libvirt_utils.copy_image(self.TEMPLATE_PATH, self.PATH) self.mox.ReplayAll() @@ -214,7 +214,7 @@ def test_create_image_generated(self): def test_create_image_extend(self): fn = self.prepare_mocks() - fn(target=self.TEMPLATE_PATH, image_id=None) + fn(max_size=self.SIZE, target=self.TEMPLATE_PATH, image_id=None) imagebackend.libvirt_utils.copy_image(self.TEMPLATE_PATH, self.PATH) imagebackend.disk.extend(self.PATH, self.SIZE, use_cow=False) self.mox.ReplayAll() @@ -264,7 +264,7 @@ def prepare_mocks(self): def test_create_image(self): fn = self.prepare_mocks() - fn(target=self.TEMPLATE_PATH) + fn(max_size=None, target=self.TEMPLATE_PATH) imagebackend.libvirt_utils.create_cow_image(self.TEMPLATE_PATH, self.PATH) self.mox.ReplayAll() @@ -276,15 +276,12 @@ def test_create_image(self): def test_create_image_with_size(self): fn = self.prepare_mocks() - fn(target=self.TEMPLATE_PATH) + fn(max_size=self.SIZE, target=self.TEMPLATE_PATH) self.mox.StubOutWithMock(os.path, 'exists') - self.mox.StubOutWithMock(imagebackend.disk, 'get_disk_size') if self.OLD_STYLE_INSTANCE_PATH: os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False) os.path.exists(self.TEMPLATE_PATH).AndReturn(False) os.path.exists(self.PATH).AndReturn(False) - imagebackend.disk.get_disk_size(self.TEMPLATE_PATH - ).AndReturn(self.SIZE) os.path.exists(self.PATH).AndReturn(False) imagebackend.libvirt_utils.create_cow_image(self.TEMPLATE_PATH, self.PATH) @@ -298,13 +295,11 @@ def test_create_image_with_size(self): def test_create_image_too_small(self): fn = self.prepare_mocks() - fn(target=self.TEMPLATE_PATH) self.mox.StubOutWithMock(os.path, 'exists') self.mox.StubOutWithMock(imagebackend.disk, 'get_disk_size') if self.OLD_STYLE_INSTANCE_PATH: os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False) - os.path.exists(self.TEMPLATE_PATH).AndReturn(False) - os.path.exists(self.PATH).AndReturn(False) + os.path.exists(self.TEMPLATE_PATH).AndReturn(True) imagebackend.disk.get_disk_size(self.TEMPLATE_PATH ).AndReturn(self.SIZE) self.mox.ReplayAll() @@ -316,9 +311,8 @@ def test_create_image_too_small(self): def test_generate_resized_backing_files(self): fn = self.prepare_mocks() - fn(target=self.TEMPLATE_PATH) + fn(max_size=self.SIZE, target=self.TEMPLATE_PATH) self.mox.StubOutWithMock(os.path, 'exists') - self.mox.StubOutWithMock(imagebackend.disk, 'get_disk_size') self.mox.StubOutWithMock(imagebackend.libvirt_utils, 'get_disk_backing_file') if self.OLD_STYLE_INSTANCE_PATH: @@ -333,8 +327,6 @@ def test_generate_resized_backing_files(self): self.QCOW2_BASE) imagebackend.disk.extend(self.QCOW2_BASE, self.SIZE, use_cow=True) - imagebackend.disk.get_disk_size(self.TEMPLATE_PATH - ).AndReturn(self.SIZE) os.path.exists(self.PATH).AndReturn(True) self.mox.ReplayAll() @@ -345,9 +337,8 @@ def test_generate_resized_backing_files(self): def test_qcow2_exists_and_has_no_backing_file(self): fn = self.prepare_mocks() - fn(target=self.TEMPLATE_PATH) + fn(max_size=self.SIZE, target=self.TEMPLATE_PATH) self.mox.StubOutWithMock(os.path, 'exists') - self.mox.StubOutWithMock(imagebackend.disk, 'get_disk_size') self.mox.StubOutWithMock(imagebackend.libvirt_utils, 'get_disk_backing_file') if self.OLD_STYLE_INSTANCE_PATH: @@ -357,8 +348,6 @@ def test_qcow2_exists_and_has_no_backing_file(self): imagebackend.libvirt_utils.get_disk_backing_file(self.PATH)\ .AndReturn(None) - imagebackend.disk.get_disk_size(self.TEMPLATE_PATH - ).AndReturn(self.SIZE) os.path.exists(self.PATH).AndReturn(True) self.mox.ReplayAll() @@ -395,7 +384,7 @@ def prepare_mocks(self): def _create_image(self, sparse): fn = self.prepare_mocks() - fn(target=self.TEMPLATE_PATH) + fn(max_size=None, target=self.TEMPLATE_PATH) self.libvirt_utils.create_lvm_image(self.VG, self.LV, self.TEMPLATE_SIZE, @@ -427,7 +416,7 @@ def _create_image_generated(self, sparse): def _create_image_resize(self, sparse): fn = self.prepare_mocks() - fn(target=self.TEMPLATE_PATH) + fn(max_size=self.SIZE, target=self.TEMPLATE_PATH) self.libvirt_utils.create_lvm_image(self.VG, self.LV, self.SIZE, sparse=sparse) self.disk.get_disk_size(self.TEMPLATE_PATH @@ -466,7 +455,7 @@ def test_create_image_resize_sparsed(self): def test_create_image_negative(self): fn = self.prepare_mocks() - fn(target=self.TEMPLATE_PATH) + fn(max_size=self.SIZE, target=self.TEMPLATE_PATH) self.libvirt_utils.create_lvm_image(self.VG, self.LV, self.SIZE, @@ -514,6 +503,7 @@ def fake_fetch(target, *args, **kwargs): return self.stubs.Set(os.path, 'exists', lambda _: True) + self.stubs.Set(image, 'check_image_exists', lambda: True) image.cache(fake_fetch, self.TEMPLATE_PATH, self.SIZE) @@ -612,7 +602,7 @@ def test_cache_template_exists(self): def test_create_image(self): fn = self.prepare_mocks() - fn(rbd=self.rbd, target=self.TEMPLATE_PATH) + fn(max_size=None, rbd=self.rbd, target=self.TEMPLATE_PATH) self.rbd.RBD_FEATURE_LAYERING = 1 diff --git a/nova/tests/virt/libvirt/test_libvirt.py b/nova/tests/virt/libvirt/test_libvirt.py index c3d2e90a273..6d8916585f2 100644 --- a/nova/tests/virt/libvirt/test_libvirt.py +++ b/nova/tests/virt/libvirt/test_libvirt.py @@ -6525,7 +6525,8 @@ def test_fetch_image(self): image_id = '4' user_id = 'fake' project_id = 'fake' - images.fetch_to_raw(context, image_id, target, user_id, project_id) + images.fetch_to_raw(context, image_id, target, user_id, project_id, + max_size=0) self.mox.ReplayAll() libvirt_utils.fetch_image(context, target, image_id, @@ -6555,20 +6556,27 @@ class FakeImgInfo(object): file_format = path.split('.')[-2] elif file_format == 'converted': file_format = 'raw' + if 'backing' in path: backing_file = 'backing' else: backing_file = None + if 'big' in path: + virtual_size = 2 + else: + virtual_size = 1 + FakeImgInfo.file_format = file_format FakeImgInfo.backing_file = backing_file + FakeImgInfo.virtual_size = virtual_size return FakeImgInfo() self.stubs.Set(utils, 'execute', fake_execute) self.stubs.Set(os, 'rename', fake_rename) self.stubs.Set(os, 'unlink', fake_unlink) - self.stubs.Set(images, 'fetch', lambda *_: None) + self.stubs.Set(images, 'fetch', lambda *_, **__: None) self.stubs.Set(images, 'qemu_img_info', fake_qemu_img_info) self.stubs.Set(fileutils, 'delete_if_exists', fake_rm_on_error) @@ -6590,7 +6598,8 @@ class FakeImgInfo(object): 't.qcow2.part', 't.qcow2.converted'), ('rm', 't.qcow2.part'), ('mv', 't.qcow2.converted', 't.qcow2')] - images.fetch_to_raw(context, image_id, target, user_id, project_id) + images.fetch_to_raw(context, image_id, target, user_id, project_id, + max_size=1) self.assertEqual(self.executes, expected_commands) target = 't.raw' @@ -6607,6 +6616,15 @@ class FakeImgInfo(object): context, image_id, target, user_id, project_id) self.assertEqual(self.executes, expected_commands) + target = 'big.qcow2' + self.executes = [] + expected_commands = [('rm', '-f', 'big.qcow2.part')] + self.assertRaises(exception.InstanceTypeDiskTooSmall, + images.fetch_to_raw, + context, image_id, target, user_id, project_id, + max_size=1) + self.assertEqual(self.executes, expected_commands) + del self.executes def test_get_disk_backing_file(self): diff --git a/nova/virt/images.py b/nova/virt/images.py index 8cc7603d6c8..5c0ce39562a 100644 --- a/nova/virt/images.py +++ b/nova/virt/images.py @@ -61,7 +61,7 @@ def convert_image(source, dest, out_format, run_as_root=False): utils.execute(*cmd, run_as_root=run_as_root) -def fetch(context, image_href, path, _user_id, _project_id): +def fetch(context, image_href, path, _user_id, _project_id, max_size=0): # TODO(vish): Improve context handling and add owner and auth data # when it is added to glance. Right now there is no # auth checking in glance, so we assume that access was @@ -72,9 +72,10 @@ def fetch(context, image_href, path, _user_id, _project_id): image_service.download(context, image_id, dst_path=path) -def fetch_to_raw(context, image_href, path, user_id, project_id): +def fetch_to_raw(context, image_href, path, user_id, project_id, max_size=0): path_tmp = "%s.part" % path - fetch(context, image_href, path_tmp, user_id, project_id) + fetch(context, image_href, path_tmp, user_id, project_id, + max_size=max_size) with fileutils.remove_path_on_error(path_tmp): data = qemu_img_info(path_tmp) @@ -91,6 +92,23 @@ def fetch_to_raw(context, image_href, path, user_id, project_id): reason=(_("fmt=%(fmt)s backed by: %(backing_file)s") % {'fmt': fmt, 'backing_file': backing_file})) + # We can't generally shrink incoming images, so disallow + # images > size of the flavor we're booting. Checking here avoids + # an immediate DoS where we convert large qcow images to raw + # (which may compress well but not be sparse). + # TODO(p-draigbrady): loop through all flavor sizes, so that + # we might continue here and not discard the download. + # If we did that we'd have to do the higher level size checks + # irrespective of whether the base image was prepared or not. + disk_size = data.virtual_size + if max_size and max_size < disk_size: + msg = _('%(base)s virtual size %(disk_size)s ' + 'larger than flavor root disk size %(size)s') + LOG.error(msg % {'base': path, + 'disk_size': disk_size, + 'size': max_size}) + raise exception.InstanceTypeDiskTooSmall() + if fmt != "raw" and CONF.force_raw_images: staged = "%s.converted" % path LOG.debug("%s was %s, converting to raw" % (image_href, fmt)) diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py index a8fa68f5197..6ec7372d303 100644 --- a/nova/virt/libvirt/imagebackend.py +++ b/nova/virt/libvirt/imagebackend.py @@ -198,6 +198,36 @@ def _can_fallocate(self): (CONF.preallocate_images, self.path)) return can_fallocate + @staticmethod + def verify_base_size(base, size, base_size=0): + """Check that the base image is not larger than size. + Since images can't be generally shrunk, enforce this + constraint taking account of virtual image size. + """ + + # Note(pbrady): The size and min_disk parameters of a glance + # image are checked against the instance size before the image + # is even downloaded from glance, but currently min_disk is + # adjustable and doesn't currently account for virtual disk size, + # so we need this extra check here. + # NOTE(cfb): Having a flavor that sets the root size to 0 and having + # nova effectively ignore that size and use the size of the + # image is considered a feature at this time, not a bug. + + if size is None: + return + + if size and not base_size: + base_size = disk.get_disk_size(base) + + if size < base_size: + msg = _('%(base)s virtual size %(base_size)s ' + 'larger than flavor root disk size %(size)s') + LOG.error(msg % {'base': base, + 'base_size': base_size, + 'size': size}) + raise exception.InstanceTypeDiskTooSmall() + def snapshot_create(self): raise NotImplementedError() @@ -239,7 +269,8 @@ def copy_raw_image(base, target, size): #Generating image in place prepare_template(target=self.path, *args, **kwargs) else: - prepare_template(target=base, *args, **kwargs) + prepare_template(target=base, max_size=size, *args, **kwargs) + self.verify_base_size(base, size) if not os.path.exists(self.path): with fileutils.remove_path_on_error(self.path): copy_raw_image(base, self.path, size) @@ -278,7 +309,9 @@ def copy_qcow2_image(base, target, size): # Download the unmodified base image unless we already have a copy. if not os.path.exists(base): - prepare_template(target=base, *args, **kwargs) + prepare_template(target=base, max_size=size, *args, **kwargs) + else: + self.verify_base_size(base, size) legacy_backing_size = None legacy_base = base @@ -304,17 +337,6 @@ def copy_qcow2_image(base, target, size): libvirt_utils.copy_image(base, legacy_base) disk.extend(legacy_base, legacy_backing_size, use_cow=True) - # NOTE(cfb): Having a flavor that sets the root size to 0 and having - # nova effectively ignore that size and use the size of the - # image is considered a feature at this time, not a bug. - disk_size = disk.get_disk_size(base) - if size and size < disk_size: - msg = _('%(base)s virtual size %(disk_size)s' - 'larger than flavor root disk size %(size)s') - LOG.error(msg % {'base': base, - 'disk_size': disk_size, - 'size': size}) - raise exception.InstanceTypeDiskTooSmall() if not os.path.exists(self.path): with fileutils.remove_path_on_error(self.path): copy_qcow2_image(base, self.path, size) @@ -372,6 +394,7 @@ def create_image(self, prepare_template, base, size, *args, **kwargs): @utils.synchronized(base, external=True, lock_path=self.lock_path) def create_lvm_image(base, size): base_size = disk.get_disk_size(base) + self.verify_base_size(base, size, base_size=base_size) resize = size > base_size size = size if resize else base_size libvirt_utils.create_lvm_image(self.vg, self.lv, @@ -389,7 +412,7 @@ def create_lvm_image(base, size): with self.remove_volume_on_error(self.path): prepare_template(target=self.path, *args, **kwargs) else: - prepare_template(target=base, *args, **kwargs) + prepare_template(target=base, max_size=size, *args, **kwargs) with self.remove_volume_on_error(self.path): create_lvm_image(base, size) @@ -590,7 +613,9 @@ def create_image(self, prepare_template, base, size, *args, **kwargs): features = self.rbd.RBD_FEATURE_LAYERING if not os.path.exists(base): - prepare_template(target=base, *args, **kwargs) + prepare_template(target=base, max_size=size, *args, **kwargs) + else: + self.verify_base_size(base, size) # keep using the command line import instead of librbd since it # detects zeroes to preserve sparseness in the image diff --git a/nova/virt/libvirt/utils.py b/nova/virt/libvirt/utils.py index 4b85b799804..65414777e9c 100644 --- a/nova/virt/libvirt/utils.py +++ b/nova/virt/libvirt/utils.py @@ -640,9 +640,10 @@ def get_fs_info(path): 'used': used} -def fetch_image(context, target, image_id, user_id, project_id): +def fetch_image(context, target, image_id, user_id, project_id, max_size=0): """Grab image.""" - images.fetch_to_raw(context, image_id, target, user_id, project_id) + images.fetch_to_raw(context, image_id, target, user_id, project_id, + max_size=max_size) def get_instance_path(instance, forceold=False, relative=False):