From: Abhijeet Malawade Date: Thu, 22 Jan 2015 13:53:42 +0000 (-0800) Subject: Check volume status in detach db api X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=909a4eb2fd5411f11e57c2589fc786611cdeda57;p=openstack-build%2Fcinder-build.git Check volume status in detach db api If a volume is detached while uploading volume to image is in progress then it sets volume status to available and then it is allowed to delete it immediately while uploading is still running in background. Set the status to available in the detach db api only when the volume is not migrating and status is not uploading. Closes-Bug: 1413887 Change-Id: I6df9c6918825fba7c8d7cd6521752b399dd61610 --- diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 432f59c02..9ae7304df 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -1212,7 +1212,9 @@ def volume_detached(context, volume_id, attachment_id): volume_ref = _volume_get(context, volume_id, session=session) if not remain_attachment: # Hide status update from user if we're performing volume migration - if not volume_ref['migration_status']: + # or uploading it to image + if (not volume_ref['migration_status'] and + not (volume_ref['status'] == 'uploading')): volume_ref['status'] = 'available' volume_ref['attach_status'] = 'detached' diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py index 7a0e5e90e..511bfc5d2 100644 --- a/cinder/tests/test_volume.py +++ b/cinder/tests/test_volume.py @@ -2353,6 +2353,31 @@ class VolumeTestCase(BaseVolumeTestCase): self.assertEqual('readonly', admin_metadata[0]['key']) self.assertEqual('True', admin_metadata[0]['value']) + def test_detach_volume_while_uploading_to_image_is_in_progress(self): + # If instance is booted from volume with 'Terminate on Delete' flag + # set, and when we delete instance then it tries to delete volume + # even it is in 'uploading' state. + # It is happening because detach call is setting volume status to + # 'available'. + mountpoint = "/dev/sdf" + # Attach volume to the instance + instance_uuid = '12345678-1234-5678-1234-567812345678' + volume = tests_utils.create_volume(self.context, + admin_metadata={'readonly': 'True'}, + **self.volume_params) + volume_id = volume['id'] + self.volume.create_volume(self.context, volume_id) + self.volume.attach_volume(self.context, volume_id, instance_uuid, + None, mountpoint, 'ro') + # Change volume status to 'uploading' + db.volume_update(self.context, volume_id, {'status': 'uploading'}) + # Call detach api + self.volume.detach_volume(self.context, volume_id) + vol = db.volume_get(self.context, volume_id) + # Check that volume status is 'uploading' + self.assertEqual("uploading", vol['status']) + self.assertEqual("detached", vol['attach_status']) + @mock.patch.object(cinder.volume.api.API, 'update') @mock.patch.object(db, 'volume_get') def test_reserve_volume_success(self, volume_get, volume_update):