From 909a4eb2fd5411f11e57c2589fc786611cdeda57 Mon Sep 17 00:00:00 2001 From: Abhijeet Malawade Date: Thu, 22 Jan 2015 05:53:42 -0800 Subject: [PATCH] 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 --- cinder/db/sqlalchemy/api.py | 4 +++- cinder/tests/test_volume.py | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 432f59c02f3..9ae7304dfa3 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 7a0e5e90e14..511bfc5d25c 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):