From b29bc97d5a69abe71dea5b9ff9dcfc65fcd59cc9 Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Thu, 11 Aug 2011 17:10:25 -0500 Subject: Check compressed image size and PEP8 cleanup. --- nova/compute/manager.py | 40 +++++++++++++++++++++++ nova/exception.py | 4 +++ nova/tests/api/openstack/contrib/test_keypairs.py | 17 ++++++++-- 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index d38213083..1c3485342 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -321,10 +321,50 @@ class ComputeManager(manager.SchedulerDependentManager): def _run_instance(self, context, instance_id, **kwargs): """Launch a new instance with specified options.""" + def _check_image_size(): + """Ensure image is smaller than the maximum size allowed by the + instance_type. + + The image stored in Glance is potentially compressed, so we use two + checks to ensure that the size isn't exceeded: + + 1) This one - checks compressed size, this a quick check to + eliminate any images which are obviously too large + + 2) Check uncompressed size in nova.virt.xenapi.vm_utils. This + is a slower check since it requires uncompressing the entire + image, but is accurate because it reflects the image's + actual size. + """ + image_href = instance['image_ref'] + image_service, image_id = nova.image.get_image_service(image_href) + image_meta = image_service.show(context, image_id) + size_bytes = image_meta['size'] + + instance_type_id = instance['instance_type_id'] + instance_type = self.db.instance_type_get(context, + instance_type_id) + allowed_size_gb = instance_type['local_gb'] + allowed_size_bytes = allowed_size_gb * 1024 * 1024 * 1024 + + LOG.debug(_("image_id=%(image_id)d, image_size_bytes=" + "%(size_bytes)d, allowed_size_bytes=" + "%(allowed_size_bytes)d") % locals()) + + if size_bytes > allowed_size_bytes: + LOG.info(_("Image '%(image_id)d' size %(size_bytes)d exceeded" + " instance_type allowed size " + "%(allowed_size_bytes)d") + % locals()) + raise exception.ImageTooLarge() + context = context.elevated() instance = self.db.instance_get(context, instance_id) if instance['name'] in self.driver.list_instances(): raise exception.Error(_("Instance has already been created")) + + _check_image_size() + LOG.audit(_("instance %s: starting..."), instance_id, context=context) updates = {} diff --git a/nova/exception.py b/nova/exception.py index 0d60cb0bf..77ebf3c88 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -717,3 +717,7 @@ class CannotResizeToSameSize(NovaException): class CannotResizeToSmallerSize(NovaException): message = _("Resizing to a smaller size is not supported.") + + +class ImageTooLarge(NovaException): + message = _("Image is larger than instance type allows") diff --git a/nova/tests/api/openstack/contrib/test_keypairs.py b/nova/tests/api/openstack/contrib/test_keypairs.py index c9dc34d65..cbc815b1a 100644 --- a/nova/tests/api/openstack/contrib/test_keypairs.py +++ b/nova/tests/api/openstack/contrib/test_keypairs.py @@ -28,6 +28,7 @@ def fake_keypair(name): 'fingerprint': 'FAKE_FINGERPRINT', 'name': name} + def db_key_pair_get_all_by_user(self, user_id): return [fake_keypair('FAKE')] @@ -78,7 +79,20 @@ class KeypairsTest(test.TestCase): def test_keypair_import(self): body = {'keypair': {'name': 'create_test', - 'public_key': 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDBYIznAx9D7118Q1VKGpXy2HDiKyUTM8XcUuhQpo0srqb9rboUp4a9NmCwpWpeElDLuva707GOUnfaBAvHBwsRXyxHJjRaI6YQj2oLJwqvaSaWUbyT1vtryRqy6J3TecN0WINY71f4uymiMZP0wby4bKBcYnac8KiCIlvkEl0ETjkOGUq8OyWRmn7ljj5SESEUdBP0JnuTFKddWTU/wD6wydeJaUhBTqOlHn0kX1GyqoNTE1UEhcM5ZRWgfUZfTjVyDF2kGj3vJLCJtJ8LoGcj7YaN4uPg1rBle+izwE/tLonRrds+cev8p6krSSrxWOwBbHkXa6OciiJDvkRzJXzf'}} + 'public_key': 'ssh-rsa ' + 'AAAAB3NzaC1yc2EAAAADAQABAAABAQDBY' + 'IznAx9D7118Q1VKGpXy2HDiKyUTM8XcUu' + 'hQpo0srqb9rboUp4a9NmCwpWpeElDLuva' + '707GOUnfaBAvHBwsRXyxHJjRaI6YQj2oL' + 'JwqvaSaWUbyT1vtryRqy6J3TecN0WINY7' + '1f4uymiMZP0wby4bKBcYnac8KiCIlvkEl' + '0ETjkOGUq8OyWRmn7ljj5SESEUdBP0Jnu' + 'TFKddWTU/wD6wydeJaUhBTqOlHn0kX1Gy' + 'qoNTE1UEhcM5ZRWgfUZfTjVyDF2kGj3vJ' + 'LCJtJ8LoGcj7YaN4uPg1rBle+izwE/tLo' + 'nRrds+cev8p6krSSrxWOwBbHkXa6OciiJ' + 'DvkRzJXzf'}} + req = webob.Request.blank('/v1.1/os-keypairs') req.method = 'POST' req.body = json.dumps(body) @@ -96,4 +110,3 @@ class KeypairsTest(test.TestCase): req.headers['Content-Type'] = 'application/json' res = req.get_response(fakes.wsgi_app()) self.assertEqual(res.status_int, 202) - -- cgit From 24796debe819641b1cba58ba966b0d6d5a253fd8 Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Fri, 12 Aug 2011 12:14:40 -0500 Subject: Fixed unit tests. --- nova/compute/manager.py | 15 ++++++++++++++- nova/tests/scheduler/test_scheduler.py | 4 +++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 1c3485342..f0184161e 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -336,10 +336,22 @@ class ComputeManager(manager.SchedulerDependentManager): image, but is accurate because it reflects the image's actual size. """ + # NOTE(jk0): image_ref is defined in the DB model, image_href is + # used by the image service. This should be refactored to be + # consistent. image_href = instance['image_ref'] image_service, image_id = nova.image.get_image_service(image_href) image_meta = image_service.show(context, image_id) - size_bytes = image_meta['size'] + + try: + size_bytes = image_meta['size'] + except KeyError: + # Size is not a required field in the image service (yet), so + # we are unable to rely on it being there even though it's in + # glance. + + # TODO(jk0): Should size be required in the image service? + return instance_type_id = instance['instance_type_id'] instance_type = self.db.instance_type_get(context, @@ -360,6 +372,7 @@ class ComputeManager(manager.SchedulerDependentManager): context = context.elevated() instance = self.db.instance_get(context, instance_id) + if instance['name'] in self.driver.list_instances(): raise exception.Error(_("Instance has already been created")) diff --git a/nova/tests/scheduler/test_scheduler.py b/nova/tests/scheduler/test_scheduler.py index 7a26fd1bb..ebaf89624 100644 --- a/nova/tests/scheduler/test_scheduler.py +++ b/nova/tests/scheduler/test_scheduler.py @@ -256,7 +256,9 @@ class SimpleDriverTestCase(test.TestCase): def _create_instance(self, **kwargs): """Create a test instance""" inst = {} - inst['image_id'] = 1 + # NOTE(jk0): If an integer is passed as the image_ref, the image + # service will use the default image service (in this case, the fake). + inst['image_ref'] = '1' inst['reservation_id'] = 'r-fakeres' inst['user_id'] = self.user_id inst['project_id'] = self.project_id -- cgit From b60fa0d09d02066863736a3e98f07094c4db05a6 Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Fri, 12 Aug 2011 16:18:48 -0500 Subject: Check uncompressed VHD size. --- nova/tests/xenapi/stubs.py | 4 +-- nova/virt/xenapi/fake.py | 1 + nova/virt/xenapi/vm_utils.py | 74 +++++++++++++++++++++++++++++++++++++++----- nova/virt/xenapi/vmops.py | 6 ++-- 4 files changed, 72 insertions(+), 13 deletions(-) diff --git a/nova/tests/xenapi/stubs.py b/nova/tests/xenapi/stubs.py index 0d0f84e32..a6a1febd6 100644 --- a/nova/tests/xenapi/stubs.py +++ b/nova/tests/xenapi/stubs.py @@ -28,10 +28,10 @@ from nova import utils def stubout_instance_snapshot(stubs): @classmethod - def fake_fetch_image(cls, context, session, instance_id, image, user, + def fake_fetch_image(cls, context, session, instance, image, user, project, type): from nova.virt.xenapi.fake import create_vdi - name_label = "instance-%s" % instance_id + name_label = "instance-%s" % instance.id #TODO: create fake SR record sr_ref = "fakesr" vdi_ref = create_vdi(name_label=name_label, read_only=False, diff --git a/nova/virt/xenapi/fake.py b/nova/virt/xenapi/fake.py index d5ac39473..eecd60d58 100644 --- a/nova/virt/xenapi/fake.py +++ b/nova/virt/xenapi/fake.py @@ -140,6 +140,7 @@ def create_vdi(name_label, read_only, sr_ref, sharable): 'location': '', 'xenstore_data': '', 'sm_config': {}, + 'physical_utilisation': '123', 'VBDs': {}}) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index 6c44d53d4..e86f72347 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -31,6 +31,7 @@ import uuid from xml.dom import minidom import glance.client +from nova import db from nova import exception from nova import flags import nova.image @@ -368,7 +369,7 @@ class VMHelper(HelperBase): session.wait_for_task(task, instance.id) @classmethod - def fetch_image(cls, context, session, instance_id, image, user_id, + def fetch_image(cls, context, session, instance, image, user_id, project_id, image_type): """Fetch image from glance based on image type. @@ -377,18 +378,19 @@ class VMHelper(HelperBase): """ if image_type == ImageType.DISK_VHD: return cls._fetch_image_glance_vhd(context, - session, instance_id, image, image_type) + session, instance, image, image_type) else: return cls._fetch_image_glance_disk(context, - session, instance_id, image, image_type) + session, instance, image, image_type) @classmethod - def _fetch_image_glance_vhd(cls, context, session, instance_id, image, + def _fetch_image_glance_vhd(cls, context, session, instance, image, image_type): """Tell glance to download an image and put the VHDs into the SR Returns: A list of dictionaries that describe VDIs """ + instance_id = instance.id LOG.debug(_("Asking xapi to fetch vhd image %(image)s") % locals()) sr_ref = safe_find_sr(session) @@ -422,17 +424,57 @@ class VMHelper(HelperBase): cls.scan_sr(session, instance_id, sr_ref) - # Pull out the UUID of the first VDI - vdi_uuid = vdis[0]['vdi_uuid'] + # Pull out the UUID of the first VDI (which is the os VDI) + os_vdi_uuid = vdis[0]['vdi_uuid'] + # Set the name-label to ease debugging - vdi_ref = session.get_xenapi().VDI.get_by_uuid(vdi_uuid) + vdi_ref = session.get_xenapi().VDI.get_by_uuid(os_vdi_uuid) primary_name_label = get_name_label_for_image(image) session.get_xenapi().VDI.set_name_label(vdi_ref, primary_name_label) + cls._check_vdi_size(context, session, instance, os_vdi_uuid) return vdis @classmethod - def _fetch_image_glance_disk(cls, context, session, instance_id, image, + def _get_vdi_chain_size(cls, context, session, vdi_uuid): + """Compute the total size of a VDI chain, starting with the specified + VDI UUID. + + This will walk the VDI chain to the root, add the size of each VDI into + the total. + """ + size_bytes = 0 + for vdi_rec in walk_vdi_chain(session, vdi_uuid): + vdi_size_bytes = int(vdi_rec['physical_utilisation']) + LOG.debug(_('vdi_uuid=%(vdi_uuid)s vdi_size_bytes=' + '%(vdi_size_bytes)d' % locals())) + size_bytes += vdi_size_bytes + return size_bytes + + @classmethod + def _check_vdi_size(cls, context, session, instance, vdi_uuid): + size_bytes = cls._get_vdi_chain_size(context, session, vdi_uuid) + + # FIXME(sirp): this was copied directly from compute.manager.py, let's + # refactor this to a common area + instance_type_id = instance['instance_type_id'] + instance_type = db.instance_type_get(context, + instance_type_id) + allowed_size_gb = instance_type['local_gb'] + allowed_size_bytes = allowed_size_gb * 1024 * 1024 * 1024 + + LOG.debug(_("image_size_bytes=%(size_bytes)d, allowed_size_bytes=" + "%(allowed_size_bytes)d") % locals()) + + if size_bytes > allowed_size_bytes: + LOG.info(_("Image size %(size_bytes)d exceeded" + " instance_type allowed size " + "%(allowed_size_bytes)d") + % locals()) + raise exception.ImageTooLarge() + + @classmethod + def _fetch_image_glance_disk(cls, context, session, instance, image, image_type): """Fetch the image from Glance @@ -444,6 +486,7 @@ class VMHelper(HelperBase): Returns: A single filename if image_type is KERNEL_RAMDISK A list of dictionaries that describe VDIs, otherwise """ + instance_id = instance.id # FIXME(sirp): Since the Glance plugin seems to be required for the # VHD disk, it may be worth using the plugin for both VHD and RAW and # DISK restores @@ -750,6 +793,21 @@ def get_vhd_parent_uuid(session, vdi_ref): return None +def walk_vdi_chain(session, vdi_uuid): + """Yield vdi_recs for each element in a VDI chain""" + # TODO: perhaps make get_vhd_parent use this + while True: + vdi_ref = session.get_xenapi().VDI.get_by_uuid(vdi_uuid) + vdi_rec = session.get_xenapi().VDI.get_record(vdi_ref) + yield vdi_rec + + parent_uuid = vdi_rec['sm_config'].get('vhd-parent') + if parent_uuid: + vdi_uuid = parent_uuid + else: + break + + def wait_for_vhd_coalesce(session, instance_id, sr_ref, vdi_ref, original_parent_uuid): """ Spin until the parent VHD is coalesced into its parent VHD diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index b1522729a..7586ade7b 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -137,7 +137,7 @@ class VMOps(object): def _create_disks(self, context, instance): disk_image_type = VMHelper.determine_disk_image_type(instance) vdis = VMHelper.fetch_image(context, self._session, - instance.id, instance.image_ref, + instance, instance.image_ref, instance.user_id, instance.project_id, disk_image_type) return vdis @@ -182,11 +182,11 @@ class VMOps(object): try: if instance.kernel_id: kernel = VMHelper.fetch_image(context, self._session, - instance.id, instance.kernel_id, instance.user_id, + instance, instance.kernel_id, instance.user_id, instance.project_id, ImageType.KERNEL)[0] if instance.ramdisk_id: ramdisk = VMHelper.fetch_image(context, self._session, - instance.id, instance.ramdisk_id, instance.user_id, + instance, instance.ramdisk_id, instance.user_id, instance.project_id, ImageType.RAMDISK)[0] # Create the VM ref and attach the first disk first_vdi_ref = self._session.call_xenapi('VDI.get_by_uuid', -- cgit From b238bcd9de989e7dabe6698b3de77a104d96a941 Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Fri, 12 Aug 2011 16:25:16 -0500 Subject: Updated logging. --- nova/virt/xenapi/vm_utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index e86f72347..1f5f9b416 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -445,8 +445,9 @@ class VMHelper(HelperBase): """ size_bytes = 0 for vdi_rec in walk_vdi_chain(session, vdi_uuid): + cur_vdi_uuid = vdi_rec['uuid'] vdi_size_bytes = int(vdi_rec['physical_utilisation']) - LOG.debug(_('vdi_uuid=%(vdi_uuid)s vdi_size_bytes=' + LOG.debug(_('vdi_uuid=%(cur_vdi_uuid)s vdi_size_bytes=' '%(vdi_size_bytes)d' % locals())) size_bytes += vdi_size_bytes return size_bytes -- cgit From 1a2bc77871af060069ba0de80637198be78f8169 Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Mon, 15 Aug 2011 09:47:01 -0500 Subject: Fixed merge conflict. --- nova/virt/xenapi/vm_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index bf3bf4610..b6f471c0f 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -414,7 +414,7 @@ class VMHelper(HelperBase): return vdi_ref @classmethod - def fetch_image(cls, context, session, instance_id, image, user_id, + def fetch_image(cls, context, session, instance, image, user_id, project_id, image_type): """Fetch image from glance based on image type. -- cgit From bbd577de616915025e524e330f1991f3f155388c Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Mon, 15 Aug 2011 13:35:53 -0500 Subject: Corrected names in TODO/FIXME. --- nova/virt/xenapi/vm_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index b6f471c0f..4a1f07bb1 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -501,7 +501,7 @@ class VMHelper(HelperBase): def _check_vdi_size(cls, context, session, instance, vdi_uuid): size_bytes = cls._get_vdi_chain_size(context, session, vdi_uuid) - # FIXME(sirp): this was copied directly from compute.manager.py, let's + # FIXME(jk0): this was copied directly from compute.manager.py, let's # refactor this to a common area instance_type_id = instance['instance_type_id'] instance_type = db.instance_type_get(context, @@ -853,7 +853,7 @@ def get_vhd_parent_uuid(session, vdi_ref): def walk_vdi_chain(session, vdi_uuid): """Yield vdi_recs for each element in a VDI chain""" - # TODO: perhaps make get_vhd_parent use this + # TODO(jk0): perhaps make get_vhd_parent use this while True: vdi_ref = session.get_xenapi().VDI.get_by_uuid(vdi_uuid) vdi_rec = session.get_xenapi().VDI.get_record(vdi_ref) -- cgit