From 566a09c322039c27663f71192bb30fca44b7987e Mon Sep 17 00:00:00 2001 From: Vincent Hou Date: Tue, 1 Sep 2015 11:17:04 +0800 Subject: [PATCH] Remove the destination volume check in delete_volume * If the volume migration fails in the middle of the progress, the destination volume will be cleaned up via rpcapi.delete_volume. The method delete_volume in manager should allow us to clean up the database record for the destination volume. * The variable update in migrate_volume_completion has been assigned twice. Remove the second one. Change-Id: Icb815476943053f1487268b33c28f3575c4fd0dc Closes-Bug: #1490445 --- cinder/tests/unit/test_volume.py | 21 +++++++++++++++++++++ cinder/volume/manager.py | 26 ++++++-------------------- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index 227b1b530..e060d8d9f 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -4801,6 +4801,27 @@ class VolumeMigrationTestCase(VolumeTestCase): self.volume.driver._initialized = True self.volume.delete_volume(self.context, volume['id']) + def test_delete_source_volume_in_migration(self): + """Test deleting a source volume that is in migration.""" + self._test_delete_volume_in_migration('migrating') + + def test_delete_destination_volume_in_migration(self): + """Test deleting a destination volume that is in migration.""" + self._test_delete_volume_in_migration('target:vol-id') + + def _test_delete_volume_in_migration(self, migration_status): + """Test deleting a volume that is in migration.""" + volume = tests_utils.create_volume(self.context, **self.volume_params) + volume = db.volume_update(self.context, volume['id'], + {'status': 'available', + 'migration_status': migration_status}) + self.volume.delete_volume(self.context, volume['id']) + + # The volume is successfully removed during the volume delete + # and won't exist in the database any more. + self.assertRaises(exception.VolumeNotFound, db.volume_get, + self.context, volume['id']) + class ConsistencyGroupTestCase(BaseVolumeTestCase): def test_delete_volume_in_consistency_group(self): diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 7a7d0cc18..d5d463236 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -579,15 +579,9 @@ class VolumeManager(manager.SchedulerDependentManager): 1. Delete a volume(normal case) Delete a volume and update quotas. - 2. Delete a migration source volume - If deleting the source volume in a migration, we want to skip - quotas. Also we want to skip other database updates for source - volume because these update will be handled at - migrate_volume_completion properly. - - 3. Delete a migration destination volume - If deleting the destination volume in a migration, we want to - skip quotas but we need database updates for the volume. + 2. Delete a migration volume + If deleting the volume in a migration, we want to skip + quotas but we need database updates for the volume. """ context = context.elevated() @@ -666,15 +660,10 @@ class VolumeManager(manager.SchedulerDependentManager): LOG.exception(_LE("Failed to update usages deleting volume."), resource=volume_ref) - # If deleting the destination volume in a migration, we should skip - # database update here. In other cases, continue to update database - # entries. - if not is_migrating_dest: + # Delete glance metadata if it exists + self.db.volume_glance_metadata_delete_by_volume(context, volume_id) - # Delete glance metadata if it exists - self.db.volume_glance_metadata_delete_by_volume(context, volume_id) - - self.db.volume_destroy(context, volume_id) + self.db.volume_destroy(context, volume_id) # If deleting source/destination volume in a migration, we should # skip quotas. @@ -1784,9 +1773,6 @@ class VolumeManager(manager.SchedulerDependentManager): LOG.error(_LE('Failed to request async delete of migration source ' 'vol %(vol)s: %(err)s'), {'vol': volume_id, 'err': ex}) - updates = {'migration_status': 'success', - 'status': orig_volume_status, - 'previous_status': volume['status']} LOG.info(_LI("Complete-Migrate volume completed successfully."), resource=volume) -- 2.45.2