From: John Griffith Date: Wed, 22 Apr 2015 04:28:34 +0000 (+0000) Subject: Remove force check from copy_volume_to_image X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=fb2c434c2461a25193067104de766fc1142a6024;p=openstack-build%2Fcinder-build.git Remove force check from copy_volume_to_image The upload_volume_to_image method allows a force parameter that will upload a volume even though the volume is attached/in-use. A user can get away with this with the LVM driver because the LVM backing is local to the Cinder worker node that's pushing the bits to Glance. The problem is, this only works for local storage, it won't work with any iSCSI devices because they can't do multi-attach. Also, the reason we required that a volume NOT be in-use for this operation is because we have no way of keeping the guest Instance from writing to the volume while we're uploading, and corrupting the data. This has been exposed like this for several releases, so removing it now likely would not be a good user experience. Instead, this patch add a config option to enable/disable it (default is to disable), and deployers can choose whether they would like to allow the use of --force True or not. DocImpact Disables the --force option to copy-volume-to-image and introduces "allow_force_upload" boolean option to re-enable Change-Id: I1210186689dea204601356f8d08805c6cb6f017c Closes-Bug: 1446954 --- diff --git a/cinder/tests/unit/api/contrib/test_volume_actions.py b/cinder/tests/unit/api/contrib/test_volume_actions.py index d59607c5d..b7a522518 100644 --- a/cinder/tests/unit/api/contrib/test_volume_actions.py +++ b/cinder/tests/unit/api/contrib/test_volume_actions.py @@ -513,7 +513,7 @@ class VolumeImageActionsTest(test.TestCase): "copy_volume_to_image", stub_upload_volume_to_image_service) - id = 1 + id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' vol = {"container_format": 'bare', "disk_format": 'raw', "image_name": 'image_name', @@ -540,7 +540,7 @@ class VolumeImageActionsTest(test.TestCase): self.stubs.Set(volume_api.API, 'get', stub_volume_get_raise_exc) - id = 1 + id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' vol = {"container_format": 'bare', "disk_format": 'raw', "image_name": 'image_name', @@ -561,7 +561,7 @@ class VolumeImageActionsTest(test.TestCase): "copy_volume_to_image", stub_upload_volume_to_image_service_raise) - id = 1 + id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' vol = {"container_format": 'bare', "disk_format": 'raw', "image_name": 'image_name', @@ -582,7 +582,7 @@ class VolumeImageActionsTest(test.TestCase): "copy_volume_to_image", stub_upload_volume_to_image_service_raise) - id = 1 + id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' vol = {"container_format": 'bare', "disk_format": 'raw', "image_name": 'image_name', @@ -603,7 +603,7 @@ class VolumeImageActionsTest(test.TestCase): "copy_volume_to_image", stub_upload_volume_to_image_service_raise) - id = 1 + id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' vol = {"container_format": 'bare', "disk_format": 'raw', "image_name": 'image_name', @@ -617,7 +617,7 @@ class VolumeImageActionsTest(test.TestCase): body) def test_volume_upload_image_typeerror(self): - id = 1 + id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' body = {"os-volume_upload_image_fake": "fake"} req = webob.Request.blank('/v2/tenant1/volumes/%s/action' % id) req.method = 'POST' @@ -627,7 +627,7 @@ class VolumeImageActionsTest(test.TestCase): self.assertEqual(res.status_int, 400) def test_volume_upload_image_without_type(self): - id = 1 + id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' vol = {"container_format": 'bare', "disk_format": 'raw', "image_name": None, @@ -641,7 +641,7 @@ class VolumeImageActionsTest(test.TestCase): self.assertEqual(res.status_int, 400) def test_extend_volume_valueerror(self): - id = 1 + id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' body = {'os-extend': {'new_size': 'fake'}} req = fakes.HTTPRequest.blank('/v2/tenant1/volumes/%s/action' % id) self.assertRaises(webob.exc.HTTPBadRequest, @@ -651,7 +651,7 @@ class VolumeImageActionsTest(test.TestCase): body) def test_copy_volume_to_image_notimagename(self): - id = 1 + id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' vol = {"container_format": 'bare', "disk_format": 'raw', "image_name": None, @@ -666,7 +666,7 @@ class VolumeImageActionsTest(test.TestCase): def test_copy_volume_to_image_with_protected_prop(self): """Test create image from volume with protected properties.""" - id = 1 + id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' def fake_get_volume_image_metadata(*args): meta_dict = { @@ -726,7 +726,7 @@ class VolumeImageActionsTest(test.TestCase): In this case volume glance metadata will not be available for this volume. """ - id = 1 + id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' def fake_get_volume_image_metadata_raise(*args): raise exception.GlanceMetadataNotFound(id=id) @@ -778,7 +778,7 @@ class VolumeImageActionsTest(test.TestCase): def test_copy_volume_to_image_without_protected_prop(self): """Test protected property is not defined with the root image.""" - id = 1 + id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' def fake_get_volume_image_metadata(*args): return [] @@ -831,7 +831,7 @@ class VolumeImageActionsTest(test.TestCase): def test_copy_volume_to_image_without_core_prop(self): """Test glance_core_properties defined in cinder.conf is empty.""" - id = 1 + id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' # Need to mock create, update, copy_volume_to_image with mock.patch.object(glance.GlanceImageService, "create") \ diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 86ad2e917..8a2af8f5f 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -51,6 +51,12 @@ from cinder.volume import utils as volume_utils from cinder.volume import volume_types +allow_force_upload = cfg.BoolOpt('enable_force_upload', + default=False, + help='Enables the Force option on ' + 'upload_to_image. This enables ' + 'running upload_volume on in-use ' + 'volumes for backends that support it.') volume_host_opt = cfg.BoolOpt('snapshot_same_host', default=True, help='Create volume from snapshot at the host ' @@ -66,6 +72,7 @@ az_cache_time_opt = cfg.IntOpt('az_cache_duration', 'seconds') CONF = cfg.CONF +CONF.register_opt(allow_force_upload) CONF.register_opt(volume_host_opt) CONF.register_opt(volume_same_az_opt) CONF.register_opt(az_cache_time_opt) @@ -1089,6 +1096,13 @@ class API(base.Base): @wrap_check_policy def copy_volume_to_image(self, context, volume, metadata, force): """Create a new image from the specified volume.""" + + if not CONF.enable_force_upload and force: + LOG.info(_LI("Force upload to image is disabled, " + "Force option will be ignored."), + resource={'type': 'volume', 'id': volume['id']}) + force = False + self._check_volume_availability(volume, force) glance_core_properties = CONF.glance_core_properties if glance_core_properties: