From f79220a1f6a12621463b410d26e31e29a9e6ea3e Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Mon, 7 Mar 2011 15:41:37 -0600 Subject: cleaned up virt.xenapi.vmops._get_vm_opaque_ref. more reliable approach to checking if param is an opaque ref. code is cleaner --- nova/virt/xenapi/vmops.py | 44 +++++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 21 deletions(-) (limited to 'nova/virt') diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index b862c9de9..b1671fde4 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -188,30 +188,32 @@ class VMOps(object): """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. """ - vm = None - try: - if instance_or_vm.startswith("OpaqueRef:"): - # Got passed an opaque ref; return it + # if instance_or_vm is a string it must be opaque ref or instance name + if isinstance(instance_or_vm, str): + vm_rec = self._session.get_xenapi().VM.get_record(instance_or_vm) + if vm_rec != None: + # an opaque ref was passed in, return it return instance_or_vm else: - # Must be the instance name + # it must be an instance name instance_name = instance_or_vm - except (AttributeError, KeyError): - # Note the the KeyError will only happen with fakes.py - # Not a string; must be an ID or a vm instance - if isinstance(instance_or_vm, (int, long)): - ctx = context.get_admin_context() - try: - instance_obj = db.instance_get(ctx, instance_or_vm) - instance_name = instance_obj.name - except exception.NotFound: - # The unit tests screw this up, as they use an integer for - # the vm name. I'd fix that up, but that's a matter for - # another bug report. So for now, just try with the passed - # value - instance_name = instance_or_vm - else: - instance_name = instance_or_vm.name + + # if instance_or_vm is an int/long it must be instance id + elif isinstance(instance_or_vm, (int, long)): + ctx = context.get_admin_context() + try: + instance_obj = db.instance_get(ctx, instance_or_vm) + instance_name = instance_obj.name + except exception.NotFound: + # The unit tests screw this up, as they use an integer for + # the vm name. I'd fix that up, but that's a matter for + # another bug report. So for now, just try with the passed + # value + instance_name = instance_or_vm + + # otherwise instance_or_vm is an instance object + else: + instance_name = instance_or_vm.name vm = VMHelper.lookup(self._session, instance_name) if vm is None: raise exception.NotFound( -- cgit From 59f73e3180731cec644b590d448e0da74711ae03 Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Mon, 7 Mar 2011 16:11:10 -0600 Subject: virt.xenapi.vmops._get_vm_opaque_ref exception caught properly --- nova/virt/xenapi/vmops.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'nova/virt') diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index b1671fde4..ae4609418 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -190,13 +190,16 @@ class VMOps(object): """ # if instance_or_vm is a string it must be opaque ref or instance name if isinstance(instance_or_vm, str): - vm_rec = self._session.get_xenapi().VM.get_record(instance_or_vm) - if vm_rec != None: - # an opaque ref was passed in, return it - return instance_or_vm - else: - # it must be an instance name - instance_name = instance_or_vm + ref = None + try: + ref = self._session.get_xenapi().VM.get_record(instance_or_vm) + if ref != None: + # an opaque ref was passed in, return it + return instance_or_vm + except: + pass + # wasn't an opaque ref, must be an instance name + instance_name = instance_or_vm # if instance_or_vm is an int/long it must be instance id elif isinstance(instance_or_vm, (int, long)): -- cgit From 3fc6b8cbbd1be5baffc300112a0e39a807209c36 Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Mon, 7 Mar 2011 16:34:59 -0600 Subject: virt.xenapi.vmops._get_vm_opaque_ref checks for basestring instance instead of str --- nova/virt/xenapi/vmops.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nova/virt') diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index ae4609418..30fa5bdd7 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -189,7 +189,7 @@ class VMOps(object): a vm name or a vm instance, and want a vm instance in return. """ # if instance_or_vm is a string it must be opaque ref or instance name - if isinstance(instance_or_vm, str): + if isinstance(instance_or_vm, basestring): ref = None try: ref = self._session.get_xenapi().VM.get_record(instance_or_vm) -- cgit From 88c5555e867c730065c18541a35b161eb861b502 Mon Sep 17 00:00:00 2001 From: Cerberus Date: Mon, 7 Mar 2011 16:40:19 -0600 Subject: First part of the bug fix --- nova/virt/xenapi/vmops.py | 8 +++++++- nova/virt/xenapi_conn.py | 9 +++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) (limited to 'nova/virt') diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index b862c9de9..37f513599 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -72,7 +72,13 @@ class VMOps(object): LOG.debug(_("Starting instance %s"), instance.name) self._session.call_xenapi('VM.start', vm_ref, False, False) - def spawn(self, instance, disk): + def spawn(self, instance): + self._spawn(instance, disk=None) + + def spawn_with_disk(self, instance, disk): + self._spawn(instance, disk=disk) + + def _spawn(self, instance, disk): """Create VM instance""" instance_name = instance.name vm = VMHelper.lookup(self._session, instance_name) diff --git a/nova/virt/xenapi_conn.py b/nova/virt/xenapi_conn.py index 62e17e851..7e8f825e9 100644 --- a/nova/virt/xenapi_conn.py +++ b/nova/virt/xenapi_conn.py @@ -154,9 +154,14 @@ class XenAPIConnection(object): """List VM instances""" return self._vmops.list_instances() - def spawn(self, instance, disk=None): + def spawn(self, instance): """Create VM instance""" - self._vmops.spawn(instance, disk) + self._vmops.spawn(instance) + + def finish_resize(self, instance, disk_info) + """Completes a resize, turning on the migrated instance""" + new_disk_info = self.attach_disk(instance, disk_info) + self._vmops.spawn_with_disk(instance, new_disk_info) def snapshot(self, instance, image_id): """ Create snapshot from a running VM instance """ -- cgit From 5c7ee13b058fb954fd9bbc4a3550716b8faa0b97 Mon Sep 17 00:00:00 2001 From: "matt.dietz@rackspace.com" <> Date: Mon, 7 Mar 2011 22:50:35 +0000 Subject: And unit tests --- nova/virt/xenapi_conn.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nova/virt') diff --git a/nova/virt/xenapi_conn.py b/nova/virt/xenapi_conn.py index 7e8f825e9..3991496b2 100644 --- a/nova/virt/xenapi_conn.py +++ b/nova/virt/xenapi_conn.py @@ -158,7 +158,7 @@ class XenAPIConnection(object): """Create VM instance""" self._vmops.spawn(instance) - def finish_resize(self, instance, disk_info) + def finish_resize(self, instance, disk_info): """Completes a resize, turning on the migrated instance""" new_disk_info = self.attach_disk(instance, disk_info) self._vmops.spawn_with_disk(instance, new_disk_info) -- cgit From 2f0845b7b80081d18ee268b94fe38326f3c5401e Mon Sep 17 00:00:00 2001 From: "matt.dietz@rackspace.com" <> Date: Mon, 7 Mar 2011 23:07:05 +0000 Subject: A few more changes --- nova/virt/xenapi/vmops.py | 10 +++++----- nova/virt/xenapi_conn.py | 9 +++------ 2 files changed, 8 insertions(+), 11 deletions(-) (limited to 'nova/virt') diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 37f513599..e658de7f3 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -75,8 +75,8 @@ class VMOps(object): def spawn(self, instance): self._spawn(instance, disk=None) - def spawn_with_disk(self, instance, disk): - self._spawn(instance, disk=disk) + def spawn_with_disk(self, instance, vdi_uuid): + self._spawn(instance, disk=vdi_uuid) def _spawn(self, instance, disk): """Create VM instance""" @@ -343,14 +343,14 @@ class VMOps(object): # sensible so we don't need to blindly pass around dictionaries return {'base_copy': base_copy_uuid, 'cow': cow_uuid} - def attach_disk(self, instance, disk_info): + def attach_disk(self, instance, base_copy_uuid, cow_uuid): """Links the base copy VHD to the COW via the XAPI plugin""" vm_ref = VMHelper.lookup(self._session, instance.name) new_base_copy_uuid = str(uuid.uuid4()) new_cow_uuid = str(uuid.uuid4()) params = {'instance_id': instance.id, - 'old_base_copy_uuid': disk_info['base_copy'], - 'old_cow_uuid': disk_info['cow'], + 'old_base_copy_uuid': base_copy_uuid, + 'old_cow_uuid': cow_uuid, 'new_base_copy_uuid': new_base_copy_uuid, 'new_cow_uuid': new_cow_uuid, 'sr_path': VMHelper.get_sr_path(self._session), } diff --git a/nova/virt/xenapi_conn.py b/nova/virt/xenapi_conn.py index 3991496b2..9965accad 100644 --- a/nova/virt/xenapi_conn.py +++ b/nova/virt/xenapi_conn.py @@ -160,8 +160,9 @@ class XenAPIConnection(object): def finish_resize(self, instance, disk_info): """Completes a resize, turning on the migrated instance""" - new_disk_info = self.attach_disk(instance, disk_info) - self._vmops.spawn_with_disk(instance, new_disk_info) + cow_uuid = self._vmops.attach_disk(instance, disk_info['base_copy'], + disk_info['cow']) + self._vmops.spawn_with_disk(instance, cow_uuid) def snapshot(self, instance, image_id): """ Create snapshot from a running VM instance """ @@ -202,10 +203,6 @@ class XenAPIConnection(object): off the instance copies over the COW disk""" return self._vmops.migrate_disk_and_power_off(instance, dest) - def attach_disk(self, instance, disk_info): - """Moves the copied VDIs into the SR""" - return self._vmops.attach_disk(instance, disk_info) - def suspend(self, instance, callback): """suspend the specified instance""" self._vmops.suspend(instance, callback) -- cgit From e39995def6a2a11cdd430b0e6f603b493be5542b Mon Sep 17 00:00:00 2001 From: "matt.dietz@rackspace.com" <> Date: Mon, 7 Mar 2011 23:51:20 +0000 Subject: Some more refactoring and a tighter unit test --- nova/virt/xenapi/vmops.py | 30 ++++++++++++++---------------- nova/virt/xenapi_conn.py | 4 ++-- 2 files changed, 16 insertions(+), 18 deletions(-) (limited to 'nova/virt') diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index e658de7f3..7fe1f6ff0 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -72,13 +72,19 @@ class VMOps(object): LOG.debug(_("Starting instance %s"), instance.name) self._session.call_xenapi('VM.start', vm_ref, False, False) - def spawn(self, instance): - self._spawn(instance, disk=None) - - def spawn_with_disk(self, instance, vdi_uuid): - self._spawn(instance, disk=vdi_uuid) + def create_disk(self, instance): + user = AuthManager().get_user(instance.user_id) + 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) + return vdi_uuid - def _spawn(self, instance, disk): + def spawn(self, instance): + vdi_uuid = self.create_disk(instance) + self._spawn_with_disk(instance, vdi_uuid=vdi_uuid) + + def _spawn_with_disk(self, instance, vdi_uuid): """Create VM instance""" instance_name = instance.name vm = VMHelper.lookup(self._session, instance_name) @@ -101,17 +107,9 @@ class VMOps(object): vdi_ref = kernel = ramdisk = pv_kernel = None # Are we building from a pre-existing disk? - if not disk: - #if kernel is not present we must download a raw disk - - 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) - - else: - vdi_ref = self._session.call_xenapi('VDI.get_by_uuid', disk) + vdi_ref = self._session.call_xenapi('VDI.get_by_uuid', vdi_uuid) + disk_image_type = VMHelper.determine_disk_image_type(instance) 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, diff --git a/nova/virt/xenapi_conn.py b/nova/virt/xenapi_conn.py index 9965accad..b63a5f8c3 100644 --- a/nova/virt/xenapi_conn.py +++ b/nova/virt/xenapi_conn.py @@ -160,9 +160,9 @@ class XenAPIConnection(object): def finish_resize(self, instance, disk_info): """Completes a resize, turning on the migrated instance""" - cow_uuid = self._vmops.attach_disk(instance, disk_info['base_copy'], + vdi_uuid = self._vmops.attach_disk(instance, disk_info['base_copy'], disk_info['cow']) - self._vmops.spawn_with_disk(instance, cow_uuid) + self._vmops._spawn_with_disk(instance, vdi_uuid) def snapshot(self, instance, image_id): """ Create snapshot from a running VM instance """ -- cgit From 5ec9cbcdee3de3868a47ca5ec351a9a2594ceea2 Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Mon, 7 Mar 2011 18:05:27 -0600 Subject: virt.xenapi.vmops._get_vm_opaque_ref assumes VM.get_record raises --- nova/virt/xenapi/vmops.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'nova/virt') diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 30fa5bdd7..c0fbf96fc 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -192,14 +192,12 @@ class VMOps(object): if isinstance(instance_or_vm, basestring): ref = None try: + # check for opaque ref ref = self._session.get_xenapi().VM.get_record(instance_or_vm) - if ref != None: - # an opaque ref was passed in, return it - return instance_or_vm - except: - pass - # wasn't an opaque ref, must be an instance name - instance_name = instance_or_vm + return instance_or_vm + except self.XenAPI.Failure: + # wasn't an opaque ref, must be an instance name + instance_name = instance_or_vm # if instance_or_vm is an int/long it must be instance id elif isinstance(instance_or_vm, (int, long)): -- cgit From 4e8b6a14324ef2d1f550233cbcfc94c6363533d8 Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Mon, 7 Mar 2011 18:46:44 -0600 Subject: virt.xenapi.vmops._get_vm_opaque_ref changed vm to vm_ref and ref to obj --- nova/virt/xenapi/vmops.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'nova/virt') diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index c0fbf96fc..0adabe7f5 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -190,10 +190,10 @@ class VMOps(object): """ # if instance_or_vm is a string it must be opaque ref or instance name if isinstance(instance_or_vm, basestring): - ref = None + obj = None try: # check for opaque ref - ref = self._session.get_xenapi().VM.get_record(instance_or_vm) + obj = self._session.get_xenapi().VM.get_record(instance_or_vm) return instance_or_vm except self.XenAPI.Failure: # wasn't an opaque ref, must be an instance name @@ -215,11 +215,11 @@ class VMOps(object): # otherwise instance_or_vm is an instance object else: instance_name = instance_or_vm.name - vm = VMHelper.lookup(self._session, instance_name) - if vm is None: + vm_ref = VMHelper.lookup(self._session, instance_name) + if vm_ref is None: raise exception.NotFound( _('Instance not present %s') % instance_name) - return vm + return vm_ref def _acquire_bootlock(self, vm): """Prevent an instance from booting""" -- cgit From 1caceddf431a1ad1ef22235c2206bccf39fde5c5 Mon Sep 17 00:00:00 2001 From: Cerberus Date: Tue, 8 Mar 2011 14:24:01 -0600 Subject: Nits --- nova/virt/xenapi/vmops.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'nova/virt') diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 7fe1f6ff0..86dbf251b 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -90,7 +90,7 @@ class VMOps(object): vm = VMHelper.lookup(self._session, instance_name) if vm is not None: raise exception.Duplicate(_('Attempted to create' - ' non-unique name %s') % instance_name) + ' non-unique name %s') % instance_name) #ensure enough free memory is available if not VMHelper.ensure_free_mem(self._session, instance): @@ -104,7 +104,7 @@ class VMOps(object): user = AuthManager().get_user(instance.user_id) project = AuthManager().get_project(instance.project_id) - vdi_ref = kernel = ramdisk = pv_kernel = None + kernel = ramdisk = pv_kernel = None # Are we building from a pre-existing disk? vdi_ref = self._session.call_xenapi('VDI.get_by_uuid', vdi_uuid) -- cgit From dd2f0019297d01fe5d6b3dae4efc72946191be75 Mon Sep 17 00:00:00 2001 From: Rick Harris Date: Tue, 8 Mar 2011 22:14:25 +0000 Subject: Use disk_format and container_format instead of image type --- nova/virt/xenapi/vm_utils.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'nova/virt') diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index 80b7540d4..ce081a2d6 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -467,19 +467,21 @@ class VMHelper(HelperBase): "%(image_id)s, instance %(instance_id)s") % locals()) def determine_from_glance(): - glance_type2nova_type = {'machine': ImageType.DISK, - 'raw': ImageType.DISK_RAW, - 'vhd': ImageType.DISK_VHD, - 'kernel': ImageType.KERNEL_RAMDISK, - 'ramdisk': ImageType.KERNEL_RAMDISK} + glance_disk_format2nova_type = { + 'ami': ImageType.DISK, + 'aki': ImageType.KERNEL_RAMDISK, + 'ari': ImageType.KERNEL_RAMDISK, + 'raw': ImageType.DISK_RAW, + 'vhd': ImageType.DISK_VHD} client = glance.client.Client(FLAGS.glance_host, FLAGS.glance_port) meta = client.get_image_meta(instance.image_id) - type_ = meta['type'] + disk_format = meta['disk_format'] try: - return glance_type2nova_type[type_] + return glance_disk_format2nova_type[disk_format] except KeyError: raise exception.NotFound( - _("Unrecognized image type '%(type_)s'") % locals()) + _("Unrecognized disk_format '%(disk_format)s'") + % locals()) def determine_from_instance(): if instance.kernel_id: -- cgit From 698398fdc2a05a0930591d3f3d386ad24a322359 Mon Sep 17 00:00:00 2001 From: "matt.dietz@rackspace.com" <> Date: Tue, 8 Mar 2011 23:24:19 +0000 Subject: Pep8 fixes --- nova/virt/xenapi/vmops.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nova/virt') diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 43c23806e..562ecd4d5 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -83,7 +83,7 @@ class VMOps(object): def spawn(self, instance): vdi_uuid = self.create_disk(instance) self._spawn_with_disk(instance, vdi_uuid=vdi_uuid) - + def _spawn_with_disk(self, instance, vdi_uuid): """Create VM instance""" instance_name = instance.name -- cgit