diff --git a/cinder/tests/unit/volume/test_volume.py b/cinder/tests/unit/volume/test_volume.py index f88af88f0f9..7b1d39d7dd7 100644 --- a/cinder/tests/unit/volume/test_volume.py +++ b/cinder/tests/unit/volume/test_volume.py @@ -1859,6 +1859,24 @@ class VolumeTestCase(base.BaseVolumeTestCase): self.assertIn("status must be available or downloading", six.text_type(ex)) + def test_attachment_reserve_with_instance_uuid_error_volume(self): + # Tests that trying to create an attachment (with an instance_uuid + # provided) on a volume that's not 'available' or 'downloading' status + # will fail if the volume does not have any attachments, similar to how + # the volume reserve action works. + volume = tests_utils.create_volume(self.context, status='error') + # Assert that we're not dealing with a multiattach volume and that + # it does not have any existing attachments. + self.assertFalse(volume.multiattach) + self.assertEqual(0, len(volume.volume_attachment)) + # Try attaching an instance to the volume which should fail based on + # the volume status. + ex = self.assertRaises(exception.InvalidVolume, + self.volume_api._attachment_reserve, + self.context, volume, fake.UUID1) + self.assertIn("status must be available or downloading", + six.text_type(ex)) + def test_unreserve_volume_success_in_use(self): UUID = six.text_type(uuid.uuid4()) volume = tests_utils.create_volume(self.context, status='attaching') diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 0d1647934fb..d6c6699b18b 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -2086,18 +2086,19 @@ class API(base.Base): result = vref.conditional_update({'status': 'reserved'}, expected) if not result: - # Make sure we're not going to the same instance, in which case - # it could be a live-migrate or similar scenario (LP BUG: 1694530) override = False if instance_uuid: - override = True # Refresh the volume reference in case multiple instances were # being concurrently attached to the same non-multiattach # volume. vref = objects.Volume.get_by_id(ctxt, vref.id) for attachment in vref.volume_attachment: - if attachment.instance_uuid != instance_uuid: - override = False + # If we're attaching the same volume to the same instance, + # we could be migrating the instance to another host in + # which case we want to allow the reservation. + # (LP BUG: 1694530) + if attachment.instance_uuid == instance_uuid: + override = True break if not override: