From 3d6e5d82e6cf70ae6e11c66d3e8ccadaccf93987 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Tue, 8 Mar 2011 18:20:20 +0000 Subject: Removes VDIs from XenServer backend if spawn process fails before vm rec is created. Fixed pep8 errors. --- nova/virt/xenapi/vm_utils.py | 189 ++++++++++++--------- nova/virt/xenapi/vmops.py | 126 ++++++++++---- plugins/xenserver/xenapi/etc/xapi.d/plugins/glance | 15 +- 3 files changed, 214 insertions(+), 116 deletions(-) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index a6de3c9aa..0baeac8ed 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -61,17 +61,19 @@ KERNEL_DIR = '/boot/guest' class ImageType: """ Enumeration class for distinguishing different image types - 0 - kernel/ramdisk image (goes on dom0's filesystem) - 1 - disk image (local SR, partitioned by objectstore plugin) - 2 - raw disk image (local SR, NOT partitioned by plugin) - 3 - vhd disk image (local SR, NOT inspected by XS, PV assumed for + 0 - kernel image (goes on dom0's filesystem) + 1 - ramdisk image (goes on dom0's filesystem) + 2 - disk image (local SR, partitioned by objectstore plugin) + 3 - raw disk image (local SR, NOT partitioned by plugin) + 4 - vhd disk image (local SR, NOT inspected by XS, PV assumed for linux, HVM assumed for Windows) """ - KERNEL_RAMDISK = 0 - DISK = 1 - DISK_RAW = 2 - DISK_VHD = 3 + KERNEL = 0 + RAMDISK = 1 + DISK = 2 + DISK_RAW = 3 + DISK_VHD = 4 class VMHelper(HelperBase): @@ -212,6 +214,15 @@ class VMHelper(HelperBase): LOG.exception(exc) raise StorageError(_('Unable to destroy VBD %s') % vbd_ref) + @classmethod + def destroy_vdi(cls, session, vdi_ref): + try: + task = session.call_xenapi('Async.VDI.destroy', vdi_ref) + session.wait_for_task(task) + except cls.XenAPI.Failure, exc: + LOG.exception(exc) + raise StorageError(_('Unable to destroy VDI %s') % vdi_ref) + @classmethod def create_vif(cls, session, vm_ref, network_ref, mac_address, dev="0"): """Create a VIF record. Returns a Deferred that gives the new @@ -329,34 +340,42 @@ class VMHelper(HelperBase): image_type): LOG.debug(_("Asking xapi to fetch vhd image %(image)s") % locals()) - - sr_ref = safe_find_sr(session) - - # NOTE(sirp): The Glance plugin runs under Python 2.4 which does not - # have the `uuid` module. To work around this, we generate the uuids - # here (under Python 2.6+) and pass them as arguments - uuid_stack = [str(uuid.uuid4()) for i in xrange(2)] - - params = {'image_id': image, - 'glance_host': FLAGS.glance_host, - 'glance_port': FLAGS.glance_port, - 'uuid_stack': uuid_stack, - 'sr_path': get_sr_path(session)} - - kwargs = {'params': pickle.dumps(params)} - task = session.async_call_plugin('glance', 'download_vhd', kwargs) - vdi_uuid = session.wait_for_task(task, instance_id) - - scan_sr(session, instance_id, sr_ref) - - # Set the name-label to ease debugging - vdi_ref = session.get_xenapi().VDI.get_by_uuid(vdi_uuid) - name_label = get_name_label_for_image(image) - session.get_xenapi().VDI.set_name_label(vdi_ref, name_label) - - LOG.debug(_("xapi 'download_vhd' returned VDI UUID %(vdi_uuid)s") - % locals()) - return vdi_uuid + try: + sr_ref = safe_find_sr(session) + + # NOTE(sirp): The Glance plugin runs under Python 2.4 which + # does not have the `uuid` module. To work around this, we + # generate the uuids here (under Python 2.6+) and pass them + # as arguments + uuid_stack = [str(uuid.uuid4()) for i in xrange(2)] + + params = {'image_id': image, + 'glance_host': FLAGS.glance_host, + 'glance_port': FLAGS.glance_port, + 'uuid_stack': uuid_stack, + 'sr_path': get_sr_path(session)} + + kwargs = {'params': pickle.dumps(params)} + task = session.async_call_plugin('glance', 'download_vhd', kwargs) + vdi_uuid = session.wait_for_task(task, instance_id) + + scan_sr(session, instance_id, sr_ref) + + # Set the name-label to ease debugging + vdi_ref = session.get_xenapi().VDI.get_by_uuid(vdi_uuid) + name_label = get_name_label_for_image(image) + session.get_xenapi().VDI.set_name_label(vdi_ref, name_label) + + LOG.debug(_("xapi 'download_vhd' returned VDI UUID %(vdi_uuid)s") + % locals()) + return vdi_uuid + except BaseException as e: + try: + vdi_uuid = session.get_xenapi().VDI.get_uuid(vdi) + e.args = e.args + ({image_type: vdi_uuid},) + except: + pass # ignore failures in retrieving VDI + raise e @classmethod def _fetch_image_glance_disk(cls, session, instance_id, image, access, @@ -372,42 +391,53 @@ class VMHelper(HelperBase): # 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 - sr_ref = safe_find_sr(session) - - client = glance.client.Client(FLAGS.glance_host, FLAGS.glance_port) - meta, image_file = client.get_image(image) - virtual_size = int(meta['size']) - vdi_size = virtual_size - LOG.debug(_("Size for image %(image)s:%(virtual_size)d") % locals()) - - if image_type == ImageType.DISK: - # Make room for MBR. - vdi_size += MBR_SIZE_BYTES - - name_label = get_name_label_for_image(image) - vdi = cls.create_vdi(session, sr_ref, name_label, vdi_size, False) - - with_vdi_attached_here(session, vdi, False, - lambda dev: - _stream_disk(dev, image_type, - virtual_size, image_file)) - if image_type == ImageType.KERNEL_RAMDISK: - #we need to invoke a plugin for copying VDI's - #content into proper path - LOG.debug(_("Copying VDI %s to /boot/guest on dom0"), vdi) - fn = "copy_kernel_vdi" - args = {} - args['vdi-ref'] = vdi - #let the plugin copy the correct number of bytes - args['image-size'] = str(vdi_size) - task = session.async_call_plugin('glance', fn, args) - filename = session.wait_for_task(task, instance_id) - #remove the VDI as it is not needed anymore - session.get_xenapi().VDI.destroy(vdi) - LOG.debug(_("Kernel/Ramdisk VDI %s destroyed"), vdi) - return filename - else: - return session.get_xenapi().VDI.get_uuid(vdi) + LOG.debug(_("Fetching image %(image)s") % locals()) + try: + sr_ref = safe_find_sr(session) + + client = glance.client.Client(FLAGS.glance_host, FLAGS.glance_port) + meta, image_file = client.get_image(image) + virtual_size = int(meta['size']) + vdi_size = virtual_size + LOG.debug(_("Size for image %(image)s:" + + "%(virtual_size)d") % locals()) + + if image_type == ImageType.DISK: + # Make room for MBR. + vdi_size += MBR_SIZE_BYTES + + name_label = get_name_label_for_image(image) + vdi = cls.create_vdi(session, sr_ref, name_label, vdi_size, False) + + with_vdi_attached_here(session, vdi, False, + lambda dev: + _stream_disk(dev, image_type, + virtual_size, image_file)) + if image_type in (ImageType.KERNEL, ImageType.RAMDISK): + #we need to invoke a plugin for copying VDI's + #content into proper path + LOG.debug(_("Copying VDI %s to /boot/guest on dom0"), vdi) + fn = "copy_kernel_vdi" + args = {} + args['vdi-ref'] = vdi + #let the plugin copy the correct number of bytes + args['image-size'] = str(vdi_size) + task = session.async_call_plugin('glance', fn, args) + filename = session.wait_for_task(task, instance_id) + #remove the VDI as it is not needed anymore + session.get_xenapi().VDI.destroy(vdi) + LOG.debug(_("Kernel/Ramdisk VDI %s destroyed"), vdi) + return filename + else: + return session.get_xenapi().VDI.get_uuid(vdi) + except BaseException as e: + if vdi: + try: + vdi_uuid = session.get_xenapi().VDI.get_uuid(vdi) + e.args = e.args + ({image_type: vdi_uuid},) + except: + pass # ignore failures in retrieving VDI + raise e @classmethod def determine_disk_image_type(cls, instance): @@ -422,7 +452,8 @@ class VMHelper(HelperBase): whether a kernel_id is specified. """ def log_disk_format(image_type): - pretty_format = {ImageType.KERNEL_RAMDISK: 'KERNEL_RAMDISK', + pretty_format = {ImageType.KERNEL: 'KERNEL', + ImageType.RAMDISK: 'RAMDISK', ImageType.DISK: 'DISK', ImageType.DISK_RAW: 'DISK_RAW', ImageType.DISK_VHD: 'DISK_VHD'} @@ -436,8 +467,8 @@ class VMHelper(HelperBase): glance_type2nova_type = {'machine': ImageType.DISK, 'raw': ImageType.DISK_RAW, 'vhd': ImageType.DISK_VHD, - 'kernel': ImageType.KERNEL_RAMDISK, - 'ramdisk': ImageType.KERNEL_RAMDISK} + 'kernel': ImageType.KERNEL, + 'ramdisk': ImageType.RAMDISK} client = glance.client.Client(FLAGS.glance_host, FLAGS.glance_port) meta = client.get_image_meta(instance.image_id) type_ = meta['type'] @@ -478,7 +509,7 @@ class VMHelper(HelperBase): secret, image_type): url = images.image_url(image) LOG.debug(_("Asking xapi to fetch %(url)s as %(access)s") % locals()) - if image_type == ImageType.KERNEL_RAMDISK: + if image_type in (ImageType.KERNEL, ImageType.RAMDISK): fn = 'get_kernel' else: fn = 'get_vdi' @@ -488,7 +519,7 @@ class VMHelper(HelperBase): args['password'] = secret args['add_partition'] = 'false' args['raw'] = 'false' - if image_type != ImageType.KERNEL_RAMDISK: + if not image_type in (ImageType.KERNEL, ImageType.RAMDISK): args['add_partition'] = 'true' if image_type == ImageType.DISK_RAW: args['raw'] = 'true' @@ -889,8 +920,8 @@ def _write_partition(virtual_size, dev): process_input=process_input, check_exit_code=check_exit_code) - execute('parted --script %s mklabel msdos' % dest) - execute('parted --script %s mkpart primary %ds %ds' % + execute('sudo parted --script %s mklabel msdos' % dest) + execute('sudo parted --script %s mkpart primary %ds %ds' % (dest, primary_first, primary_last)) LOG.debug(_('Writing partition table %s done.'), dest) diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 9ac83efb0..69242be4c 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -25,6 +25,7 @@ import os import subprocess import tempfile import uuid +import sys from nova import db from nova import context @@ -82,36 +83,91 @@ class VMOps(object): project = AuthManager().get_project(instance.project_id) disk_image_type = VMHelper.determine_disk_image_type(instance) - - vdi_uuid = VMHelper.fetch_image(self._session, instance.id, - instance.image_id, user, project, disk_image_type) - - vdi_ref = self._session.call_xenapi('VDI.get_by_uuid', vdi_uuid) - - pv_kernel = False - if disk_image_type == ImageType.DISK_RAW: - #Have a look at the VDI and see if it has a PV kernel - pv_kernel = VMHelper.lookup_image(self._session, instance.id, - vdi_ref) - elif disk_image_type == ImageType.DISK_VHD: - # TODO(sirp): Assuming PV for now; this will need to be - # configurable as Windows will use HVM. - pv_kernel = True - + vdi_uuid = None + vdi_ref = None kernel = None - if instance.kernel_id: - kernel = VMHelper.fetch_image(self._session, instance.id, - instance.kernel_id, user, project, ImageType.KERNEL_RAMDISK) - ramdisk = None - if instance.ramdisk_id: - ramdisk = VMHelper.fetch_image(self._session, instance.id, - instance.ramdisk_id, user, project, ImageType.KERNEL_RAMDISK) + try: + vdi_uuid = VMHelper.fetch_image(self._session, instance.id, + instance.image_id, user, project, disk_image_type) + vdi_ref = self._session.call_xenapi('VDI.get_by_uuid', vdi_uuid) + + pv_kernel = False + if disk_image_type == ImageType.DISK_RAW: + #Have a look at the VDI and see if it has a PV kernel + pv_kernel = VMHelper.lookup_image(self._session, instance.id, + vdi_ref) + elif disk_image_type == ImageType.DISK_VHD: + # TODO(sirp): Assuming PV for now; this will need to be + # configurable as Windows will use HVM. + pv_kernel = True + + if instance.kernel_id: + kernel = VMHelper.fetch_image(self._session, instance.id, + instance.kernel_id, user, project, + ImageType.KERNEL) + + if instance.ramdisk_id: + ramdisk = VMHelper.fetch_image(self._session, instance.id, + instance.ramdisk_id, user, project, + ImageType.RAMDISK) + + vm_ref = VMHelper.create_vm(self._session, + instance, kernel, ramdisk, pv_kernel) + except BaseException as spawn_error: + + def _vdi_uuid(filename): + #Note: we assume the file name is the same as + #the uuid of the VDI. If this changes in + #_copy_kernel_vdi (glance plugin) + #this function must be updated accordingly + splits = filename.split('/') + n_splits = len(splits) + if n_splits > 0: + return splits[n_splits - 1] + return None + + LOG.exception(_("instance %s: Failed to spawn"), + instance.id, exc_info=sys.exc_info()) + LOG.debug(_('Instance %s failed to spawn - performing clean-up'), + instance.id) + vdis = {} + if vdi_uuid: + vdis[ImageType.DISK] = vdi_uuid + vdis[ImageType.KERNEL] = kernel and _vdi_uuid(kernel) or None + vdis[ImageType.RAMDISK] = ramdisk and _vdi_uuid(ramdisk) or None + #extract VDI uuid from spawn error + if len(spawn_error.args) > 0: + last_arg = spawn_error.args[len(spawn_error.args) - 1] + if isinstance(last_arg, dict): + for item in last_arg: + vdis[item] = last_arg[item] + LOG.debug(_("VDIS to remove:%s"), vdis) + remove_from_dom0 = False + for vdi_type in vdis: + vdi_uuid = vdis[vdi_type] + if vdi_type in (ImageType.KERNEL, ImageType.RAMDISK): + remove_from_dom0 = True + try: + vdi_ref = self._session.call_xenapi('VDI.get_by_uuid', + vdi_uuid) + LOG.debug(_('Removing VDI %(vdi_ref)s' + + '(uuid:%(vdi_uuid)s)'), + locals()) + except: + #vdi already deleted + LOG.debug(_("Skipping VDI destroy for %s"), vdi_uuid) + continue + VMHelper.destroy_vdi(self._session, vdi_ref) + if remove_from_dom0: + LOG.debug(_("Removing kernel/ramdisk files from dom0")) + self._destroy_kernel_ramdisk_plugin_call( + vdis[ImageType.KERNEL], vdis[ImageType.RAMDISK]) + + #re-throw the error + raise spawn_error - vm_ref = VMHelper.create_vm(self._session, - instance, kernel, ramdisk, pv_kernel) VMHelper.create_vbd(self._session, vm_ref, vdi_ref, 0, True) - # inject_network_info and create vifs networks = self.inject_network_info(instance) self.create_vifs(instance, networks) @@ -378,6 +434,16 @@ class VMOps(object): except self.XenAPI.Failure, exc: LOG.exception(exc) + def _destroy_kernel_ramdisk_plugin_call(self, kernel, ramdisk): + args = {} + if kernel: + args['kernel-uuid'] = kernel + if ramdisk: + args['ramdisk-uuid'] = ramdisk + task = self._session.async_call_plugin( + 'glance', 'remove_kernel_ramdisk', args) + self._session.wait_for_task(task) + def _destroy_kernel_ramdisk(self, instance, vm): """ Three situations can occur: @@ -408,13 +474,7 @@ class VMOps(object): (kernel, ramdisk) = VMHelper.lookup_kernel_ramdisk( self._session, vm) - LOG.debug(_("Removing kernel/ramdisk files")) - - args = {'kernel-file': kernel, 'ramdisk-file': ramdisk} - task = self._session.async_call_plugin( - 'glance', 'remove_kernel_ramdisk', args) - self._session.wait_for_task(task, instance.id) - + self._destroy_kernel_ramdisk_plugin_call(kernel, ramdisk) LOG.debug(_("kernel/ramdisk files removed")) def _destroy_vm(self, instance, vm): diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance b/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance index aa12d432a..4251461ae 100644 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance @@ -351,18 +351,25 @@ def copy_kernel_vdi(session, args): _copy_kernel_vdi('/dev/%s' % dev, copy_args)) return filename - + def remove_kernel_ramdisk(session, args): """Removes kernel and/or ramdisk from dom0's file system""" - kernel_file = exists(args, 'kernel-file') - ramdisk_file = exists(args, 'ramdisk-file') + kernel_file = optional(args, 'kernel-file') + ramdisk_file = optional(args, 'ramdisk-file') if kernel_file: os.remove(kernel_file) + else: + kernel_uuid = optional(args, 'kernel-uuid') + if kernel_uuid: + os.remove(KERNEL_DIR + '/' + kernel_uuid) if ramdisk_file: os.remove(ramdisk_file) + else: + ramdisk_uuid = optional(args, 'ramdisk-uuid') + if ramdisk_uuid: + os.remove(KERNEL_DIR + '/' + ramdisk_uuid) return "ok" - if __name__ == '__main__': XenAPIPlugin.dispatch({'upload_vhd': upload_vhd, 'download_vhd': download_vhd, -- cgit From 6e269b1e0b9c768b0977be140c48cc5229564177 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Wed, 9 Mar 2011 11:12:24 +0000 Subject: Added unit tests for ensuring VDI are cleaned up upon spawn failures --- nova/tests/test_xenapi.py | 30 ++++++++++++++++++++++++++++++ nova/tests/xenapi/stubs.py | 23 +++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py index 106c0bd6f..7e57dd7d4 100644 --- a/nova/tests/test_xenapi.py +++ b/nova/tests/test_xenapi.py @@ -251,6 +251,16 @@ class XenAPIVMTestCase(test.TestCase): # Check that the VM is running according to XenAPI. self.assertEquals(vm['power_state'], 'Running') + def _check_no_unbound_vdi(self): + url = FLAGS.xenapi_connection_url + username = FLAGS.xenapi_connection_username + password = FLAGS.xenapi_connection_password + session = xenapi_conn.XenAPISession(url, username, password) + vdi_refs = session.call_xenapi('VDI.get_all') + for vdi_ref in vdi_refs: + vdi_rec = session.call_xenapi('VDI.get_record', vdi_ref) + self.assertEquals(vdi_rec['VBDs'], {}) + def _test_spawn(self, image_id, kernel_id, ramdisk_id, instance_type="m1.large"): stubs.stubout_session(self.stubs, stubs.FakeSessionForVMTests) @@ -275,6 +285,26 @@ class XenAPIVMTestCase(test.TestCase): self._test_spawn, 1, 2, 3, "m1.xlarge") + def test_spawn_fail_cleanup_1(self): + """Simulates an error while downloading image + Verifies VDI create are properly cleaned up""" + FLAGS.xenapi_image_service = 'glance' + stubs.stubout_fetch_image_glance_disk(self.stubs) + self.assertRaises(Exception, + self._test_spawn, 1, 2, 3) + #ensure there is no VDI without a VBD + self._check_no_unbound_vdi() + + def test_spawn_fail_cleanup_2(self): + """Simulates an error while creating VM record. + Verifies VDI create are properly cleaned up""" + FLAGS.xenapi_image_service = 'glance' + stubs.stubout_create_vm(self.stubs) + self.assertRaises(Exception, + self._test_spawn, 1, 2, 3) + #ensure there is no VDI without a VBD + self._check_no_unbound_vdi() + def test_spawn_raw_objectstore(self): FLAGS.xenapi_image_service = 'objectstore' self._test_spawn(1, None, None) diff --git a/nova/tests/xenapi/stubs.py b/nova/tests/xenapi/stubs.py index 3de73d617..285cf2d53 100644 --- a/nova/tests/xenapi/stubs.py +++ b/nova/tests/xenapi/stubs.py @@ -136,6 +136,29 @@ def stubout_is_vdi_pv(stubs): stubs.Set(vm_utils, '_is_vdi_pv', f) +def stubout_lookup_image(stubs): + """Simulates a failure in lookup image""" + def f(_1, _2, _3, _4): + raise Exception("Test Exception raised by fake lookup_image") + stubs.Set(vm_utils, 'lookup_image', f) + + +def stubout_fetch_image_glance_disk(stubs): + """Simulates a failure in fetch image_glance_disk""" + def f(_1, _2, _3, _4, _5, _6): + raise Exception("Test Exception raised by " + + "fake fetch_image_glance_disk") + stubs.Set(vm_utils.VMHelper, '_fetch_image_glance_disk', f) + + +def stubout_create_vm(stubs): + """Simulates a failure in create_vm""" + def f(_1, _2, _3, _4, _5, _6): + raise Exception("Test Exception raised by " + + "fake create_vm") + stubs.Set(vm_utils.VMHelper, 'create_vm', f) + + class FakeSessionForVMTests(fake.SessionBase): """ Stubs out a XenAPISession for VM tests """ def __init__(self, uri): -- cgit From 748b3102320a9de3444b067aa783e8f3d7bc5f5c Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Wed, 9 Mar 2011 11:17:41 +0000 Subject: Fixed pep8 violation in glance plugin --- plugins/xenserver/xenapi/etc/xapi.d/plugins/glance | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance b/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance index 4251461ae..13f974fb9 100644 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance @@ -351,7 +351,7 @@ def copy_kernel_vdi(session, args): _copy_kernel_vdi('/dev/%s' % dev, copy_args)) return filename - + def remove_kernel_ramdisk(session, args): """Removes kernel and/or ramdisk from dom0's file system""" kernel_file = optional(args, 'kernel-file') -- cgit From 68dae00b7b1580c4b816058f748b272fe5e07c64 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Mon, 14 Mar 2011 14:49:16 +0000 Subject: Fixed failing tests in test_xenapi --- nova/virt/xenapi/vm_utils.py | 6 +++--- nova/virt/xenapi/vmops.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index 3ae91a4ef..53e402dd2 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -472,7 +472,7 @@ class VMHelper(HelperBase): lambda dev: _stream_disk(dev, image_type, virtual_size, image_file)) - if image_type == ImageType.KERNEL_RAMDISK: + if image_type in (ImageType.KERNEL,ImageType.RAMDISK): #we need to invoke a plugin for copying VDI's #content into proper path LOG.debug(_("Copying VDI %s to /boot/guest on dom0"), vdi_ref) @@ -490,9 +490,9 @@ class VMHelper(HelperBase): else: return session.get_xenapi().VDI.get_uuid(vdi_ref) except BaseException as e: - if vdi: + if vdi_ref: try: - vdi_uuid = session.get_xenapi().VDI.get_uuid(vdi) + vdi_uuid = session.get_xenapi().VDI.get_uuid(vdi_ref) e.args = e.args + ({image_type: vdi_uuid},) except: pass # ignore failures in retrieving VDI diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 16374f7c2..8489a7216 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -159,7 +159,7 @@ class VMOps(object): ramdisk = None if instance.ramdisk_id: ramdisk = VMHelper.fetch_image(self._session, instance.id, - instance.ramdisk_id, user, project, ImageType.KERNEL_RAMDISK) + instance.ramdisk_id, user, project, ImageType.RAMDISK) use_pv_kernel = VMHelper.determine_is_pv(self._session, instance.id, vdi_ref, disk_image_type, instance.os_type) -- cgit From e2aed1036c0fb61a2924ffa28d66f87539d43ba1 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Mon, 14 Mar 2011 14:52:19 +0000 Subject: Fixed pep8 errors --- nova/tests/db/fakes.py | 2 +- nova/tests/test_xenapi.py | 6 +++--- nova/virt/xenapi/vm_utils.py | 8 ++++---- nova/virt/xenapi/vmops.py | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/nova/tests/db/fakes.py b/nova/tests/db/fakes.py index 142f6b1c6..22a60e012 100644 --- a/nova/tests/db/fakes.py +++ b/nova/tests/db/fakes.py @@ -77,7 +77,7 @@ def stub_out_db_instance_api(stubs): 'mac_address': values['mac_address'], 'vcpus': type_data['vcpus'], 'local_gb': type_data['local_gb'], - 'os_type': values['os_type'] + 'os_type': values['os_type'], } return FakeModel(base_options) diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py index 60ef39cbb..7119f3078 100644 --- a/nova/tests/test_xenapi.py +++ b/nova/tests/test_xenapi.py @@ -78,7 +78,7 @@ class XenAPIVolumeTestCase(test.TestCase): 'ramdisk_id': 3, 'instance_type': 'm1.large', 'mac_address': 'aa:bb:cc:dd:ee:ff', - 'os_type': 'linux' + 'os_type': 'linux', } def _create_volume(self, size='0'): @@ -328,7 +328,7 @@ class XenAPIVMTestCase(test.TestCase): 'ramdisk_id': ramdisk_id, 'instance_type': instance_type, 'mac_address': 'aa:bb:cc:dd:ee:ff', - 'os_type': os_type + 'os_type': os_type, } conn = xenapi_conn.get_connection(False) instance = db.instance_create(values) @@ -473,7 +473,7 @@ class XenAPIMigrateInstance(test.TestCase): 'ramdisk_id': None, 'instance_type': 'm1.large', 'mac_address': 'aa:bb:cc:dd:ee:ff', - 'os_type': 'linux' + 'os_type': 'linux', } stubs.stub_out_migration_methods(self.stubs) glance_stubs.stubout_glance_client(self.stubs, diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index 53e402dd2..1dad29736 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -138,7 +138,7 @@ class VMHelper(HelperBase): 'VCPUs_at_startup': vcpus, 'VCPUs_max': vcpus, 'VCPUs_params': {}, - 'xenstore_data': {} + 'xenstore_data': {}, } # Complete VM configuration record according to the image type @@ -465,14 +465,14 @@ class VMHelper(HelperBase): vdi_size += MBR_SIZE_BYTES name_label = get_name_label_for_image(image) - vdi_ref = cls.create_vdi(session, sr_ref, name_label, + vdi_ref = cls.create_vdi(session, sr_ref, name_label, vdi_size, False) - + with_vdi_attached_here(session, vdi_ref, False, lambda dev: _stream_disk(dev, image_type, virtual_size, image_file)) - if image_type in (ImageType.KERNEL,ImageType.RAMDISK): + if image_type in (ImageType.KERNEL, ImageType.RAMDISK): #we need to invoke a plugin for copying VDI's #content into proper path LOG.debug(_("Copying VDI %s to /boot/guest on dom0"), vdi_ref) diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 8489a7216..58ffff8fb 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -164,8 +164,8 @@ class VMOps(object): use_pv_kernel = VMHelper.determine_is_pv(self._session, instance.id, vdi_ref, disk_image_type, instance.os_type) try: - vm_ref = VMHelper.create_vm(self._session, instance, kernel, ramdisk, - use_pv_kernel) + vm_ref = VMHelper.create_vm(self._session, instance, + kernel, ramdisk, use_pv_kernel) except BaseException as vm_create_error: # if the spwan process fails here it will be necessary to # clean up kernel and ramdisk (VDIs and files in dom0) -- cgit From 7649a9af1fb04886a25124b92e6ba4f88c1fd0b0 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Thu, 17 Mar 2011 11:19:03 +0000 Subject: Improved exception handling: - catching appropriate errors (OSError, IOError, XenAPI.Failure) - reduced size of try blocks - moved exception handling code in separate method - verifing for appropriate exeception type in unit tests --- nova/tests/test_xenapi.py | 4 +- nova/tests/xenapi/stubs.py | 16 +++++--- nova/virt/xenapi/vm_utils.py | 94 ++++++++++++++++++++++---------------------- nova/virt/xenapi/vmops.py | 74 +++++++++++++++++----------------- 4 files changed, 96 insertions(+), 92 deletions(-) diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py index a65aecf6b..8a8b13933 100644 --- a/nova/tests/test_xenapi.py +++ b/nova/tests/test_xenapi.py @@ -346,7 +346,7 @@ class XenAPIVMTestCase(test.TestCase): Verifies VDI create are properly cleaned up""" FLAGS.xenapi_image_service = 'glance' stubs.stubout_fetch_image_glance_disk(self.stubs) - self.assertRaises(Exception, + self.assertRaises(xenapi_fake.Failure, self._test_spawn, 1, 2, 3) #ensure there is no VDI without a VBD self._check_no_unbound_vdi() @@ -356,7 +356,7 @@ class XenAPIVMTestCase(test.TestCase): Verifies VDI create are properly cleaned up""" FLAGS.xenapi_image_service = 'glance' stubs.stubout_create_vm(self.stubs) - self.assertRaises(Exception, + self.assertRaises(xenapi_fake.Failure, self._test_spawn, 1, 2, 3) #ensure there is no VDI without a VBD self._check_no_unbound_vdi() diff --git a/nova/tests/xenapi/stubs.py b/nova/tests/xenapi/stubs.py index 6d050db04..924cecf2c 100644 --- a/nova/tests/xenapi/stubs.py +++ b/nova/tests/xenapi/stubs.py @@ -146,17 +146,21 @@ def stubout_lookup_image(stubs): def stubout_fetch_image_glance_disk(stubs): """Simulates a failure in fetch image_glance_disk""" - def f(_1, _2, _3, _4, _5, _6): - raise Exception("Test Exception raised by " + - "fake fetch_image_glance_disk") + + @classmethod + def f(cls, *args): + raise fake.Failure("Test Exception raised by " + + "fake fetch_image_glance_disk") stubs.Set(vm_utils.VMHelper, '_fetch_image_glance_disk', f) def stubout_create_vm(stubs): """Simulates a failure in create_vm""" - def f(_1, _2, _3, _4, _5, _6): - raise Exception("Test Exception raised by " + - "fake create_vm") + + @classmethod + def f(cls, *args): + raise fake.Failure("Test Exception raised by " + + "fake create_vm") stubs.Set(vm_utils.VMHelper, 'create_vm', f) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index 211166db2..a7337e38b 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -397,24 +397,26 @@ class VMHelper(HelperBase): image_type): LOG.debug(_("Asking xapi to fetch vhd image %(image)s") % locals()) - try: - sr_ref = safe_find_sr(session) + sr_ref = safe_find_sr(session) - # NOTE(sirp): The Glance plugin runs under Python 2.4 - # which does not have the `uuid` module. To work around this, - # we generate the uuids here (under Python 2.6+) and - # pass them as arguments - uuid_stack = [str(uuid.uuid4()) for i in xrange(2)] + # NOTE(sirp): The Glance plugin runs under Python 2.4 + # which does not have the `uuid` module. To work around this, + # we generate the uuids here (under Python 2.6+) and + # pass them as arguments + uuid_stack = [str(uuid.uuid4()) for i in xrange(2)] - params = {'image_id': image, - 'glance_host': FLAGS.glance_host, - 'glance_port': FLAGS.glance_port, - 'uuid_stack': uuid_stack, - 'sr_path': cls.get_sr_path(session)} + params = {'image_id': image, + 'glance_host': FLAGS.glance_host, + 'glance_port': FLAGS.glance_port, + 'uuid_stack': uuid_stack, + 'sr_path': cls.get_sr_path(session)} - kwargs = {'params': pickle.dumps(params)} - task = session.async_call_plugin('glance', 'download_vhd', kwargs) - vdi_uuid = session.wait_for_task(task, instance_id) + kwargs = {'params': pickle.dumps(params)} + task = session.async_call_plugin('glance', 'download_vhd', kwargs) + vdi_uuid = session.wait_for_task(task, instance_id) + #from this point we have a VDI on Xen host + #if anything goes wrong, we need to remember its uuid + try: cls.scan_sr(session, instance_id, sr_ref) @@ -426,14 +428,11 @@ class VMHelper(HelperBase): LOG.debug(_("xapi 'download_vhd' returned VDI UUID %(vdi_uuid)s") % locals()) return vdi_uuid - except BaseException as e: + except cls.XenAPI.Failure as e: + #Looking for XenAPI failures only LOG.exception(_("instance %s: Failed to fetch glance image"), instance_id, exc_info=sys.exc_info()) - try: - vdi_uuid = session.get_xenapi().VDI.get_uuid(vdi) - e.args = e.args + ({image_type: vdi_uuid},) - except: - pass # ignore failures in retrieving VDI + e.args = e.args + ({image_type: vdi_uuid},) raise e @classmethod @@ -451,24 +450,25 @@ class VMHelper(HelperBase): # VHD disk, it may be worth using the plugin for both VHD and RAW and # DISK restores LOG.debug(_("Fetching image %(image)s") % locals()) - try: - sr_ref = safe_find_sr(session) - - client = glance.client.Client(FLAGS.glance_host, FLAGS.glance_port) - meta, image_file = client.get_image(image) - virtual_size = int(meta['size']) - vdi_size = virtual_size - LOG.debug(_("Size for image %(image)s:" + - "%(virtual_size)d") % locals()) - - if image_type == ImageType.DISK: - # Make room for MBR. - vdi_size += MBR_SIZE_BYTES - - name_label = get_name_label_for_image(image) - vdi_ref = cls.create_vdi(session, sr_ref, name_label, - vdi_size, False) + sr_ref = safe_find_sr(session) + client = glance.client.Client(FLAGS.glance_host, FLAGS.glance_port) + meta, image_file = client.get_image(image) + virtual_size = int(meta['size']) + vdi_size = virtual_size + LOG.debug(_("Size for image %(image)s:" + + "%(virtual_size)d") % locals()) + + if image_type == ImageType.DISK: + # Make room for MBR. + vdi_size += MBR_SIZE_BYTES + + name_label = get_name_label_for_image(image) + vdi_ref = cls.create_vdi(session, sr_ref, name_label, + vdi_size, False) + #from this point we have a VDI on Xen host + #if anything goes wrong, we need to remember its uuid + try: with_vdi_attached_here(session, vdi_ref, False, lambda dev: _stream_disk(dev, image_type, @@ -490,24 +490,22 @@ class VMHelper(HelperBase): return filename else: return session.get_xenapi().VDI.get_uuid(vdi_ref) - except BaseException as e: + except (cls.XenAPI.Failure, IOError, OSError) as e: + #Looking for XenAPI and OS failures LOG.exception(_("instance %s: Failed to fetch glance image"), instance_id, exc_info=sys.exc_info()) if vdi_ref: try: vdi_uuid = session.get_xenapi().VDI.get_uuid(vdi_ref) e.args = e.args + ({image_type: vdi_uuid},) - except: + except cls.XenAPI.Failure: pass # ignore failures in retrieving VDI if filename: - try: - splits = filename.split("/") - if len(splits) > 0: - vdi_uuid = splits[len(splits) - 1] - e.args = e.args + ({image_type: vdi_uuid},) - except: - pass # ignore errors parsing file name - + splits = filename.split("/") + #split always return at least the original string + if splits[len(splits) - 1] != None: + vdi_uuid = splits[len(splits) - 1] + e.args = e.args + ({image_type: vdi_uuid},) raise e @classmethod diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 1c48f1bd7..db76421db 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -87,46 +87,13 @@ class VMOps(object): vdi_uuid = disk_image_type = None (vdi_uuid, disk_image_type) = self.create_disk(instance) self._spawn_with_disk(instance, vdi_uuid=vdi_uuid) - except BaseException as spawn_error: + except (self.XenAPI.Failure, OSError, IOError) as spawn_error: LOG.exception(_("instance %s: Failed to spawn"), instance.id, exc_info=sys.exc_info()) LOG.debug(_('Instance %s failed to spawn - performing clean-up'), instance.id) - vdis = { - ImageType.KERNEL: None, - ImageType.RAMDISK: None, - } - if vdi_uuid: - vdis[disk_image_type] = vdi_uuid - #extract VDI uuid from spawn error - if len(spawn_error.args) > 0: - last_arg = spawn_error.args[len(spawn_error.args) - 1] - if isinstance(last_arg, dict): - for item in last_arg: - vdis[item] = last_arg[item] - LOG.debug(_("VDIS to remove:%s"), vdis) - remove_from_dom0 = False - for vdi_type in vdis: - vdi_to_remove = vdis[vdi_type] - if vdi_type in (ImageType.KERNEL, ImageType.RAMDISK): - remove_from_dom0 = True - try: - vdi_ref = self._session.call_xenapi('VDI.get_by_uuid', - vdi_to_remove) - LOG.debug(_('Removing VDI %(vdi_ref)s' + - '(uuid:%(vdi_to_remove)s)'), locals()) - except: - #vdi already deleted - LOG.debug(_("Skipping VDI destroy for %s"), vdi_to_remove) - continue - VMHelper.destroy_vdi(self._session, vdi_ref) - if remove_from_dom0: - LOG.debug(_("Removing kernel/ramdisk files from dom0")) - self._destroy_kernel_ramdisk_plugin_call( - vdis[ImageType.KERNEL], vdis[ImageType.RAMDISK], - False) - + self._handle_spawn_error(vdi_uuid, disk_image_type, spawn_error) #re-throw the error raise spawn_error @@ -170,7 +137,7 @@ class VMOps(object): try: vm_ref = VMHelper.create_vm(self._session, instance, kernel, ramdisk, use_pv_kernel) - except BaseException as vm_create_error: + except self.XenAPI.Failure as vm_create_error: # if the spwan process fails here it will be necessary to # clean up kernel and ramdisk (VDIs and files in dom0) @@ -253,6 +220,41 @@ class VMOps(object): return timer.start(interval=0.5, now=True) + def _handle_spawn_error(self, vdi_uuid, disk_image_type, spawn_error): + vdis = { + ImageType.KERNEL: None, + ImageType.RAMDISK: None, + } + if vdi_uuid: + vdis[disk_image_type] = vdi_uuid + #extract VDI uuid from spawn error + if len(spawn_error.args) > 0: + last_arg = spawn_error.args[len(spawn_error.args) - 1] + if isinstance(last_arg, dict): + for item in last_arg: + vdis[item] = last_arg[item] + LOG.debug(_("VDIS to remove:%s"), vdis) + remove_from_dom0 = False + for vdi_type in vdis: + vdi_to_remove = vdis[vdi_type] + if vdi_type in (ImageType.KERNEL, ImageType.RAMDISK): + remove_from_dom0 = True + try: + vdi_ref = self._session.call_xenapi('VDI.get_by_uuid', + vdi_to_remove) + LOG.debug(_('Removing VDI %(vdi_ref)s' + + '(uuid:%(vdi_to_remove)s)'), locals()) + except: + #vdi already deleted + LOG.debug(_("Skipping VDI destroy for %s"), vdi_to_remove) + continue + VMHelper.destroy_vdi(self._session, vdi_ref) + if remove_from_dom0: + LOG.debug(_("Removing kernel/ramdisk files from dom0")) + self._destroy_kernel_ramdisk_plugin_call( + vdis[ImageType.KERNEL], vdis[ImageType.RAMDISK], + False) + def _get_vm_opaque_ref(self, instance_or_vm): """Refactored out the common code of many methods that receive either a vm name or a vm instance, and want a vm instance in return. -- cgit From 0174d584708f2e9b7fa02a27eeb707892eb213ec Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Fri, 18 Mar 2011 08:31:21 +0000 Subject: Ensuring kernel/ramdisk files are always removed in case of failures --- nova/virt/xenapi/vm_utils.py | 2 +- nova/virt/xenapi/vmops.py | 30 ++++++++++++++---------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index dd62c61f6..0610d5c11 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -469,8 +469,8 @@ class VMHelper(HelperBase): #from this point we have a VDI on Xen host #if anything goes wrong, we need to remember its uuid try: - vdi_uuid = session.get_xenapi().VDI.get_uuid(vdi_ref) filename = None + vdi_uuid = session.get_xenapi().VDI.get_uuid(vdi_ref) with_vdi_attached_here(session, vdi_ref, False, lambda dev: _stream_disk(dev, image_type, diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 0d16bdc6a..e4701762f 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -136,30 +136,29 @@ class VMOps(object): ramdisk = VMHelper.fetch_image(self._session, instance.id, instance.ramdisk_id, user, project, ImageType.RAMDISK) - use_pv_kernel = VMHelper.determine_is_pv(self._session, instance.id, vdi_ref, disk_image_type, instance.os_type) - vm_ref = VMHelper.create_vm(self._session, instance, kernel, ramdisk, use_pv_kernel) - except self.XenAPI.Failure as vm_create_error: - # if the spawn process fails here it will be necessary to - # clean up kernel and ramdisk (only files in dom0) + except (self.XenAPI.Failure, OSError, IOError) as vm_create_error: + #collect resources to clean up + #_handle_spawn_error will remove unused resources LOG.exception(_("instance %s: Failed to spawn - " + "Unable to create VM"), instance.id, exc_info=sys.exc_info()) - last_arg = None resources = {} + if len(vm_create_error.args) > 0: last_arg = vm_create_error.args[len(vm_create_error.args) - 1] if isinstance(last_arg, dict): resources = last_arg + else: + vm_create_error.args = vm_create_error.args + (resources,) if ImageType.KERNEL not in resources and kernel: resources[ImageType.KERNEL] = (None, kernel) if ImageType.RAMDISK not in resources and ramdisk: resources[ImageType.RAMDISK] = (None, ramdisk) - vm_create_error.args = vm_create_error.args + (resources,) raise vm_create_error VMHelper.create_vbd(session=self._session, vm_ref=vm_ref, @@ -224,16 +223,15 @@ class VMOps(object): def _handle_spawn_error(self, vdi_uuid, disk_image_type, spawn_error): resources = {} - if vdi_uuid: - resources[disk_image_type] = (vdi_uuid,) + files_to_remove = {} #extract resource dictionary from spawn error if len(spawn_error.args) > 0: last_arg = spawn_error.args[len(spawn_error.args) - 1] - if isinstance(last_arg, dict): - for item in last_arg: - resources[item] = last_arg[item] - LOG.debug(_("resources to remove:%s"), resources) - files_to_remove = {} + resources = last_arg + if vdi_uuid: + resources[disk_image_type] = (vdi_uuid,) + LOG.debug(_("Resources to remove:%s"), resources) + for vdi_type in resources: items_to_remove = resources[vdi_type] vdi_to_remove = items_to_remove[0] @@ -243,14 +241,14 @@ class VMOps(object): vdi_to_remove) LOG.debug(_('Removing VDI %(vdi_ref)s' + '(uuid:%(vdi_to_remove)s)'), locals()) + VMHelper.destroy_vdi(self._session, vdi_ref) except self.XenAPI.Failure: #vdi already deleted LOG.debug(_("Skipping VDI destroy for %s"), vdi_to_remove) - continue - VMHelper.destroy_vdi(self._session, vdi_ref) if len(items_to_remove) > 1: # there is also a file to remove files_to_remove[vdi_type] = items_to_remove[1] + if len(files_to_remove) > 0: LOG.debug(_("Removing kernel/ramdisk files from dom0")) self._destroy_kernel_ramdisk_plugin_call( -- cgit From 381354babab58688458b2a6cdc463eb0a0fee461 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Mon, 28 Mar 2011 13:11:45 +0100 Subject: addressed reviewers' concerns --- nova/tests/test_xenapi.py | 35 ++++++++++++---------- nova/tests/xenapi/stubs.py | 6 ++-- nova/virt/xenapi/vm_utils.py | 18 +++++++---- nova/virt/xenapi_conn.py | 7 ++--- plugins/xenserver/xenapi/etc/xapi.d/plugins/glance | 1 + 5 files changed, 38 insertions(+), 29 deletions(-) diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py index 1c5a81f0d..feaced8ac 100644 --- a/nova/tests/test_xenapi.py +++ b/nova/tests/test_xenapi.py @@ -343,8 +343,10 @@ class XenAPIVMTestCase(test.TestCase): 1, 2, 3, "m1.xlarge") def test_spawn_fail_cleanup_1(self): - """Simulates an error while downloading image - Verifies VDI create are properly cleaned up""" + """ + Simulates an error while downloading image + Verifies VDI create are properly cleaned up. + """ FLAGS.xenapi_image_service = 'glance' stubs.stubout_fetch_image_glance_disk(self.stubs) self.assertRaises(xenapi_fake.Failure, @@ -353,8 +355,10 @@ class XenAPIVMTestCase(test.TestCase): self._check_no_unbound_vdi() def test_spawn_fail_cleanup_2(self): - """Simulates an error while creating VM record. - Verifies VDI create are properly cleaned up""" + """ + Simulates an error while creating VM record. It + verifies that VDI created are properly cleaned up. + """ FLAGS.xenapi_image_service = 'glance' stubs.stubout_create_vm(self.stubs) self.assertRaises(xenapi_fake.Failure, @@ -423,18 +427,17 @@ class XenAPIVMTestCase(test.TestCase): self.stubs.UnsetAll() def _create_instance(self): - """Creates and spawns a test instance""" - values = { - 'name': 1, - 'id': 1, - 'project_id': self.project.id, - 'user_id': self.user.id, - 'image_id': 1, - 'kernel_id': 2, - 'ramdisk_id': 3, - 'instance_type': 'm1.large', - 'mac_address': 'aa:bb:cc:dd:ee:ff', - 'os_type': 'linux'} + """Creates and spawns a test instance.""" + values = {'name': 1, + 'id': 1, + 'project_id': self.project.id, + 'user_id': self.user.id, + 'image_id': 1, + 'kernel_id': 2, + 'ramdisk_id': 3, + 'instance_type': 'm1.large', + 'mac_address': 'aa:bb:cc:dd:ee:ff', + 'os_type': 'linux'} instance = db.instance_create(values) self.conn.spawn(instance) return instance diff --git a/nova/tests/xenapi/stubs.py b/nova/tests/xenapi/stubs.py index 709a8b6ff..b745205f5 100644 --- a/nova/tests/xenapi/stubs.py +++ b/nova/tests/xenapi/stubs.py @@ -138,14 +138,14 @@ def stubout_is_vdi_pv(stubs): def stubout_lookup_image(stubs): - """Simulates a failure in lookup image""" + """Simulates a failure in lookup image.""" def f(_1, _2, _3, _4): raise Exception("Test Exception raised by fake lookup_image") stubs.Set(vm_utils, 'lookup_image', f) def stubout_fetch_image_glance_disk(stubs): - """Simulates a failure in fetch image_glance_disk""" + """Simulates a failure in fetch image_glance_disk.""" @classmethod def f(cls, *args): @@ -155,7 +155,7 @@ def stubout_fetch_image_glance_disk(stubs): def stubout_create_vm(stubs): - """Simulates a failure in create_vm""" + """Simulates a failure in create_vm.""" @classmethod def f(cls, *args): diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index 8abd19c3c..bcf2a9962 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -466,17 +466,23 @@ class VMHelper(HelperBase): vdi_size += MBR_SIZE_BYTES name_label = get_name_label_for_image(image) - vdi_ref = cls.create_vdi(session, sr_ref, name_label, - vdi_size, False) + vdi_ref = cls.create_vdi(session, + sr_ref, + name_label, + vdi_size, + False) # from this point we have a VDI on Xen host # if anything goes wrong, we need to remember its uuid try: filename = None vdi_uuid = session.get_xenapi().VDI.get_uuid(vdi_ref) - with_vdi_attached_here(session, vdi_ref, False, - lambda dev: - _stream_disk(dev, image_type, - virtual_size, image_file)) + with_vdi_attached_here(session, + vdi_ref, + False, + lambda dev: _stream_disk(dev, + image_type, + virtual_size, + image_file)) if image_type in (ImageType.KERNEL, ImageType.RAMDISK): # we need to invoke a plugin for copying VDI's # content into proper path diff --git a/nova/virt/xenapi_conn.py b/nova/virt/xenapi_conn.py index c7e94c508..191498916 100644 --- a/nova/virt/xenapi_conn.py +++ b/nova/virt/xenapi_conn.py @@ -367,10 +367,9 @@ class XenAPISession(object): try: name = self._session.xenapi.task.get_name_label(task) status = self._session.xenapi.task.get_status(task) - action = dict( - action=name[0:255], # Ensure action is never > 255 - instance_id=id and int(id) or None, - error=None) + action = dict(action=name[0:255], + instance_id=id and int(id) or None, + error=None) if status == "pending": return elif status == "success": diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance b/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance index b12175845..1eed7e862 100644 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance @@ -374,6 +374,7 @@ def remove_kernel_ramdisk(session, args): os.remove(ramdisk_file) return "ok" + if __name__ == '__main__': XenAPIPlugin.dispatch({'upload_vhd': upload_vhd, 'download_vhd': download_vhd, -- cgit From e722689c7b82bbf895213c2ebf9afaf34f292662 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Wed, 15 Jun 2011 12:31:20 +0100 Subject: Fixing code to ensure unit tests for objectstore, vhd & snapshots pass --- nova/tests/test_xenapi.py | 12 ++++++++++++ nova/tests/xenapi/stubs.py | 2 +- nova/virt/xenapi/vm_utils.py | 19 +++++++++++++------ nova/virt/xenapi/vmops.py | 6 ++++-- 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py index 7b6ceeb37..cddade0ce 100644 --- a/nova/tests/test_xenapi.py +++ b/nova/tests/test_xenapi.py @@ -350,6 +350,18 @@ class XenAPIVMTestCase(test.TestCase): self.assertEquals(self.vm['HVM_boot_params'], {}) self.assertEquals(self.vm['HVM_boot_policy'], '') + def _list_vdis(self): + url = FLAGS.xenapi_connection_url + username = FLAGS.xenapi_connection_username + password = FLAGS.xenapi_connection_password + session = xenapi_conn.XenAPISession(url, username, password) + return session.call_xenapi('VDI.get_all') + + def _check_vdis(self, start_list, end_list): + for vdi_ref in end_list: + if not vdi_ref in start_list: + self.fail('Found unexpected VDI:%s' % vdi_ref) + def _test_spawn(self, image_ref, kernel_id, ramdisk_id, instance_type_id="3", os_type="linux", instance_id=1, check_injection=False): diff --git a/nova/tests/xenapi/stubs.py b/nova/tests/xenapi/stubs.py index 0ecf83187..91731ad01 100644 --- a/nova/tests/xenapi/stubs.py +++ b/nova/tests/xenapi/stubs.py @@ -151,7 +151,7 @@ class FakeSessionForVMTests(fake.SessionBase): def host_call_plugin(self, _1, _2, plugin, method, _5): # copy_kernel_vdi returns nothing - if fn == 'copy_kernel_vdi': + if method == 'copy_kernel_vdi': return sr_ref = fake.get_all('SR')[0] vdi_ref = fake.create_vdi('', False, sr_ref, False) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index 43621cf2f..322f258b4 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -503,6 +503,7 @@ class VMHelper(HelperBase): # VHD disk, it may be worth using the plugin for both VHD and RAW and # DISK restores LOG.debug(_("Fetching image %(image)s") % locals()) + LOG.debug(_("Image Type: %s"), ImageType.pretty_format(image_type)) sr_ref = safe_find_sr(session) glance_client, image_id = nova.image.get_glance_client(image) @@ -512,7 +513,6 @@ class VMHelper(HelperBase): #vdi_type = 'file' # will be set to os if image_type==ImageType.DISK LOG.debug(_("Size for image %(image)s:" + "%(virtual_size)d") % locals()) - if image_type == ImageType.DISK: # Make room for MBR. vdi_size += MBR_SIZE_BYTES @@ -531,6 +531,7 @@ class VMHelper(HelperBase): try: filename = None vdi_uuid = session.get_xenapi().VDI.get_uuid(vdi_ref) + LOG.debug("HERE-1") with_vdi_attached_here(session, vdi_ref, False, lambda dev: _stream_disk(dev, image_type, @@ -538,6 +539,7 @@ class VMHelper(HelperBase): if image_type in (ImageType.KERNEL, ImageType.RAMDISK): #we need to invoke a plugin for copying VDI's #content into proper path + LOG.debug("HERE-2") LOG.debug(_("Copying VDI %s to /boot/guest on dom0"), vdi_ref) fn = "copy_kernel_vdi" args = {} @@ -563,7 +565,7 @@ class VMHelper(HelperBase): e.args = e.args + ([dict(vdi_type=ImageType. pretty_format(image_type), vdi_uuid=vdi_uuid, - file=filename)],) + file=filename)],) raise e @classmethod @@ -663,10 +665,15 @@ class VMHelper(HelperBase): if image_type == ImageType.DISK_RAW: args['raw'] = 'true' task = session.async_call_plugin('objectstore', fn, args) - uuid_or_fn = session.wait_for_task(task, instance_id) - if not image_type in (ImageType.KERNEL, ImageType.RAMDISK): - return [dict(vdi_type='os', vdi_uuid=uuid_or_fn)] - return uuid_or_fn + vdi_uuid = None + filename = None + if image_type in (ImageType.KERNEL, ImageType.RAMDISK): + filename = session.wait_for_task(task, instance_id) + else: + vdi_uuid = session.wait_for_task(task, instance_id) + return [dict(vdi_type=ImageType.pretty_format(image_type), + vdi_uuid=vdi_uuid, + file=filename)] @classmethod def determine_is_pv(cls, session, instance_id, vdi_ref, disk_image_type, diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index c198bee4a..e3e32a266 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -166,7 +166,6 @@ class VMOps(object): ramdisk = VMHelper.fetch_image(self._session, instance.id, instance.ramdisk_id, user, project, ImageType.RAMDISK)[0] - # Create the VM ref and attach the first disk first_vdi_ref = self._session.call_xenapi('VDI.get_by_uuid', vdis[0]['vdi_uuid']) @@ -174,7 +173,9 @@ class VMOps(object): instance.id, first_vdi_ref, disk_image_type, instance.os_type) vm_ref = VMHelper.create_vm(self._session, instance, - kernel['file'], ramdisk['file'], use_pv_kernel) + kernel and kernel.get('file', None) or None, + ramdisk and ramdisk.get('file', None) or None, + use_pv_kernel) except (self.XenAPI.Failure, OSError, IOError) as vm_create_error: # collect resources to clean up # _handle_spawn_error will remove unused resources @@ -288,6 +289,7 @@ class VMOps(object): def _handle_spawn_error(self, vdis, spawn_error): # extract resource dictionary from spawn error + resources = [] if spawn_error.args: last_arg = spawn_error.args[-1] resources = last_arg -- cgit From 7ca20797496947c0bdd60e77b4962fd360e01f55 Mon Sep 17 00:00:00 2001 From: Ed Leafe Date: Fri, 1 Jul 2011 13:44:12 +0000 Subject: after trunk merge --- nova/api/openstack/auth.py | 1 + nova/api/openstack/wsgi.py | 1 + nova/compute/api.py | 10 ++++++++++ nova/compute/manager.py | 10 ++++++++++ nova/scheduler/api.py | 5 +++++ nova/scheduler/manager.py | 4 ++++ nova/scheduler/zone_manager.py | 15 ++++++++++++++- 7 files changed, 45 insertions(+), 1 deletion(-) diff --git a/nova/api/openstack/auth.py b/nova/api/openstack/auth.py index 7c3e683d6..6231216c9 100644 --- a/nova/api/openstack/auth.py +++ b/nova/api/openstack/auth.py @@ -100,6 +100,7 @@ class AuthMiddleware(wsgi.Middleware): token, user = self._authorize_user(username, key, req) if user and token: + print "TOKEN:", token['token_hash'] res = webob.Response() res.headers['X-Auth-Token'] = token['token_hash'] res.headers['X-Server-Management-Url'] = \ diff --git a/nova/api/openstack/wsgi.py b/nova/api/openstack/wsgi.py index 5b6e3cb1d..ba8ee8bfd 100644 --- a/nova/api/openstack/wsgi.py +++ b/nova/api/openstack/wsgi.py @@ -362,6 +362,7 @@ class Resource(wsgi.Application): "url": request.url}) try: + print "BODY: >%s<"%request.body action, action_args, accept = self.deserializer.deserialize( request) except exception.InvalidContentType: diff --git a/nova/compute/api.py b/nova/compute/api.py index 28459dc75..4dba6cf1f 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -912,6 +912,16 @@ class API(base.Base): """Unpause the given instance.""" self._cast_compute_message('unpause_instance', context, instance_id) + def disable_host(self, context, instance_id=None, host=None): + """Sets the specified to not receive new instances.""" + return self._call_compute_message("disable_host", context, + instance_id=None, host=host) + + def enable_host(self, context, instance_id=None, host=None): + """Sets the specified to receive new instances.""" + return self._call_compute_message("enable_host", context, + instance_id=None, host=host) + @scheduler_api.reroute_compute("diagnostics") def get_diagnostics(self, context, instance_id): """Retrieve diagnostics for the given instance.""" diff --git a/nova/compute/manager.py b/nova/compute/manager.py index bbbddde0a..152b2670c 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -874,6 +874,16 @@ class ComputeManager(manager.SchedulerDependentManager): instance_id, result)) + @exception.wrap_exception + def disable_host(self, context, instance_id=None, host=None): + """Set a host so that it can not accept new instances.""" + return self.driver.disable_host(host) + + @exception.wrap_exception + def enable_host(self, context, instance_id=None, host=None): + """Set a host so that it can accept new instances.""" + return self.driver.enable_host(host) + @exception.wrap_exception def get_diagnostics(self, context, instance_id): """Retrieve diagnostics for an instance on this host.""" diff --git a/nova/scheduler/api.py b/nova/scheduler/api.py index 0f4fc48c8..137b671c0 100644 --- a/nova/scheduler/api.py +++ b/nova/scheduler/api.py @@ -51,6 +51,11 @@ def _call_scheduler(method, context, params=None): return rpc.call(context, queue, kwargs) +def get_host_list(context): + """Return a list of hosts associated with this zone.""" + return _call_scheduler('get_host_list', context) + + def get_zone_list(context): """Return a list of zones assoicated with this zone.""" items = _call_scheduler('get_zone_list', context) diff --git a/nova/scheduler/manager.py b/nova/scheduler/manager.py index 6cb75aa8d..749d66cad 100644 --- a/nova/scheduler/manager.py +++ b/nova/scheduler/manager.py @@ -56,6 +56,10 @@ class SchedulerManager(manager.Manager): """Poll child zones periodically to get status.""" self.zone_manager.ping(context) + def get_host_list(self, context=None): + """Get a list of hosts from the ZoneManager.""" + return self.zone_manager.get_host_list() + def get_zone_list(self, context=None): """Get a list of zones from the ZoneManager.""" return self.zone_manager.get_zone_list() diff --git a/nova/scheduler/zone_manager.py b/nova/scheduler/zone_manager.py index ba7403c15..169a96989 100644 --- a/nova/scheduler/zone_manager.py +++ b/nova/scheduler/zone_manager.py @@ -115,6 +115,17 @@ class ZoneManager(object): """Return the list of zones we know about.""" return [zone.to_dict() for zone in self.zone_states.values()] + def get_host_list(self): + """Returns a list of all the host names that the Zone Manager + knows about. + """ + all_hosts = self.service_states.keys() + ret = [] + for host in self.service_states: + for svc in self.service_states[host]: + ret.append({"service": svc, "host_name": host}) + return ret + def get_zone_capabilities(self, context): """Roll up all the individual host info to generic 'service' capabilities. Each capability is aggregated into @@ -127,13 +138,15 @@ class ZoneManager(object): combined = {} # { _ : (min, max), ... } for host, host_dict in hosts_dict.iteritems(): for service_name, service_dict in host_dict.iteritems(): + if not service_dict.get("enabled", True): + # Service is disabled; do no include it + continue for cap, value in service_dict.iteritems(): key = "%s_%s" % (service_name, cap) min_value, max_value = combined.get(key, (value, value)) min_value = min(min_value, value) max_value = max(max_value, value) combined[key] = (min_value, max_value) - return combined def _refresh_from_db(self, context): -- cgit From fb6aba61ef03032c31196bd58c68fa7b7d4c2769 Mon Sep 17 00:00:00 2001 From: Ed Leafe Date: Fri, 1 Jul 2011 14:26:05 +0000 Subject: completed api changes. still need plugin changes --- nova/compute/api.py | 13 ++++--------- nova/compute/manager.py | 12 ++++-------- nova/virt/driver.py | 4 ++++ nova/virt/fake.py | 4 ++++ nova/virt/hyperv.py | 4 ++++ nova/virt/libvirt/connection.py | 4 ++++ nova/virt/vmwareapi_conn.py | 4 ++++ nova/virt/xenapi/vmops.py | 18 ++++++++++++++++++ nova/virt/xenapi_conn.py | 4 ++++ 9 files changed, 50 insertions(+), 17 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 4dba6cf1f..c99b64c50 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -912,15 +912,10 @@ class API(base.Base): """Unpause the given instance.""" self._cast_compute_message('unpause_instance', context, instance_id) - def disable_host(self, context, instance_id=None, host=None): - """Sets the specified to not receive new instances.""" - return self._call_compute_message("disable_host", context, - instance_id=None, host=host) - - def enable_host(self, context, instance_id=None, host=None): - """Sets the specified to receive new instances.""" - return self._call_compute_message("enable_host", context, - instance_id=None, host=host) + def set_host_enabled(self, context, host, enabled): + """Sets the specified host's ability to accept new instances.""" + return self._call_compute_message("set_host_enabled", context, + instance_id=None, host=host, enabled=enabled) @scheduler_api.reroute_compute("diagnostics") def get_diagnostics(self, context, instance_id): diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 152b2670c..91a604934 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -875,14 +875,10 @@ class ComputeManager(manager.SchedulerDependentManager): result)) @exception.wrap_exception - def disable_host(self, context, instance_id=None, host=None): - """Set a host so that it can not accept new instances.""" - return self.driver.disable_host(host) - - @exception.wrap_exception - def enable_host(self, context, instance_id=None, host=None): - """Set a host so that it can accept new instances.""" - return self.driver.enable_host(host) + def set_host_enabled(self, context, instance_id=None, host=None, + enabled=None): + """Sets the specified host's ability to accept new instances.""" + return self.driver.set_host_enabled(host, enabled) @exception.wrap_exception def get_diagnostics(self, context, instance_id): diff --git a/nova/virt/driver.py b/nova/virt/driver.py index 1c9797973..3c4a073bf 100644 --- a/nova/virt/driver.py +++ b/nova/virt/driver.py @@ -249,3 +249,7 @@ class ComputeDriver(object): def poll_rescued_instances(self, timeout): """Poll for rescued instances""" raise NotImplementedError() + + def set_host_enabled(self, host, enabled): + """Sets the specified host's ability to accept new instances.""" + raise NotImplementedError() diff --git a/nova/virt/fake.py b/nova/virt/fake.py index 5fe9d674f..ea0a59f21 100644 --- a/nova/virt/fake.py +++ b/nova/virt/fake.py @@ -514,3 +514,7 @@ class FakeConnection(driver.ComputeDriver): def get_host_stats(self, refresh=False): """Return fake Host Status of ram, disk, network.""" return self.host_status + + def set_host_enabled(self, host, enabled): + """Sets the specified host's ability to accept new instances.""" + pass diff --git a/nova/virt/hyperv.py b/nova/virt/hyperv.py index f6783f3aa..5c1dc772d 100644 --- a/nova/virt/hyperv.py +++ b/nova/virt/hyperv.py @@ -499,3 +499,7 @@ class HyperVConnection(driver.ComputeDriver): def get_host_stats(self, refresh=False): """See xenapi_conn.py implementation.""" pass + + def set_host_enabled(self, host, enabled): + """Sets the specified host's ability to accept new instances.""" + pass diff --git a/nova/virt/libvirt/connection.py b/nova/virt/libvirt/connection.py index 0c6eaab84..b80a3daee 100644 --- a/nova/virt/libvirt/connection.py +++ b/nova/virt/libvirt/connection.py @@ -1591,3 +1591,7 @@ class LibvirtConnection(driver.ComputeDriver): def get_host_stats(self, refresh=False): """See xenapi_conn.py implementation.""" pass + + def set_host_enabled(self, host, enabled): + """Sets the specified host's ability to accept new instances.""" + pass diff --git a/nova/virt/vmwareapi_conn.py b/nova/virt/vmwareapi_conn.py index 3c6345ec8..d80e14931 100644 --- a/nova/virt/vmwareapi_conn.py +++ b/nova/virt/vmwareapi_conn.py @@ -190,6 +190,10 @@ class VMWareESXConnection(driver.ComputeDriver): """This method is supported only by libvirt.""" return + def set_host_enabled(self, host, enabled): + """Sets the specified host's ability to accept new instances.""" + pass + class VMWareAPISession(object): """ diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index b116c8467..44645840c 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -24,6 +24,7 @@ import json import M2Crypto import os import pickle +import random import subprocess import time import uuid @@ -932,6 +933,23 @@ class VMOps(object): # TODO: implement this! return 'http://fakeajaxconsole/fake_url' + def set_host_enabled(self, host, enabled): + """Sets the specified host's ability to accept new instances.""" + return self._call_xenhost("set_enabled", {"enabled": enabled}) + + def _call_xenhost(self, method, arg_dict): + # Create a task ID as something that won't match any instance ID + task_id = random.randint(-80000, -70000) + try: + task = self._session.async_call_plugin("xenhost", method, + args=arg_dict) + ret = self._session.wait_for_task(task, task_id) + except self.XenAPI.Failure as e: + ret = None + LOG.error(_("The call to %(method)s returned an error: %(e)s.") + % locals()) + return ret + def inject_network_info(self, instance, network_info, vm_ref=None): """ Generate the network info and make calls to place it into the diff --git a/nova/virt/xenapi_conn.py b/nova/virt/xenapi_conn.py index cd4dc1b60..ec8c44c1c 100644 --- a/nova/virt/xenapi_conn.py +++ b/nova/virt/xenapi_conn.py @@ -336,6 +336,10 @@ class XenAPIConnection(driver.ComputeDriver): True, run the update first.""" return self.HostState.get_host_stats(refresh=refresh) + def set_host_enabled(self, host, enabled): + """Sets the specified host's ability to accept new instances.""" + return self._vmops.set_host_enabled(host, enabled) + class XenAPISession(object): """The session to invoke XenAPI SDK calls""" -- cgit From 42dabbc86e3af49215ced275d76d241b4daf8bdc Mon Sep 17 00:00:00 2001 From: Ed Leafe Date: Fri, 1 Jul 2011 19:44:10 +0000 Subject: Updated unit tests --- nova/compute/api.py | 2 +- nova/virt/xenapi/vmops.py | 26 ++++++++++++- .../xenserver/xenapi/etc/xapi.d/plugins/xenhost | 45 +++++++++++++++++++--- 3 files changed, 65 insertions(+), 8 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index c99b64c50..b0eedcd64 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -915,7 +915,7 @@ class API(base.Base): def set_host_enabled(self, context, host, enabled): """Sets the specified host's ability to accept new instances.""" return self._call_compute_message("set_host_enabled", context, - instance_id=None, host=host, enabled=enabled) + instance_id=None, host=host, params={"enabled": enabled}) @scheduler_api.reroute_compute("diagnostics") def get_diagnostics(self, context, instance_id): diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 44645840c..535a55ebb 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -935,20 +935,44 @@ class VMOps(object): def set_host_enabled(self, host, enabled): """Sets the specified host's ability to accept new instances.""" - return self._call_xenhost("set_enabled", {"enabled": enabled}) + args = {"enabled": json.dumps(enabled)} + json_resp = self._call_xenhost("set_host_enabled", args) + resp = json.loads(json_resp) + return resp["status"] def _call_xenhost(self, method, arg_dict): + """There will be several methods that will need this general + handling for interacting with the xenhost plugin, so this abstracts + out that behavior. + """ # Create a task ID as something that won't match any instance ID task_id = random.randint(-80000, -70000) try: task = self._session.async_call_plugin("xenhost", method, args=arg_dict) + #args={"params": arg_dict}) ret = self._session.wait_for_task(task, task_id) except self.XenAPI.Failure as e: ret = None LOG.error(_("The call to %(method)s returned an error: %(e)s.") % locals()) return ret +# def set_host_enabled(self, host, enabled): +# """Sets the specified host's ability to accept new instances.""" +# return self._call_xenhost("set_host_enabled", {"enabled": enabled}) +# +# def _call_xenhost(self, method, arg_dict): +# # Create a task ID as something that won't match any instance ID +# task_id = random.randint(-80000, -70000) +# try: +# task = self._session.async_call_plugin("xenhost", method, +# args=arg_dict) +# ret = self._session.wait_for_task(task, task_id) +# except self.XenAPI.Failure as e: +# ret = None +# LOG.error(_("The call to %(method)s returned an error: %(e)s.") +# % locals()) +# return ret def inject_network_info(self, instance, network_info, vm_ref=None): """ diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost b/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost index a8428e841..13e62929a 100644 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost @@ -33,9 +33,10 @@ import tempfile import time import XenAPIPlugin +import pluginlib_nova as pluginlib -from pluginlib_nova import * -configure_logging("xenhost") + +pluginlib.configure_logging("xenhost") host_data_pattern = re.compile(r"\s*(\S+) \([^\)]+\) *: ?(.*)") @@ -65,14 +66,45 @@ def _run_command(cmd): return proc.stdout.read() +def _get_host_uuid(): + cmd = "xe host-list | grep uuid" + resp = _run_command(cmd) + return resp.split(":")[-1].strip() + + +@jsonify +def set_host_enabled(self, arg_dict): + """Sets this host's ability to accept new instances. + It will otherwise continue to operate normally. + """ + enabled = arg_dict.get("enabled") + if enabled is None: + raise pluginlib.PluginError( + _("Missing 'enabled' argument to set_host_enabled")) + if enabled == "true": + result = _run_command("xe host-enable") + elif enabled == "false": + result = _run_command("xe host-disable") + else: + raise pluginlib.PluginError(_("Illegal enabled status: %s") % enabled) + # Should be empty string + if result: + raise pluginlib.PluginError(result) + # Return the current enabled status + host_uuid = _get_host_uuid() + cmd = "xe host-param-list uuid=%s | grep enabled" % host_uuid + resp = _run_command(cmd) + # Response should be in the format: "enabled ( RO): true" + status = resp.strip().split()[-1] + return {"status": status} + + @jsonify def host_data(self, arg_dict): """Runs the commands on the xenstore host to return the current status information. """ - cmd = "xe host-list | grep uuid" - resp = _run_command(cmd) - host_uuid = resp.split(":")[-1].strip() + host_uuid = _get_host_uuid() cmd = "xe host-param-list uuid=%s" % host_uuid resp = _run_command(cmd) parsed_data = parse_response(resp) @@ -180,4 +212,5 @@ def cleanup(dct): if __name__ == "__main__": XenAPIPlugin.dispatch( - {"host_data": host_data}) + {"host_data": host_data, + "set_host_enabled": set_host_enabled}) -- cgit From 49c8202f43b9f606d9bd0a362b5805be98460326 Mon Sep 17 00:00:00 2001 From: Ed Leafe Date: Fri, 1 Jul 2011 19:53:06 +0000 Subject: removed debugging output --- nova/api/openstack/wsgi.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nova/api/openstack/wsgi.py b/nova/api/openstack/wsgi.py index ba8ee8bfd..5b6e3cb1d 100644 --- a/nova/api/openstack/wsgi.py +++ b/nova/api/openstack/wsgi.py @@ -362,7 +362,6 @@ class Resource(wsgi.Application): "url": request.url}) try: - print "BODY: >%s<"%request.body action, action_args, accept = self.deserializer.deserialize( request) except exception.InvalidContentType: -- cgit From 2dc2a5f66dc039ff1755981374f4065d048bcc26 Mon Sep 17 00:00:00 2001 From: Ed Leafe Date: Fri, 1 Jul 2011 20:33:00 +0000 Subject: removed more stray debug output --- nova/api/openstack/auth.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nova/api/openstack/auth.py b/nova/api/openstack/auth.py index 6231216c9..7c3e683d6 100644 --- a/nova/api/openstack/auth.py +++ b/nova/api/openstack/auth.py @@ -100,7 +100,6 @@ class AuthMiddleware(wsgi.Middleware): token, user = self._authorize_user(username, key, req) if user and token: - print "TOKEN:", token['token_hash'] res = webob.Response() res.headers['X-Auth-Token'] = token['token_hash'] res.headers['X-Server-Management-Url'] = \ -- cgit From 723e5076a414b52e716f5a5cac7667c5b09a36d3 Mon Sep 17 00:00:00 2001 From: Ed Leafe Date: Fri, 1 Jul 2011 20:39:56 +0000 Subject: removed unneeded old commented code --- nova/virt/xenapi/vmops.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 535a55ebb..cb96930c1 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -957,22 +957,6 @@ class VMOps(object): LOG.error(_("The call to %(method)s returned an error: %(e)s.") % locals()) return ret -# def set_host_enabled(self, host, enabled): -# """Sets the specified host's ability to accept new instances.""" -# return self._call_xenhost("set_host_enabled", {"enabled": enabled}) -# -# def _call_xenhost(self, method, arg_dict): -# # Create a task ID as something that won't match any instance ID -# task_id = random.randint(-80000, -70000) -# try: -# task = self._session.async_call_plugin("xenhost", method, -# args=arg_dict) -# ret = self._session.wait_for_task(task, task_id) -# except self.XenAPI.Failure as e: -# ret = None -# LOG.error(_("The call to %(method)s returned an error: %(e)s.") -# % locals()) -# return ret def inject_network_info(self, instance, network_info, vm_ref=None): """ -- cgit From 7307f17edeb284a6b2da076ffa16b2ef5c82a4f4 Mon Sep 17 00:00:00 2001 From: Ed Leafe Date: Mon, 4 Jul 2011 15:41:37 +0000 Subject: Added missing extension file and tests. Also modified the get_host_list() docstring to be more accurate about the return value. --- nova/api/openstack/contrib/hosts.py | 112 ++++++++++++++++++++++++++++++++++++ nova/scheduler/zone_manager.py | 5 +- nova/tests/test_hosts.py | 101 ++++++++++++++++++++++++++++++++ 3 files changed, 216 insertions(+), 2 deletions(-) create mode 100644 nova/api/openstack/contrib/hosts.py create mode 100644 nova/tests/test_hosts.py diff --git a/nova/api/openstack/contrib/hosts.py b/nova/api/openstack/contrib/hosts.py new file mode 100644 index 000000000..40a260c20 --- /dev/null +++ b/nova/api/openstack/contrib/hosts.py @@ -0,0 +1,112 @@ +# Copyright (c) 2011 Openstack, LLC. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""The hosts admin extension.""" + +from webob import exc + +from nova import compute +from nova import exception +from nova import flags +from nova import log as logging +from nova.api.openstack import common +from nova.api.openstack import extensions +from nova.api.openstack import faults +from nova.scheduler import api as scheduler_api + + +LOG = logging.getLogger("nova.api.hosts") +FLAGS = flags.FLAGS + + +def _list_hosts(req, service=None): + """Returns a summary list of hosts, optionally filtering + by service type. + """ + context = req.environ['nova.context'] + hosts = scheduler_api.get_host_list(context) + if service: + hosts = [host for host in hosts + if host["service"] == service] + return hosts + + +def check_host(fn): + """Makes sure that the host exists.""" + def wrapped(self, req, id, service=None, *args, **kwargs): + listed_hosts = _list_hosts(req, service) + hosts = [h["host_name"] for h in listed_hosts] + if id in hosts: + return fn(self, req, id, *args, **kwargs) + else: + raise exception.HostNotFound(host=id) + return wrapped + + +class HostController(object): + """The Hosts API controller for the OpenStack API.""" + def __init__(self): + self.compute_api = compute.API() + super(HostController, self).__init__() + + def index(self, req): + return {'hosts': _list_hosts(req)} + + @check_host + def update(self, req, id, body): + for raw_key, raw_val in body.iteritems(): + key = raw_key.lower().strip() + val = raw_val.lower().strip() + # NOTE: (dabo) Right now only 'status' can be set, but other + # actions may follow. + if key == "status": + if val in ("enable", "disable"): + return self._set_enabled_status(req, id, + enabled=(val == "enable")) + else: + raise ValueError(_("Invalid status: '%s'") % raw_val) + else: + raise ValueError(_("Invalid update setting: '%s'") % raw_key) + + def _set_enabled_status(self, req, host, enabled): + """Sets the specified host's ability to accept new instances.""" + context = req.environ['nova.context'] + state = "enabled" if enabled else "disabled" + LOG.audit(_("Setting host %(host)s to %(state)s.") % locals()) + result = self.compute_api.set_host_enabled(context, host=host, + enabled=enabled) + return {"host": host, "status": result} + + +class Hosts(extensions.ExtensionDescriptor): + def get_name(self): + return "Hosts" + + def get_alias(self): + return "os-hosts" + + def get_description(self): + return "Host administration" + + def get_namespace(self): + return "http://docs.openstack.org/ext/hosts/api/v1.1" + + def get_updated(self): + return "2011-06-29T00:00:00+00:00" + + def get_resources(self): + resources = [extensions.ResourceExtension('os-hosts', HostController(), + collection_actions={'update': 'PUT'}, member_actions={})] + return resources diff --git a/nova/scheduler/zone_manager.py b/nova/scheduler/zone_manager.py index 169a96989..6093443a9 100644 --- a/nova/scheduler/zone_manager.py +++ b/nova/scheduler/zone_manager.py @@ -116,8 +116,9 @@ class ZoneManager(object): return [zone.to_dict() for zone in self.zone_states.values()] def get_host_list(self): - """Returns a list of all the host names that the Zone Manager - knows about. + """Returns a list of dicts for each host that the Zone Manager + knows about. Each dict contains the host_name and the service + for that host. """ all_hosts = self.service_states.keys() ret = [] diff --git a/nova/tests/test_hosts.py b/nova/tests/test_hosts.py new file mode 100644 index 000000000..9c3344873 --- /dev/null +++ b/nova/tests/test_hosts.py @@ -0,0 +1,101 @@ +# Copyright (c) 2011 Openstack, LLC. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import stubout + +from nova import context +from nova import exception +from nova import flags +from nova import log as logging +from nova import test +from nova.api.openstack.contrib import hosts as os_hosts +from nova.scheduler import api as scheduler_api + + +FLAGS = flags.FLAGS +LOG = logging.getLogger('nova.tests.hosts') +# Simulate the hosts returned by the zone manager. +HOST_LIST = [ + {"host_name": "host_c1", "service": "compute"}, + {"host_name": "host_c2", "service": "compute"}, + {"host_name": "host_v1", "service": "volume"}, + {"host_name": "host_v2", "service": "volume"}] + + +def stub_get_host_list(req): + return HOST_LIST + + +def stub_set_host_enabled(context, host, enabled): + # We'll simulate success and failure by assuming + # that 'host_c1' always succeeds, and 'host_c2' + # always fails + fail = (host == "host_c2") + status = "enabled" if (enabled ^ fail) else "disabled" + return status + + +class FakeRequest(object): + environ = {"nova.context": context.get_admin_context()} + + +class HostTestCase(test.TestCase): + """Test Case for hosts.""" + + def setUp(self): + super(HostTestCase, self).setUp() + self.controller = os_hosts.HostController() + self.req = FakeRequest() + self.stubs.Set(scheduler_api, 'get_host_list', stub_get_host_list) + self.stubs.Set(self.controller.compute_api, 'set_host_enabled', + stub_set_host_enabled) + + def test_list_hosts(self): + """Verify that the compute hosts are returned.""" + hosts = os_hosts._list_hosts(self.req) + self.assertEqual(hosts, HOST_LIST) + + compute_hosts = os_hosts._list_hosts(self.req, "compute") + expected = [host for host in HOST_LIST + if host["service"] == "compute"] + self.assertEqual(compute_hosts, expected) + + def test_disable_host(self): + dis_body = {"status": "disable"} + result_c1 = self.controller.update(self.req, "host_c1", body=dis_body) + self.assertEqual(result_c1["status"], "disabled") + result_c2 = self.controller.update(self.req, "host_c2", body=dis_body) + self.assertEqual(result_c2["status"], "enabled") + + def test_enable_host(self): + en_body = {"status": "enable"} + result_c1 = self.controller.update(self.req, "host_c1", body=en_body) + self.assertEqual(result_c1["status"], "enabled") + result_c2 = self.controller.update(self.req, "host_c2", body=en_body) + self.assertEqual(result_c2["status"], "disabled") + + def test_bad_status_value(self): + bad_body = {"status": "bad"} + self.assertRaises(ValueError, self.controller.update, self.req, + "host_c1", body=bad_body) + + def test_bad_update_key(self): + bad_body = {"crazy": "bad"} + self.assertRaises(ValueError, self.controller.update, self.req, + "host_c1", body=bad_body) + + def test_bad_host(self): + self.assertRaises(exception.HostNotFound, self.controller.update, + self.req, "bogus_host_name", body={"status": "disable"}) -- cgit From 33ee3015e28e23e2d875d6467215fd7e0071f7e4 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Mon, 4 Jul 2011 17:02:50 +0100 Subject: Fixing weird error while running tests. Fix required patching nova/tests/___init__.py explictly importing nova.test --- nova/__init__.py | 1 + nova/tests/test_xenapi.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/nova/__init__.py b/nova/__init__.py index 884c4a713..92516f48b 100644 --- a/nova/__init__.py +++ b/nova/__init__.py @@ -33,5 +33,6 @@ import gettext +from nova import test gettext.install("nova", unicode=1) diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py index 3302bd095..4cb7447d3 100644 --- a/nova/tests/test_xenapi.py +++ b/nova/tests/test_xenapi.py @@ -450,7 +450,7 @@ class XenAPIVMTestCase(test.TestCase): self._check_vdis(vdi_recs_start, vdi_recs_end) def test_spawn_fail_cleanup_2(self): - """Simulates an error while creating VM record. + """Simulates an error while creating VM record. It verifies that VDIs created are properly cleaned up. -- cgit From cb4e9afc4648e341cc14416a1d31d6459b9d9a61 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Mon, 4 Jul 2011 17:22:08 +0100 Subject: Removing import of nova.test added to nova/__init.py__ as problem turned out to be somewhere else (not in nova source code tree) --- nova/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nova/__init__.py b/nova/__init__.py index 92516f48b..884c4a713 100644 --- a/nova/__init__.py +++ b/nova/__init__.py @@ -33,6 +33,5 @@ import gettext -from nova import test gettext.install("nova", unicode=1) -- cgit From f9b6c84842cbb494d09de9debaee2ee37d49815c Mon Sep 17 00:00:00 2001 From: Chuck Short Date: Tue, 5 Jul 2011 08:32:04 -0400 Subject: UML doesnt do vnc as well --- nova/virt/libvirt/connection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/virt/libvirt/connection.py b/nova/virt/libvirt/connection.py index 0c6eaab84..233d14203 100644 --- a/nova/virt/libvirt/connection.py +++ b/nova/virt/libvirt/connection.py @@ -1015,7 +1015,7 @@ class LibvirtConnection(driver.ComputeDriver): 'volumes': block_device_mapping} if FLAGS.vnc_enabled: - if FLAGS.libvirt_type != 'lxc': + if FLAGS.libvirt_type != 'lxc' or FLAGS.libvirt_type != 'uml': xml_info['vncserver_host'] = FLAGS.vncserver_host xml_info['vnc_keymap'] = FLAGS.vnc_keymap if not rescue: -- cgit From b02b1d78245634f81a27d0ba0a6e29024495c162 Mon Sep 17 00:00:00 2001 From: Ed Leafe Date: Tue, 5 Jul 2011 14:36:52 +0000 Subject: Updated the plugin to return the actual enabled status instead of just 'true' or 'false' . --- plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost b/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost index 13e62929a..292bbce12 100644 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost @@ -95,7 +95,11 @@ def set_host_enabled(self, arg_dict): cmd = "xe host-param-list uuid=%s | grep enabled" % host_uuid resp = _run_command(cmd) # Response should be in the format: "enabled ( RO): true" - status = resp.strip().split()[-1] + host_enabled = resp.strip().split()[-1] + if host_enabled == "true": + status = "enabled" + else: + status = "disabled" return {"status": status} -- cgit From 6435ba27edea7e525305d349cafea3d08f5db2c6 Mon Sep 17 00:00:00 2001 From: Ed Leafe Date: Wed, 6 Jul 2011 16:53:08 +0000 Subject: Changed the exception type for invalid requests to webob.exc.HTTPBadRequest. --- nova/api/openstack/contrib/hosts.py | 12 +++++++----- nova/tests/test_hosts.py | 5 +++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/nova/api/openstack/contrib/hosts.py b/nova/api/openstack/contrib/hosts.py index 40a260c20..55e57e1a4 100644 --- a/nova/api/openstack/contrib/hosts.py +++ b/nova/api/openstack/contrib/hosts.py @@ -15,7 +15,7 @@ """The hosts admin extension.""" -from webob import exc +import webob.exc from nova import compute from nova import exception @@ -72,13 +72,15 @@ class HostController(object): # NOTE: (dabo) Right now only 'status' can be set, but other # actions may follow. if key == "status": - if val in ("enable", "disable"): + if val[:6] in ("enable", "disabl"): return self._set_enabled_status(req, id, - enabled=(val == "enable")) + enabled=(val.startswith("enable"))) else: - raise ValueError(_("Invalid status: '%s'") % raw_val) + explanation = _("Invalid status: '%s'") % raw_val + raise webob.exc.HTTPBadRequest(explanation=explanation) else: - raise ValueError(_("Invalid update setting: '%s'") % raw_key) + explanation = _("Invalid update setting: '%s'") % raw_key + raise webob.exc.HTTPBadRequest(explanation=explanation) def _set_enabled_status(self, req, host, enabled): """Sets the specified host's ability to accept new instances.""" diff --git a/nova/tests/test_hosts.py b/nova/tests/test_hosts.py index 9c3344873..c14ea349b 100644 --- a/nova/tests/test_hosts.py +++ b/nova/tests/test_hosts.py @@ -14,6 +14,7 @@ # under the License. import stubout +import webob.exc from nova import context from nova import exception @@ -88,12 +89,12 @@ class HostTestCase(test.TestCase): def test_bad_status_value(self): bad_body = {"status": "bad"} - self.assertRaises(ValueError, self.controller.update, self.req, + self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, self.req, "host_c1", body=bad_body) def test_bad_update_key(self): bad_body = {"crazy": "bad"} - self.assertRaises(ValueError, self.controller.update, self.req, + self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, self.req, "host_c1", body=bad_body) def test_bad_host(self): -- cgit From 9d29dc60d904f2c5037d03cead71933dc62777ff Mon Sep 17 00:00:00 2001 From: Ed Leafe Date: Wed, 6 Jul 2011 17:14:46 +0000 Subject: pep8 fixes --- nova/tests/test_hosts.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nova/tests/test_hosts.py b/nova/tests/test_hosts.py index c14ea349b..548f81f8b 100644 --- a/nova/tests/test_hosts.py +++ b/nova/tests/test_hosts.py @@ -89,13 +89,13 @@ class HostTestCase(test.TestCase): def test_bad_status_value(self): bad_body = {"status": "bad"} - self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, self.req, - "host_c1", body=bad_body) + self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, + self.req, "host_c1", body=bad_body) def test_bad_update_key(self): bad_body = {"crazy": "bad"} - self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, self.req, - "host_c1", body=bad_body) + self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, + self.req, "host_c1", body=bad_body) def test_bad_host(self): self.assertRaises(exception.HostNotFound, self.controller.update, -- cgit