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(-) 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(-) 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(-) 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 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(-) 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(-) 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