From d0cc37f8095bef38605930683af43af7ddb7e4a8 Mon Sep 17 00:00:00 2001 From: Liam Kelleher Date: Thu, 5 Jan 2012 15:37:18 +0000 Subject: [PATCH] Add attaching state for Volumes bug 897726 Add "attaching" status for volumes and clear state on error This closes a gap where multiple calls of attach for the same volume can happen before the volume is actually attached. Change-Id: I59d22ceda83729c1a455af9994c9ffec1912e23b --- Authors | 1 + nova/compute/api.py | 1 + nova/compute/manager.py | 17 +++++++++++++---- nova/tests/policy.json | 2 ++ nova/tests/test_compute.py | 6 +++++- nova/utils.py | 6 ++++++ nova/volume/api.py | 10 ++++++++++ 7 files changed, 38 insertions(+), 5 deletions(-) diff --git a/Authors b/Authors index ad6351b07e6c..888f3008fb7d 100644 --- a/Authors +++ b/Authors @@ -106,6 +106,7 @@ Kevin L. Mitchell Kiall Mac Innes Kirill Shileev Koji Iida +Liam Kelleher Likitha Shetty Loganathan Parthipan Lorin Hochstein diff --git a/nova/compute/api.py b/nova/compute/api.py index edadf19788d1..874dcae47859 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1594,6 +1594,7 @@ class API(base.Base): raise exception.InvalidDevicePath(path=device) volume = self.volume_api.get(context, volume_id) self.volume_api.check_attach(context, volume) + self.volume_api.reserve_volume(context, volume) params = {"volume_id": volume_id, "mountpoint": device} self._cast_compute_message('attach_volume', context, diff --git a/nova/compute/manager.py b/nova/compute/manager.py index f31a0d2e4a92..ee392f3e8aba 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1647,12 +1647,21 @@ class ComputeManager(manager.SchedulerDependentManager): context = context.elevated() instance_ref = self.db.instance_get_by_uuid(context, instance_uuid) instance_id = instance_ref['id'] + msg = _("instance %(instance_uuid)s: attaching volume %(volume_id)s" + " to %(mountpoint)s") LOG.audit(_('Attaching volume %(volume_id)s to %(mountpoint)s'), locals(), context=context, instance=instance_ref) - connector = self.driver.get_volume_connector(instance_ref) - connection_info = self.volume_api.initialize_connection(context, - volume, - connector) + try: + connector = self.driver.get_volume_connector(instance_ref) + connection_info = self.volume_api.initialize_connection(context, + volume, + connector) + except Exception: # pylint: disable=W0702 + with utils.save_and_reraise_exception(): + msg = _("instance %(instance_uuid)s: attach failed" + " %(mountpoint)s, removing") + LOG.exception(msg % locals(), context=context) + self.volume_api.unreserve_volume(context, volume) try: self.driver.attach_volume(connection_info, instance_ref['name'], diff --git a/nova/tests/policy.json b/nova/tests/policy.json index c231bff60a8f..d2e647f9f188 100644 --- a/nova/tests/policy.json +++ b/nova/tests/policy.json @@ -127,6 +127,8 @@ "volume:attach": [], "volume:detach": [], + "volume:reserve_volume": [], + "volume:unreserve_volume": [], "volume:check_attach": [], "volume:check_detach": [], "volume:initialize_connection": [], diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py index 30027eb67d63..f8f87cba7ffe 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -3049,12 +3049,16 @@ class ComputeAPITestCase(BaseTestCase): def fake_check_attach(*args, **kwargs): pass + def fake_reserve_volume(*args, **kwargs): + pass + def fake_volume_get(self, context, volume_id): return {'id': volume_id} self.stubs.Set(nova.volume.api.API, 'get', fake_volume_get) self.stubs.Set(nova.volume.api.API, 'check_attach', fake_check_attach) - + self.stubs.Set(nova.volume.api.API, 'reserve_volume', + fake_reserve_volume) instance = self._create_fake_instance() self.compute_api.attach_volume(self.context, instance, 1, '/dev/vdb') diff --git a/nova/utils.py b/nova/utils.py index 85f39dbe0734..172b68afa670 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -24,6 +24,7 @@ import datetime import functools import hashlib import inspect +import itertools import json import lockfile import os @@ -712,6 +713,11 @@ def to_primitive(value, convert_instances=False, level=0): if test(value): return unicode(value) + # value of itertools.count doesn't get caught by inspects + # above and results in infinite loop when list(value) is called. + if type(value) == itertools.count: + return unicode(value) + # FIXME(vish): Workaround for LP bug 852095. Without this workaround, # tests that raise an exception in a mocked method that # has a @wrap_exception with a notifier will fail. If diff --git a/nova/volume/api.py b/nova/volume/api.py index 8646f65e99e0..4719f4f1e49a 100644 --- a/nova/volume/api.py +++ b/nova/volume/api.py @@ -228,6 +228,15 @@ class API(base.Base): "args": {'instance_id': instance_id, 'volume_id': volume['id']}}) + @wrap_check_policy + def reserve_volume(self, context, volume): + self.update(context, volume, {"status": "attaching"}) + + @wrap_check_policy + def unreserve_volume(self, context, volume): + if volume['status'] == "attaching": + self.update(context, volume, {"status": "available"}) + @wrap_check_policy def attach(self, context, volume, instance_id, mountpoint): host = volume['host'] @@ -257,6 +266,7 @@ class API(base.Base): @wrap_check_policy def terminate_connection(self, context, volume, connector): + self.unreserve_volume(context, volume) host = volume['host'] queue = self.db.queue_get_for(context, FLAGS.volume_topic, host) return rpc.call(context, queue,