diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index 227b1b53042..82b3f0a2e3d 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -3497,7 +3497,7 @@ class VolumeTestCase(BaseVolumeTestCase): """Test function of create_volume_from_image. Test cases call this function to create a volume from image, caller - can choose whether to fake out copy_image_to_volume and conle_image, + can choose whether to fake out copy_image_to_volume and clone_image, after calling this, test cases should check status of the volume. """ def fake_local_path(volume): @@ -3523,7 +3523,7 @@ class VolumeTestCase(BaseVolumeTestCase): self.stubs.Set(self.volume.driver, 'clone_image', fake_clone_image) self.stubs.Set(image_utils, 'fetch_to_raw', fake_fetch_to_raw) if fakeout_copy_image_to_volume: - self.stubs.Set(self.volume, '_copy_image_to_volume', + self.stubs.Set(self.volume.driver, 'copy_image_to_volume', fake_copy_image_to_volume) mock_clone_image_volume.return_value = ({}, clone_image_volume) mock_fetch_img.return_value = mock.MagicMock( @@ -3622,6 +3622,32 @@ class VolumeTestCase(BaseVolumeTestCase): self.assertDictEqual(self.volume.stats['pools'], {'_pool0': {'allocated_capacity_gb': 1}}) + @mock.patch('cinder.utils.brick_get_connector_properties') + @mock.patch('cinder.utils.brick_get_connector') + @mock.patch('cinder.volume.driver.BaseVD.secure_file_operations_enabled') + @mock.patch('cinder.volume.driver.BaseVD._detach_volume') + def test_create_volume_from_image_unavailable(self, mock_detach, + mock_secure, *args): + """Test create volume with ImageCopyFailure + + We'll raise an exception inside _connect_device after volume has + already been attached to confirm that it detaches the volume. + """ + mock_secure.side_effect = NameError + + # We want to test BaseVD copy_image_to_volume and since FakeISCSIDriver + # inherits from LVM it overwrites it, so we'll mock it to use the + # BaseVD implementation. + unbound_copy_method = cinder.volume.driver.BaseVD.copy_image_to_volume + bound_copy_method = unbound_copy_method.__get__(self.volume.driver) + with mock.patch.object(self.volume.driver, 'copy_image_to_volume', + side_effect=bound_copy_method): + self.assertRaises(exception.ImageCopyFailure, + self._create_volume_from_image, + fakeout_copy_image_to_volume=False) + # We must have called detach method. + self.assertEqual(1, mock_detach.call_count) + def test_create_volume_from_image_clone_image_volume(self): """Test create volume from image via image volume. diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index afce7a33638..707238c309b 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -963,7 +963,26 @@ class BaseVD(object): LOG.error(err_msg) raise exception.VolumeBackendAPIException(data=ex_msg) raise exception.VolumeBackendAPIException(data=err_msg) - return (self._connect_device(conn), volume) + + try: + attach_info = self._connect_device(conn) + except exception.DeviceUnavailable as exc: + # We may have reached a point where we have attached the volume, + # so we have to detach it (do the cleanup). + attach_info = exc.kwargs.get('attach_info', None) + if attach_info: + try: + LOG.debug('Device for volume %s is unavailable but did ' + 'attach, detaching it.', volume['id']) + self._detach_volume(context, attach_info, volume, + properties, force=True, + remote=remote) + except Exception: + LOG.exception(_LE('Error detaching volume %s'), + volume['id']) + raise + + return (attach_info, volume) def _attach_snapshot(self, context, snapshot, properties, remote=False): """Attach the snapshot.""" @@ -1036,17 +1055,26 @@ class BaseVD(object): device = connector.connect_volume(conn['data']) host_device = device['path'] - # Secure network file systems will NOT run as root. - root_access = not self.secure_file_operations_enabled() + attach_info = {'conn': conn, 'device': device, 'connector': connector} - if not connector.check_valid_device(host_device, root_access): + unavailable = True + try: + # Secure network file systems will NOT run as root. + root_access = not self.secure_file_operations_enabled() + unavailable = not connector.check_valid_device(host_device, + root_access) + except Exception: + LOG.exception(_LE('Could not validate device %s'), host_device) + + if unavailable: raise exception.DeviceUnavailable(path=host_device, + attach_info=attach_info, reason=(_("Unable to access " "the backend storage " "via the path " "%(path)s.") % {'path': host_device})) - return {'conn': conn, 'device': device, 'connector': connector} + return attach_info def clone_image(self, context, volume, image_location, image_meta,