From: Abhijeet Malawade <Abhijeet.Malawade@nttdata.com>
Date: Mon, 22 Dec 2014 13:50:28 +0000 (-0800)
Subject: Get volume from db again before updating it's status
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=4d6ddcb94435a20476536f4956de88ce4ab15b66;p=openstack-build%2Fcinder-build.git
Get volume from db again before updating it's status
During uploading a volume which is attached to an instance to image service,
if the instance is deleted then the volume remains in "in-use" status.
So get the volume again from db before updating it's status.
Added new db api called 'volume_update_status_based_on_attachment'.
It gets volume from db and updates its status using the same session.
Closes-Bug: #1406703
Change-Id: I01d313438f021cd2fe0d6f5c23f2029279d4b55a
---
diff --git a/cinder/db/api.py b/cinder/db/api.py
index 1717631a4..f0726fea9 100644
--- a/cinder/db/api.py
+++ b/cinder/db/api.py
@@ -262,6 +262,11 @@ def volume_attachment_get_by_instance_uuid(context, volume_id, instance_uuid):
instance_uuid)
+def volume_update_status_based_on_attachment(context, volume_id):
+ """Update volume status according to attached instance id"""
+ return IMPL.volume_update_status_based_on_attachment(context, volume_id)
+
+
####################
diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index 5fc00640b..4f2b36e80 100644
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -1686,8 +1686,34 @@ def volume_attachment_update(context, attachment_id, values):
return volume_attachment_ref
+def volume_update_status_based_on_attachment(context, volume_id):
+ """Update volume status based on attachment.
+
+ Get volume and check if 'volume_attachment' parameter is present in volume.
+ If 'volume_attachment' is None then set volume status to 'available'
+ else set volume status to 'in-use'.
+
+ :param context: context to query under
+ :param volume_id: id of volume to be updated
+ :returns: updated volume
+ """
+ session = get_session()
+ with session.begin():
+ volume_ref = _volume_get(context, volume_id, session=session)
+ # We need to get and update volume using same session because
+ # there is possibility that instance is deleted between the 'get'
+ # and 'update' volume call.
+ if not volume_ref['volume_attachment']:
+ volume_ref.update({'status': 'available'})
+ else:
+ volume_ref.update({'status': 'in-use'})
+
+ return volume_ref
+
+
####################
+
def _volume_x_metadata_get_query(context, volume_id, model, session=None):
return model_query(context, model, session=session, read_deleted="no").\
filter_by(volume_id=volume_id)
diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index 68ca32ca1..818d3deac 100644
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -4576,6 +4576,46 @@ class CopyVolumeToImageTestCase(BaseVolumeTestCase):
volume = db.volume_get(self.context, self.volume_id)
self.assertEqual(volume['status'], 'available')
+ def test_copy_volume_to_image_instance_deleted(self):
+ # During uploading volume to image if instance is deleted,
+ # volume should be in available status.
+ self.image_meta['id'] = 'a440c04b-79fa-479c-bed1-0b816eaec379'
+ # Creating volume testdata
+ self.volume_attrs['instance_uuid'] = 'b21f957d-a72f-4b93-b5a5-' \
+ '45b1161abb02'
+ db.volume_create(self.context, self.volume_attrs)
+
+ # Storing unmocked db api function reference here, because we have to
+ # update volume status (set instance_uuid to None) before calling the
+ # 'volume_update_status_based_on_attached_instance_id' db api.
+ unmocked_db_api = db.volume_update_status_based_on_attachment
+
+ def mock_volume_update_after_upload(context, volume_id):
+ # First update volume and set 'instance_uuid' to None
+ # because after deleting instance, instance_uuid of volume is
+ # set to None
+ db.volume_update(context, volume_id, {'instance_uuid': None})
+ # Calling unmocked db api
+ unmocked_db_api(context, volume_id)
+
+ with mock.patch.object(
+ db,
+ 'volume_update_status_based_on_attachment',
+ side_effect=mock_volume_update_after_upload) as mock_update:
+
+ # Start test
+ self.volume.copy_volume_to_image(self.context,
+ self.volume_id,
+ self.image_meta)
+ # Check 'volume_update_status_after_copy_volume_to_image'
+ # is called 1 time
+ self.assertEqual(1, mock_update.call_count)
+
+ # Check volume status has changed to available because
+ # instance is deleted
+ volume = db.volume_get(self.context, self.volume_id)
+ self.assertEqual('available', volume['status'])
+
def test_copy_volume_to_image_status_use(self):
self.image_meta['id'] = 'a440c04b-79fa-479c-bed1-0b816eaec379'
# creating volume testdata
diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py
index bd728d1e5..2dd0823d9 100644
--- a/cinder/volume/manager.py
+++ b/cinder/volume/manager.py
@@ -938,12 +938,8 @@ class VolumeManager(manager.SchedulerDependentManager):
with excutils.save_and_reraise_exception():
payload['message'] = unicode(error)
finally:
- if not volume['volume_attachment']:
- self.db.volume_update(context, volume_id,
- {'status': 'available'})
- else:
- self.db.volume_update(context, volume_id,
- {'status': 'in-use'})
+ self.db.volume_update_status_based_on_attachment(context,
+ volume_id)
def _delete_image(self, context, image_id, image_service):
"""Deletes an image stuck in queued or saving state."""