From 8aea573bd2e44e152fb4ef1627640bab1818dede Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Tue, 28 Dec 2010 23:55:58 -0600 Subject: initial lock functionality commit --- nova/compute/api.py | 35 ++++++++++++++++++++++++++++++++++- nova/compute/manager.py | 24 ++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) (limited to 'nova/compute') diff --git a/nova/compute/api.py b/nova/compute/api.py index a47703461..361ab9914 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -141,7 +141,8 @@ class ComputeAPI(base.Base): 'display_description': description, 'user_data': user_data or '', 'key_name': key_name, - 'key_data': key_data} + 'key_data': key_data, + 'locked': False} elevated = context.elevated() instances = [] @@ -319,3 +320,35 @@ class ComputeAPI(base.Base): self.db.queue_get_for(context, FLAGS.compute_topic, host), {"method": "unrescue_instance", "args": {"instance_id": instance['id']}}) + + def lock(self, context, instance_id): + """ + lock the instance with instance_id + + """ + instance = self.get_instance(context, instance_id) + host = instance['host'] + rpc.cast(context, + self.db.queue_get_for(context, FLAGS.compute_topic, host), + {"method": "lock_instance", + "args": {"instance_id": instance['id']}}) + + def unlock(self, context, instance_id): + """ + unlock the instance with instance_id + + """ + instance = self.get_instance(context, instance_id) + host = instance['host'] + rpc.cast(context, + self.db.queue_get_for(context, FLAGS.compute_topic, host), + {"method": "unlock_instance", + "args": {"instance_id": instance['id']}}) + + def get_lock(self, context, instance_id): + """ + return the boolean state of (instance with instance_id)'s lock + + """ + instance = self.get_instance(context, instance_id) + return instance['locked'] diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 70b175e7c..05f1e44a2 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -329,6 +329,30 @@ class ComputeManager(manager.Manager): instance_id, result)) + @exception.wrap_exception + def lock_instance(self, context, instance_id): + """ + lock the instance with instance_id + + """ + context = context.elevated() + instance_ref = self.db.instance_get(context, instance_id) + + logging.debug(_('instance %s: locking'), instance_ref['internal_id']) + self.db.instance_set_lock(context, instance_id, True) + + @exception.wrap_exception + def unlock_instance(self, context, instance_id): + """ + unlock the instance with instance_id + + """ + context = context.elevated() + instance_ref = self.db.instance_get(context, instance_id) + + logging.debug(_('instance %s: unlocking'), instance_ref['internal_id']) + self.db.instance_set_lock(context, instance_id, False) + @exception.wrap_exception def get_console_output(self, context, instance_id): """Send the console output for an instance.""" -- cgit From 24e253a1feaa0a39e4095f447f62f7ea9b43c8bb Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Wed, 29 Dec 2010 18:30:01 -0600 Subject: moved check lock decorator to compute api level. altered openstack.test_servers according and wrote test for lock in tests.test_compute --- nova/compute/api.py | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) (limited to 'nova/compute') diff --git a/nova/compute/api.py b/nova/compute/api.py index 361ab9914..d720a8f2c 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -23,6 +23,7 @@ Handles all API requests relating to instances (guest vms). import datetime import logging import time +import functools from nova import db from nova import exception @@ -36,6 +37,40 @@ from nova.db import base FLAGS = flags.FLAGS +def checks_instance_lock(function): + """ + decorator used for preventing action against locked instances + unless, of course, you happen to be admin + + """ + + @functools.wraps(function) + def decorated_function(*args, **kwargs): + + # grab args to function + try: + if 'context' in kwargs: + context = kwargs['context'] + else: + context = args[1] + if 'instance_id' in kwargs: + instance_id = kwargs['instance_id'] + else: + instance_id = args[2] + locked = ComputeAPI().get_lock(context, instance_id) + admin = context.is_admin + except: + logging.error(_("check_instance_lock: argument error: |%s|, |%s|"), + args, + kwargs) + # if admin or unlocked call function, otherwise 405 + if admin or not locked: + return function(*args, **kwargs) + raise Exception(_("Instance is locked, cannot execute |%s|"), function) + + return decorated_function + + def generate_default_hostname(internal_id): """Default function to generate a hostname given an instance reference.""" return str(internal_id) @@ -198,6 +233,7 @@ class ComputeAPI(base.Base): 'project_id': context.project_id} db.security_group_create(context, values) + @checks_instance_lock def update_instance(self, context, instance_id, **kwargs): """Updates the instance in the datastore. @@ -212,6 +248,7 @@ class ComputeAPI(base.Base): """ return self.db.instance_update(context, instance_id, kwargs) + @checks_instance_lock def delete_instance(self, context, instance_id): logging.debug("Going to try and terminate %d" % instance_id) try: @@ -258,6 +295,7 @@ class ComputeAPI(base.Base): def get_instance(self, context, instance_id): return self.db.instance_get_by_internal_id(context, instance_id) + @checks_instance_lock def reboot(self, context, instance_id): """Reboot the given instance.""" instance = self.db.instance_get_by_internal_id(context, instance_id) @@ -267,6 +305,7 @@ class ComputeAPI(base.Base): {"method": "reboot_instance", "args": {"instance_id": instance['id']}}) + @checks_instance_lock def pause(self, context, instance_id): """Pause the given instance.""" instance = self.db.instance_get_by_internal_id(context, instance_id) @@ -276,6 +315,7 @@ class ComputeAPI(base.Base): {"method": "pause_instance", "args": {"instance_id": instance['id']}}) + @checks_instance_lock def unpause(self, context, instance_id): """Unpause the given instance.""" instance = self.db.instance_get_by_internal_id(context, instance_id) @@ -285,6 +325,7 @@ class ComputeAPI(base.Base): {"method": "unpause_instance", "args": {"instance_id": instance['id']}}) + @checks_instance_lock def suspend(self, context, instance_id): """suspend the instance with instance_id""" instance = self.db.instance_get_by_internal_id(context, instance_id) @@ -294,6 +335,7 @@ class ComputeAPI(base.Base): {"method": "suspend_instance", "args": {"instance_id": instance['id']}}) + @checks_instance_lock def resume(self, context, instance_id): """resume the instance with instance_id""" instance = self.db.instance_get_by_internal_id(context, instance_id) @@ -303,6 +345,7 @@ class ComputeAPI(base.Base): {"method": "resume_instance", "args": {"instance_id": instance['id']}}) + @checks_instance_lock def rescue(self, context, instance_id): """Rescue the given instance.""" instance = self.db.instance_get_by_internal_id(context, instance_id) @@ -312,6 +355,7 @@ class ComputeAPI(base.Base): {"method": "rescue_instance", "args": {"instance_id": instance['id']}}) + @checks_instance_lock def unrescue(self, context, instance_id): """Unrescue the given instance.""" instance = self.db.instance_get_by_internal_id(context, instance_id) -- cgit From 74785bf8c070bf0760724b3412f4ee1bb05cf72b Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Wed, 29 Dec 2010 18:34:49 -0600 Subject: fixd variables being out of scope in lock decorator --- nova/compute/api.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'nova/compute') diff --git a/nova/compute/api.py b/nova/compute/api.py index d720a8f2c..073129c13 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -47,6 +47,10 @@ def checks_instance_lock(function): @functools.wraps(function) def decorated_function(*args, **kwargs): + # assume worst case (have to declare so they are in scope) + admin = False + locked = True + # grab args to function try: if 'context' in kwargs: @@ -60,7 +64,7 @@ def checks_instance_lock(function): locked = ComputeAPI().get_lock(context, instance_id) admin = context.is_admin except: - logging.error(_("check_instance_lock: argument error: |%s|, |%s|"), + raise Exception(_("check_instance_lock argument error |%s|, |%s|"), args, kwargs) # if admin or unlocked call function, otherwise 405 -- cgit From d06f85c611adf244f2c757f023c92c2b6cad2e7c Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Wed, 29 Dec 2010 18:40:03 -0600 Subject: altered error exception/logging --- nova/compute/api.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'nova/compute') diff --git a/nova/compute/api.py b/nova/compute/api.py index 073129c13..19459c6d9 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -64,8 +64,7 @@ def checks_instance_lock(function): locked = ComputeAPI().get_lock(context, instance_id) admin = context.is_admin except: - raise Exception(_("check_instance_lock argument error |%s|, |%s|"), - args, + logging.error(_("check_instance_lock: arguments: |%s| |%s|"), args, kwargs) # if admin or unlocked call function, otherwise 405 if admin or not locked: -- cgit From 837724193ece16310ff588a84d23891a75ced2f2 Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Wed, 29 Dec 2010 18:46:36 -0600 Subject: altered error exception/logging --- nova/compute/api.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'nova/compute') diff --git a/nova/compute/api.py b/nova/compute/api.py index 19459c6d9..6602f2534 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -63,9 +63,11 @@ def checks_instance_lock(function): instance_id = args[2] locked = ComputeAPI().get_lock(context, instance_id) admin = context.is_admin - except: + except Excetion as e: logging.error(_("check_instance_lock: arguments: |%s| |%s|"), args, kwargs) + raise e + # if admin or unlocked call function, otherwise 405 if admin or not locked: return function(*args, **kwargs) -- cgit From 6f76367d2fefcec9b957352dd60e76c2cc3ba233 Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Wed, 29 Dec 2010 18:49:45 -0600 Subject: typo, trying to hurry.. look where that got me --- nova/compute/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nova/compute') diff --git a/nova/compute/api.py b/nova/compute/api.py index 6602f2534..232d1f26b 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -63,7 +63,7 @@ def checks_instance_lock(function): instance_id = args[2] locked = ComputeAPI().get_lock(context, instance_id) admin = context.is_admin - except Excetion as e: + except Exception as e: logging.error(_("check_instance_lock: arguments: |%s| |%s|"), args, kwargs) raise e -- cgit From b848f7459eb65ad365177d831783b3d63818f977 Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Wed, 29 Dec 2010 18:57:33 -0600 Subject: added some logging --- nova/compute/api.py | 3 +++ 1 file changed, 3 insertions(+) (limited to 'nova/compute') diff --git a/nova/compute/api.py b/nova/compute/api.py index 232d1f26b..0513ce95d 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -47,6 +47,9 @@ def checks_instance_lock(function): @functools.wraps(function) def decorated_function(*args, **kwargs): + logging.info(_("check_instance_locks decorating |%s|"), function) + logging.info(_("check_instance_locks: arguments: |%s| |%s|"), args, + kwargs) # assume worst case (have to declare so they are in scope) admin = False locked = True -- cgit From 32b310f430c5db05c99de65a5bd400675770ef1d Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Wed, 29 Dec 2010 19:27:43 -0600 Subject: removed db.set_lock, using update_instance instead --- nova/compute/manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'nova/compute') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 05f1e44a2..9a33c7cac 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -339,7 +339,7 @@ class ComputeManager(manager.Manager): instance_ref = self.db.instance_get(context, instance_id) logging.debug(_('instance %s: locking'), instance_ref['internal_id']) - self.db.instance_set_lock(context, instance_id, True) + self.db.instance_update(context, instance_id, {'locked': True}) @exception.wrap_exception def unlock_instance(self, context, instance_id): @@ -351,7 +351,7 @@ class ComputeManager(manager.Manager): instance_ref = self.db.instance_get(context, instance_id) logging.debug(_('instance %s: unlocking'), instance_ref['internal_id']) - self.db.instance_set_lock(context, instance_id, False) + self.db.instance_update(context, instance_id, {'locked': False}) @exception.wrap_exception def get_console_output(self, context, instance_id): -- cgit From f1523f2fd19cde4ddbb046dc0362a0ac7d6b79e8 Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Wed, 29 Dec 2010 20:48:33 -0600 Subject: moved check lock decorator from the compute api to the come manager... when it rains it pours --- nova/compute/api.py | 52 --------------------------------------- nova/compute/manager.py | 65 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 52 deletions(-) (limited to 'nova/compute') diff --git a/nova/compute/api.py b/nova/compute/api.py index 0513ce95d..361ab9914 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -23,7 +23,6 @@ Handles all API requests relating to instances (guest vms). import datetime import logging import time -import functools from nova import db from nova import exception @@ -37,48 +36,6 @@ from nova.db import base FLAGS = flags.FLAGS -def checks_instance_lock(function): - """ - decorator used for preventing action against locked instances - unless, of course, you happen to be admin - - """ - - @functools.wraps(function) - def decorated_function(*args, **kwargs): - - logging.info(_("check_instance_locks decorating |%s|"), function) - logging.info(_("check_instance_locks: arguments: |%s| |%s|"), args, - kwargs) - # assume worst case (have to declare so they are in scope) - admin = False - locked = True - - # grab args to function - try: - if 'context' in kwargs: - context = kwargs['context'] - else: - context = args[1] - if 'instance_id' in kwargs: - instance_id = kwargs['instance_id'] - else: - instance_id = args[2] - locked = ComputeAPI().get_lock(context, instance_id) - admin = context.is_admin - except Exception as e: - logging.error(_("check_instance_lock: arguments: |%s| |%s|"), args, - kwargs) - raise e - - # if admin or unlocked call function, otherwise 405 - if admin or not locked: - return function(*args, **kwargs) - raise Exception(_("Instance is locked, cannot execute |%s|"), function) - - return decorated_function - - def generate_default_hostname(internal_id): """Default function to generate a hostname given an instance reference.""" return str(internal_id) @@ -241,7 +198,6 @@ class ComputeAPI(base.Base): 'project_id': context.project_id} db.security_group_create(context, values) - @checks_instance_lock def update_instance(self, context, instance_id, **kwargs): """Updates the instance in the datastore. @@ -256,7 +212,6 @@ class ComputeAPI(base.Base): """ return self.db.instance_update(context, instance_id, kwargs) - @checks_instance_lock def delete_instance(self, context, instance_id): logging.debug("Going to try and terminate %d" % instance_id) try: @@ -303,7 +258,6 @@ class ComputeAPI(base.Base): def get_instance(self, context, instance_id): return self.db.instance_get_by_internal_id(context, instance_id) - @checks_instance_lock def reboot(self, context, instance_id): """Reboot the given instance.""" instance = self.db.instance_get_by_internal_id(context, instance_id) @@ -313,7 +267,6 @@ class ComputeAPI(base.Base): {"method": "reboot_instance", "args": {"instance_id": instance['id']}}) - @checks_instance_lock def pause(self, context, instance_id): """Pause the given instance.""" instance = self.db.instance_get_by_internal_id(context, instance_id) @@ -323,7 +276,6 @@ class ComputeAPI(base.Base): {"method": "pause_instance", "args": {"instance_id": instance['id']}}) - @checks_instance_lock def unpause(self, context, instance_id): """Unpause the given instance.""" instance = self.db.instance_get_by_internal_id(context, instance_id) @@ -333,7 +285,6 @@ class ComputeAPI(base.Base): {"method": "unpause_instance", "args": {"instance_id": instance['id']}}) - @checks_instance_lock def suspend(self, context, instance_id): """suspend the instance with instance_id""" instance = self.db.instance_get_by_internal_id(context, instance_id) @@ -343,7 +294,6 @@ class ComputeAPI(base.Base): {"method": "suspend_instance", "args": {"instance_id": instance['id']}}) - @checks_instance_lock def resume(self, context, instance_id): """resume the instance with instance_id""" instance = self.db.instance_get_by_internal_id(context, instance_id) @@ -353,7 +303,6 @@ class ComputeAPI(base.Base): {"method": "resume_instance", "args": {"instance_id": instance['id']}}) - @checks_instance_lock def rescue(self, context, instance_id): """Rescue the given instance.""" instance = self.db.instance_get_by_internal_id(context, instance_id) @@ -363,7 +312,6 @@ class ComputeAPI(base.Base): {"method": "rescue_instance", "args": {"instance_id": instance['id']}}) - @checks_instance_lock def unrescue(self, context, instance_id): """Unrescue the given instance.""" instance = self.db.instance_get_by_internal_id(context, instance_id) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 9a33c7cac..224159596 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -36,6 +36,7 @@ terminating it. import datetime import logging +import functools from nova import exception from nova import flags @@ -53,6 +54,48 @@ flags.DEFINE_string('stub_network', False, 'Stub network related code') +def checks_instance_lock(function): + """ + decorator used for preventing action against locked instances + unless, of course, you happen to be admin + + """ + + @functools.wraps(function) + def decorated_function(*args, **kwargs): + + logging.info(_("check_instance_locks decorating |%s|"), function) + logging.info(_("check_instance_locks: arguments: |%s| |%s|"), args, + kwargs) + # assume worst case (have to declare so they are in scope) + admin = False + locked = True + + # grab args to function + try: + if 'context' in kwargs: + context = kwargs['context'] + else: + context = args[1] + if 'instance_id' in kwargs: + instance_id = kwargs['instance_id'] + else: + instance_id = args[2] + locked = args[0].get_locked(context, instance_id) + admin = context.is_admin + except Exception as e: + logging.error(_("check_instance_lock: arguments: |%s| |%s|"), args, + kwargs) + raise e + + # if admin or unlocked call function, otherwise 405 + if admin or not locked: + return function(*args, **kwargs) + raise Exception(_("Instance is locked, cannot execute |%s|"), function) + + return decorated_function + + class ComputeManager(manager.Manager): """Manages the running instances from creation to destruction.""" @@ -158,6 +201,7 @@ class ComputeManager(manager.Manager): self._update_state(context, instance_id) @exception.wrap_exception + @checks_instance_lock def terminate_instance(self, context, instance_id): """Terminate an instance on this machine.""" context = context.elevated() @@ -202,6 +246,7 @@ class ComputeManager(manager.Manager): self.db.instance_destroy(context, instance_id) @exception.wrap_exception + @checks_instance_lock def reboot_instance(self, context, instance_id): """Reboot an instance on this server.""" context = context.elevated() @@ -225,6 +270,7 @@ class ComputeManager(manager.Manager): self._update_state(context, instance_id) @exception.wrap_exception + @checks_instance_lock def rescue_instance(self, context, instance_id): """Rescue an instance on this server.""" context = context.elevated() @@ -241,6 +287,7 @@ class ComputeManager(manager.Manager): self._update_state(context, instance_id) @exception.wrap_exception + @checks_instance_lock def unrescue_instance(self, context, instance_id): """Rescue an instance on this server.""" context = context.elevated() @@ -261,6 +308,7 @@ class ComputeManager(manager.Manager): self._update_state(context, instance_id) @exception.wrap_exception + @checks_instance_lock def pause_instance(self, context, instance_id): """Pause an instance on this server.""" context = context.elevated() @@ -279,6 +327,7 @@ class ComputeManager(manager.Manager): result)) @exception.wrap_exception + @checks_instance_lock def unpause_instance(self, context, instance_id): """Unpause a paused instance on this server.""" context = context.elevated() @@ -297,6 +346,7 @@ class ComputeManager(manager.Manager): result)) @exception.wrap_exception + @checks_instance_lock def suspend_instance(self, context, instance_id): """suspend the instance with instance_id""" context = context.elevated() @@ -314,6 +364,7 @@ class ComputeManager(manager.Manager): result)) @exception.wrap_exception + @checks_instance_lock def resume_instance(self, context, instance_id): """resume the suspended instance with instance_id""" context = context.elevated() @@ -353,6 +404,18 @@ class ComputeManager(manager.Manager): logging.debug(_('instance %s: unlocking'), instance_ref['internal_id']) self.db.instance_update(context, instance_id, {'locked': False}) + @exception.wrap_exception + def get_locked(self, context, instance_id): + """ + return the boolean state of (instance with instance_id)'s lock + + """ + context = context.elevated() + logging.debug(_('instance %s: getting locked'), + instance_ref['internal_id']) + instance_ref = self.db.instance_get(context, instance_id) + return instance_ref['locked'] + @exception.wrap_exception def get_console_output(self, context, instance_id): """Send the console output for an instance.""" @@ -363,6 +426,7 @@ class ComputeManager(manager.Manager): return self.driver.get_console_output(instance_ref) @exception.wrap_exception + @checks_instance_lock def attach_volume(self, context, instance_id, volume_id, mountpoint): """Attach a volume to an instance.""" context = context.elevated() @@ -392,6 +456,7 @@ class ComputeManager(manager.Manager): return True @exception.wrap_exception + @checks_instance_lock def detach_volume(self, context, instance_id, volume_id): """Detach a volume from an instance.""" context = context.elevated() -- cgit From 656233762a61929d43f671e4765d52f25299562f Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Wed, 29 Dec 2010 20:52:48 -0600 Subject: syntax error --- nova/compute/manager.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'nova/compute') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 224159596..cd2c95d8d 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -411,8 +411,7 @@ class ComputeManager(manager.Manager): """ context = context.elevated() - logging.debug(_('instance %s: getting locked'), - instance_ref['internal_id']) + logging.debug(_('instance %s: getting locked state'), instance_id) instance_ref = self.db.instance_get(context, instance_id) return instance_ref['locked'] -- cgit From 4531600425d71659581aa549bdc5e719e41efc9e Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Wed, 29 Dec 2010 22:08:38 -0600 Subject: altered the compute lock test --- nova/compute/manager.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'nova/compute') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index cd2c95d8d..2671d401c 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -70,6 +70,7 @@ def checks_instance_lock(function): # assume worst case (have to declare so they are in scope) admin = False locked = True + instance_id = False # grab args to function try: @@ -81,17 +82,20 @@ def checks_instance_lock(function): instance_id = kwargs['instance_id'] else: instance_id = args[2] - locked = args[0].get_locked(context, instance_id) + locked = args[0].get_lock(context, instance_id) admin = context.is_admin except Exception as e: - logging.error(_("check_instance_lock: arguments: |%s| |%s|"), args, + logging.error(_("check_instance_lock fail! args: |%s| |%s|"), args, kwargs) raise e - # if admin or unlocked call function, otherwise 405 + # if admin or unlocked call function otherwise log error if admin or not locked: - return function(*args, **kwargs) - raise Exception(_("Instance is locked, cannot execute |%s|"), function) + function(*args, **kwargs) + else: + logging.error(_("Instance |%s| is locked, cannot execute |%s|"), + instance_id, function) + return False return decorated_function @@ -405,7 +409,7 @@ class ComputeManager(manager.Manager): self.db.instance_update(context, instance_id, {'locked': False}) @exception.wrap_exception - def get_locked(self, context, instance_id): + def get_lock(self, context, instance_id): """ return the boolean state of (instance with instance_id)'s lock -- cgit From 006a3c43093ce3324173e0aed172a3be1396d5dc Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Thu, 6 Jan 2011 16:21:59 -0600 Subject: altered argument handling --- nova/compute/manager.py | 44 +++++++++++++++----------------------------- 1 file changed, 15 insertions(+), 29 deletions(-) (limited to 'nova/compute') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 2671d401c..a5343ff22 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -62,39 +62,25 @@ def checks_instance_lock(function): """ @functools.wraps(function) - def decorated_function(*args, **kwargs): - - logging.info(_("check_instance_locks decorating |%s|"), function) - logging.info(_("check_instance_locks: arguments: |%s| |%s|"), args, - kwargs) - # assume worst case (have to declare so they are in scope) - admin = False - locked = True - instance_id = False - - # grab args to function - try: - if 'context' in kwargs: - context = kwargs['context'] - else: - context = args[1] - if 'instance_id' in kwargs: - instance_id = kwargs['instance_id'] - else: - instance_id = args[2] - locked = args[0].get_lock(context, instance_id) - admin = context.is_admin - except Exception as e: - logging.error(_("check_instance_lock fail! args: |%s| |%s|"), args, - kwargs) - raise e + def decorated_function(self, context, instance_id, *args, **kwargs): + + logging.info(_("check_instance_lock: decorating: |%s|"), function) + logging.info(_("check_instance_lock: arguments: |%s| |%s| |%s|"), + self, + context, + instance_id) + unlocked = not self.get_lock(context, instance_id) + admin = context.is_admin + logging.info(_("check_instance_lock: locked: |%s|"), locked) + logging.info(_("check_instance_lock: admin: |%s|"), admin) # if admin or unlocked call function otherwise log error - if admin or not locked: + if admin or unlocked: + logging.info(_("check_instance_lock: executing: |%s|"), function) function(*args, **kwargs) else: - logging.error(_("Instance |%s| is locked, cannot execute |%s|"), - instance_id, function) + logging.error(_("check_instance_lock: not executing |%s|"), + function) return False return decorated_function -- cgit From 11d6e9d2f917d124946d0fa47c1512a1f8ab940d Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Thu, 6 Jan 2011 17:02:20 -0600 Subject: accidentally left unlocked in there, it should have been locked --- nova/compute/manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'nova/compute') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 1ed1eb40a..3c20b7c73 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -69,13 +69,13 @@ def checks_instance_lock(function): self, context, instance_id) - unlocked = not self.get_lock(context, instance_id) + locked = self.get_lock(context, instance_id) admin = context.is_admin logging.info(_("check_instance_lock: locked: |%s|"), locked) logging.info(_("check_instance_lock: admin: |%s|"), admin) # if admin or unlocked call function otherwise log error - if admin or unlocked: + if admin or not locked: logging.info(_("check_instance_lock: executing: |%s|"), function) function(*args, **kwargs) else: -- cgit From 7450f84e0f491f8a24273135432e105677c4a589 Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Thu, 6 Jan 2011 17:23:00 -0600 Subject: passing the correct parameters to decorated function --- nova/compute/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nova/compute') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 3c20b7c73..b7ec2e93b 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -77,7 +77,7 @@ def checks_instance_lock(function): # if admin or unlocked call function otherwise log error if admin or not locked: logging.info(_("check_instance_lock: executing: |%s|"), function) - function(*args, **kwargs) + function(self, context, isntance_id, *args, **kwargs) else: logging.error(_("check_instance_lock: not executing |%s|"), function) -- cgit From ebec92778bdaf4af58029f9977697865c53f881d Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Thu, 6 Jan 2011 17:25:17 -0600 Subject: refers to instance_id instead of instance_ref[instance_id] --- nova/compute/manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'nova/compute') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index b7ec2e93b..24c321130 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -411,7 +411,7 @@ class ComputeManager(manager.Manager): context = context.elevated() instance_ref = self.db.instance_get(context, instance_id) - logging.debug(_('instance %s: locking'), instance_ref['internal_id']) + logging.debug(_('instance %s: locking'), instance_id) self.db.instance_update(context, instance_id, {'locked': True}) @exception.wrap_exception @@ -423,7 +423,7 @@ class ComputeManager(manager.Manager): context = context.elevated() instance_ref = self.db.instance_get(context, instance_id) - logging.debug(_('instance %s: unlocking'), instance_ref['internal_id']) + logging.debug(_('instance %s: unlocking'), instance_id) self.db.instance_update(context, instance_id, {'locked': False}) @exception.wrap_exception -- cgit From 76e3923c40dff2f754b045847d8ad19ea9a7cef1 Mon Sep 17 00:00:00 2001 From: Trey Morris Date: Thu, 6 Jan 2011 17:27:57 -0600 Subject: typo --- nova/compute/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nova/compute') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 24c321130..d5136eb26 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -77,7 +77,7 @@ def checks_instance_lock(function): # if admin or unlocked call function otherwise log error if admin or not locked: logging.info(_("check_instance_lock: executing: |%s|"), function) - function(self, context, isntance_id, *args, **kwargs) + function(self, context, instance_id, *args, **kwargs) else: logging.error(_("check_instance_lock: not executing |%s|"), function) -- cgit