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 d59607c5d61241df7bbb610c94d356a6004aa3d8..b7a5225188c89548c205e5759a75772f78ff9bf1 100644 (file)
--- 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 86ad2e917b6523c898b5f4207e9b9f77f0cfafef..8a2af8f5f48645552e28e49e3bff07f3501ee1ba 100644 (file)
--- 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: