Merge "Prevent rescuing a VM with a partially mounted volume."

4 files changed, 43 insertions, 1 deletions

diff --git a/nova/api/openstack/compute/contrib/rescue.py b/nova/api/openstack/compute/contrib/rescue.py
index 143b59063..c89d11117 100644
--- a/nova/api/openstack/compute/contrib/rescue.py
+++ b/nova/api/openstack/compute/contrib/rescue.py
@@ -60,6 +60,8 @@ class RescueController(wsgi.Controller):
         except exception.InstanceInvalidState as state_error:
             common.raise_http_conflict_for_instance_invalid_state(state_error,
                                                                   'rescue')
+        except exception.InvalidVolume as volume_error:
+            raise exc.HTTPConflict(explanation=volume_error.format_message())
         except exception.InstanceNotRescuable as non_rescuable:
             raise exc.HTTPBadRequest(
                 explanation=non_rescuable.format_message())
diff --git a/nova/compute/api.py b/nova/compute/api.py
index ac52b805d..f4560bd0f 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -2148,6 +2148,12 @@ class API(base.Base):
     @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED])
     def rescue(self, context, instance, rescue_password=None):
         """Rescue the given instance."""
+
+        bdms = self.get_instance_bdms(context, instance)
+        for bdm in bdms:
+            if bdm['volume_id']:
+                volume = self.volume_api.get(context, bdm['volume_id'])
+                self.volume_api.check_attached(context, volume)
         # TODO(ndipanov): This check can be generalized as a decorator to
         # check for valid combinations of src and dests - for now check
         # if it's booted from volume only
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index bd253b8ee..cd70e0cb2 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -5645,11 +5645,17 @@ class ComputeAPITestCase(BaseTestCase):
         volume_backed_uuid_2 = volume_backed_inst_2['uuid']
 
         def fake_get_instance_bdms(*args, **kwargs):
-            return [{'device_name': '/dev/vda'}]
+            return [{'device_name': '/dev/vda',
+                     'volume_id':'bf0b6b00-a20c-11e2-9e96-0800200c9a66'}]
 
         self.stubs.Set(self.compute_api, 'get_instance_bdms',
                        fake_get_instance_bdms)
 
+        def fake_volume_get(self, context, volume_id):
+            return {'id': volume_id, 'status': 'in-use'}
+
+        self.stubs.Set(cinder.API, 'get', fake_volume_get)
+
         self.compute.run_instance(self.context,
                                   instance=volume_backed_inst_1)
         self.compute.run_instance(self.context,
@@ -6900,6 +6906,28 @@ class ComputeAPITestCase(BaseTestCase):
                 self.compute_api.detach_volume,
                 self.context, instance, volume)
 
+    def test_no_rescue_in_volume_state_attaching(self):
+        # Make sure a VM cannot be rescued while volume is being attached
+        instance = self._create_fake_instance()
+
+        def fake_get_instance_bdms(*args, **kwargs):
+            return [{'device_name': '/dev/vda',
+                     'volume_id':'bf0b6b00-a20c-11e2-9e96-0800200c9a66'}]
+
+        self.stubs.Set(self.compute_api, 'get_instance_bdms',
+                       fake_get_instance_bdms)
+
+        def fake_volume_get(self, context, volume_id):
+            return {'id': volume_id, 'status': 'attaching'}
+
+        self.stubs.Set(cinder.API, 'get', fake_volume_get)
+
+        volume = {'id': 'bf0b6b00-a20c-11e2-9e96-0800200c9a66',
+                  'state': 'active', 'instance_uuid': instance['uuid']}
+
+        self.assertRaises(exception.InvalidVolume,
+                self.compute_api.rescue, self.context, instance)
+
     def test_vnc_console(self):
         # Make sure we can a vnc console for an instance.
 
diff --git a/nova/volume/cinder.py b/nova/volume/cinder.py
index 0d2666038..de4948928 100644
--- a/nova/volume/cinder.py
+++ b/nova/volume/cinder.py
@@ -213,6 +213,12 @@ class API(base.Base):
 
         return rval
 
+    def check_attached(self, context, volume):
+        """Raise exception if volume in use."""
+        if volume['status'] != "in-use":
+            msg = _("status must be 'in-use'")
+            raise exception.InvalidVolume(reason=msg)
+
     def check_attach(self, context, volume, instance=None):
         # TODO(vish): abstract status checking?
         if volume['status'] != "available":