433 lines
19 KiB
Diff
433 lines
19 KiB
Diff
From 135faa7b5d9855312bedc19e5e1ecebae34d3d18 Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <pbrady@redhat.com>
|
|
Date: Fri, 27 Sep 2013 04:07:14 +0100
|
|
Subject: [PATCH] ensure we don't boot oversized images
|
|
|
|
Since we can't generally shrink incoming images, add extra checks
|
|
to ensure oversized images are not allowed through.
|
|
All cases when populating the libvirt image cache are now handled,
|
|
including the initial download from glance, where we avoid
|
|
converting to raw, as that could generate non sparse images
|
|
much larger than the downloaded image.
|
|
|
|
* nova/virt/libvirt/utils.py (fetch_image): Allow passing through
|
|
of the max_size parameter.
|
|
* nova/virt/images.py (fetch_to_raw): Accept the max_size parameter,
|
|
and use it to discard images with larger (virtual) sizes.
|
|
* nova/virt/libvirt/imagebackend.py (verify_base_size): A new
|
|
refactored function to identify and raise exception to oversized images.
|
|
(Raw.create_image): Pass the max_size to the fetch function.
|
|
Also enforce virtual image size checking for already fetched images,
|
|
as this class (despite the name) can be handling qcow files.
|
|
(Qcow2.create_image): Pass the max_size to the fetch function,
|
|
or verify the virtual size for the instance as done previously.
|
|
(Lvm.create_image): Pass the max_size to the fetch function.
|
|
Also check the size before transferring to the volume to improve
|
|
efficiency by not even attempting the transfer of oversized images.
|
|
(Rbd.create_image): Likewise.
|
|
* nova/tests/fake_libvirt_utils.py: Support max_size arg.
|
|
* nova/tests/test_libvirt.py (test_fetch_raw_image):
|
|
Add a case to check oversized images are discarded.
|
|
* nova/tests/test_imagebackend.py (test_create_image_too_small):
|
|
Adjust to avoid the fetch size check.
|
|
|
|
Fixes bug: 1177830
|
|
Fixes bug: 1206081
|
|
|
|
Conflicts:
|
|
|
|
nova/tests/test_imagebackend.py
|
|
nova/virt/libvirt/imagebackend.py
|
|
|
|
Change-Id: Idc35fce580be4f74e23883d1b4bea6475c3f6e30
|
|
---
|
|
nova/tests/fake_libvirt_utils.py | 2 +-
|
|
nova/tests/test_imagebackend.py | 35 ++++++++++-------------------
|
|
nova/tests/test_libvirt.py | 24 +++++++++++++++++---
|
|
nova/virt/images.py | 24 +++++++++++++++++---
|
|
nova/virt/libvirt/imagebackend.py | 47 ++++++++++++++++++++++++++++++---------
|
|
nova/virt/libvirt/utils.py | 5 +++--
|
|
6 files changed, 95 insertions(+), 42 deletions(-)
|
|
|
|
diff --git a/nova/tests/fake_libvirt_utils.py b/nova/tests/fake_libvirt_utils.py
|
|
index 23b758e..ecf357a 100644
|
|
--- a/nova/tests/fake_libvirt_utils.py
|
|
+++ b/nova/tests/fake_libvirt_utils.py
|
|
@@ -193,7 +193,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/test_imagebackend.py b/nova/tests/test_imagebackend.py
|
|
index 77446e8..93ed23d 100644
|
|
--- a/nova/tests/test_imagebackend.py
|
|
+++ b/nova/tests/test_imagebackend.py
|
|
@@ -189,7 +189,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()
|
|
|
|
@@ -210,7 +210,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)
|
|
self.mox.ReplayAll()
|
|
@@ -260,7 +260,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()
|
|
@@ -272,15 +272,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)
|
|
@@ -294,27 +291,24 @@ 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()
|
|
|
|
image = self.image_class(self.INSTANCE, self.NAME)
|
|
- self.assertRaises(exception.ImageTooLarge, image.create_image, fn,
|
|
- self.TEMPLATE_PATH, 1)
|
|
+ self.assertRaises(exception.InstanceTypeDiskTooSmall,
|
|
+ image.create_image, fn, self.TEMPLATE_PATH, 1)
|
|
self.mox.VerifyAll()
|
|
|
|
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:
|
|
@@ -329,8 +323,6 @@ def test_generate_resized_backing_files(self):
|
|
self.QCOW2_BASE)
|
|
imagebackend.disk.extend(self.QCOW2_BASE, self.SIZE)
|
|
|
|
- imagebackend.disk.get_disk_size(self.TEMPLATE_PATH
|
|
- ).AndReturn(self.SIZE)
|
|
os.path.exists(self.PATH).AndReturn(True)
|
|
self.mox.ReplayAll()
|
|
|
|
@@ -341,9 +333,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:
|
|
@@ -353,8 +344,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()
|
|
|
|
@@ -391,7 +380,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,
|
|
@@ -423,7 +412,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
|
|
@@ -462,7 +451,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,
|
|
diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py
|
|
index d8c4cf2..e422ec7 100644
|
|
--- a/nova/tests/test_libvirt.py
|
|
+++ b/nova/tests/test_libvirt.py
|
|
@@ -4826,7 +4826,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,
|
|
@@ -4856,20 +4857,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(utils, 'delete_if_exists', fake_rm_on_errror)
|
|
|
|
@@ -4884,7 +4892,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'
|
|
@@ -4901,6 +4910,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 b40f566..541779a 100755
|
|
--- a/nova/virt/images.py
|
|
+++ b/nova/virt/images.py
|
|
@@ -190,7 +190,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
|
|
@@ -202,9 +202,10 @@ def fetch(context, image_href, path, _user_id, _project_id):
|
|
image_service.download(context, image_id, image_file)
|
|
|
|
|
|
-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 utils.remove_path_on_error(path_tmp):
|
|
data = qemu_img_info(path_tmp)
|
|
@@ -220,6 +221,23 @@ def fetch_to_raw(context, image_href, path, user_id, project_id):
|
|
raise exception.ImageUnacceptable(image_id=image_href,
|
|
reason=_("fmt=%(fmt)s backed by: %(backing_file)s") % locals())
|
|
|
|
+ # 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 e2c7ccf..dc85c97 100755
|
|
--- a/nova/virt/libvirt/imagebackend.py
|
|
+++ b/nova/virt/libvirt/imagebackend.py
|
|
@@ -177,6 +177,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
|
|
|
|
@@ -217,7 +247,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 utils.remove_path_on_error(self.path):
|
|
copy_raw_image(base, self.path, size)
|
|
@@ -257,7 +288,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
|
|
@@ -283,13 +316,6 @@ def copy_qcow2_image(base, target, size):
|
|
libvirt_utils.copy_image(base, legacy_base)
|
|
disk.extend(legacy_base, legacy_backing_size)
|
|
|
|
- # 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 and size < disk.get_disk_size(base):
|
|
- LOG.error('%s virtual size larger than flavor root disk size %s' %
|
|
- (base, size))
|
|
- raise exception.ImageTooLarge()
|
|
if not os.path.exists(self.path):
|
|
with utils.remove_path_on_error(self.path):
|
|
copy_qcow2_image(base, self.path, size)
|
|
@@ -348,6 +374,7 @@ def create_image(self, prepare_template, base, size, *args, **kwargs):
|
|
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,
|
|
@@ -365,7 +392,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)
|
|
|
|
diff --git a/nova/virt/libvirt/utils.py b/nova/virt/libvirt/utils.py
|
|
index 6972243..4c31fcb 100755
|
|
--- a/nova/virt/libvirt/utils.py
|
|
+++ b/nova/virt/libvirt/utils.py
|
|
@@ -592,9 +592,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):
|
|
--
|
|
1.8.4
|
|
|