Fix extend_volume error handling.

If the async call to the manager/driver failed, the API still updated
the quota and volume size in the DB. Solution is to move these tasks
down to the manager, where we know if the extend succeeded.

Change-Id: I668fd659830bd6d410be64a1f5116377b08a9e96
Fixes: bug 1201814

cinder/tests/test_volume.py

@@ -1228,7 +1228,7 @@ def test_snapshot_get_active_by_window(self):
         self.assertEqual(snapshots[2].id, u'4')
 
     def test_extend_volume(self):
-        """Test volume can be extended."""
+        """Test volume can be extended at API level."""
         # create a volume and assign to host
         volume = self._create_volume(2)
         self.volume.create_volume(self.context, volume['id'])
@@ -1255,7 +1255,55 @@ def test_extend_volume(self):
         volume_api.extend(self.context, volume, 3)
 
         volume = db.volume_get(context.get_admin_context(), volume['id'])
-        self.assertEquals(volume['size'], 3)
+        self.assertEquals(volume['status'], 'extending')
+
+        # clean up
+        self.volume.delete_volume(self.context, volume['id'])
+
+    def test_extend_volume_manager(self):
+        """Test volume can be extended at the manager level."""
+        def fake_reserve(context, expire=None, project_id=None, **deltas):
+            return ['RESERVATION']
+
+        def fake_reserve_exc(context, expire=None, project_id=None, **deltas):
+            raise exception.OverQuota(overs=['gigabytes'],
+                                      quotas={'gigabytes': 20},
+                                      usages={'gigabytes': {'reserved': 5,
+                                                            'in_use': 15}})
+
+        def fake_extend_exc(volume, new_size):
+            raise exception.CinderException('fake exception')
+
+        volume = self._create_volume(2)
+        self.volume.create_volume(self.context, volume['id'])
+
+        # Test quota exceeded
+        self.stubs.Set(QUOTAS, 'reserve', fake_reserve_exc)
+        self.stubs.Set(QUOTAS, 'commit', lambda x, y, project_id=None: True)
+        self.stubs.Set(QUOTAS, 'rollback', lambda x, y: True)
+        volume['status'] = 'extending'
+        self.volume.extend_volume(self.context, volume['id'], '4')
+        volume = db.volume_get(context.get_admin_context(), volume['id'])
+        self.assertEquals(volume['size'], 2)
+        self.assertEquals(volume['status'], 'error_extending')
+
+        # Test driver exception
+        self.stubs.Set(QUOTAS, 'reserve', fake_reserve)
+        self.stubs.Set(self.volume.driver, 'extend_volume', fake_extend_exc)
+        volume['status'] = 'extending'
+        self.volume.extend_volume(self.context, volume['id'], '4')
+        volume = db.volume_get(context.get_admin_context(), volume['id'])
+        self.assertEquals(volume['size'], 2)
+        self.assertEquals(volume['status'], 'error_extending')
+
+        # Test driver success
+        self.stubs.Set(self.volume.driver, 'extend_volume',
+                       lambda x, y: True)
+        volume['status'] = 'extending'
+        self.volume.extend_volume(self.context, volume['id'], '4')
+        volume = db.volume_get(context.get_admin_context(), volume['id'])
+        self.assertEquals(volume['size'], 4)
+        self.assertEquals(volume['status'], 'available')
 
         # clean up
         self.volume.delete_volume(self.context, volume['id'])

cinder/volume/api.py

@@ -811,41 +811,9 @@ def extend(self, context, volume, new_size):
                      "extended: %(new_size)s)") % {'new_size': new_size,
                                                    'size': volume['size']})
             raise exception.InvalidInput(reason=msg)
-        try:
-            reservations = QUOTAS.reserve(context, gigabytes=+size_increase)
-        except exception.OverQuota as exc:
-            overs = exc.kwargs['overs']
-            usages = exc.kwargs['usages']
-            quotas = exc.kwargs['quotas']
-
-            def _consumed(name):
-                return (usages[name]['reserved'] + usages[name]['in_use'])
-
-            if 'gigabytes' in overs:
-                msg = _("Quota exceeded for %(s_pid)s, "
-                        "tried to extend volume by "
-                        "%(s_size)sG, (%(d_consumed)dG of %(d_quota)dG "
-                        "already consumed)")
-                LOG.warn(msg % {'s_pid': context.project_id,
-                                's_size': size_increase,
-                                'd_consumed': _consumed('gigabytes'),
-                                'd_quota': quotas['gigabytes']})
-                raise exception.VolumeSizeExceedsAvailableQuota()
 
         self.update(context, volume, {'status': 'extending'})
-
-        try:
-            self.volume_rpcapi.extend_volume(context, volume, new_size)
-        except Exception:
-            with excutils.save_and_reraise_exception():
-                try:
-                    self.update(context, volume, {'status': 'error_extending'})
-                finally:
-                    QUOTAS.rollback(context, reservations)
-
-        self.update(context, volume, {'size': new_size})
-        QUOTAS.commit(context, reservations)
-        self.update(context, volume, {'status': 'available'})
+        self.volume_rpcapi.extend_volume(context, volume, new_size)
 
 
 class HostAPI(base.Base):

cinder/volume/manager.py

@@ -802,14 +802,46 @@ def _notify_about_snapshot_usage(self,
             extra_usage_info=extra_usage_info, host=self.host)
 
     def extend_volume(self, context, volume_id, new_size):
-        volume_ref = self.db.volume_get(context, volume_id)
+        volume = self.db.volume_get(context, volume_id)
+        size_increase = (int(new_size)) - volume['size']
+
+        try:
+            reservations = QUOTAS.reserve(context, gigabytes=+size_increase)
+        except exception.OverQuota as exc:
+            self.db.volume_update(context, volume['id'],
+                                  {'status': 'error_extending'})
+            overs = exc.kwargs['overs']
+            usages = exc.kwargs['usages']
+            quotas = exc.kwargs['quotas']
+
+            def _consumed(name):
+                return (usages[name]['reserved'] + usages[name]['in_use'])
+
+            if 'gigabytes' in overs:
+                msg = _("Quota exceeded for %(s_pid)s, "
+                        "tried to extend volume by "
+                        "%(s_size)sG, (%(d_consumed)dG of %(d_quota)dG "
+                        "already consumed)")
+                LOG.error(msg % {'s_pid': context.project_id,
+                                 's_size': size_increase,
+                                 'd_consumed': _consumed('gigabytes'),
+                                 'd_quota': quotas['gigabytes']})
+            return
 
         try:
-            LOG.info(_("volume %s: extending"), volume_ref['name'])
-            self.driver.extend_volume(volume_ref, new_size)
-            LOG.info(_("volume %s: extended successfully"), volume_ref['name'])
+            LOG.info(_("volume %s: extending"), volume['name'])
+            self.driver.extend_volume(volume, new_size)
+            LOG.info(_("volume %s: extended successfully"), volume['name'])
         except Exception:
             LOG.exception(_("volume %s: Error trying to extend volume"),
                           volume_id)
-            self.db.volume_update(context, volume_ref['id'],
-                                  {'status': 'error_extending'})
+            try:
+                self.db.volume_update(context, volume['id'],
+                                      {'status': 'error_extending'})
+            finally:
+                QUOTAS.rollback(context, reservations)
+                return
+
+        QUOTAS.commit(context, reservations)
+        self.db.volume_update(context, volume['id'], {'size': int(new_size),
+                                                      'status': 'available'})