Update quota when deleting volume that failed to be scheduled

If one volume was failed to get scheduled, removing such volume should
also clean up reservation.
Also when create_volume is ready to send to scheduler, reservation
should be committed no matter backend can successfully create that
volume or not since deleting volume call will do a minus reservation
even on volume with 'error' status.

This change updates RPC API to version 2.2

Fix bug 1052052

Change-Id: Ia632a0e49318d534f0acbd3df5c9f6bb86eefa2a

8 files changed, 37 insertions, 28 deletions

diff --git a/nova/scheduler/chance.py b/nova/scheduler/chance.py
index b8dd468f0..f6a289f26 100644
--- a/nova/scheduler/chance.py
+++ b/nova/scheduler/chance.py
@@ -93,10 +93,10 @@ class ChanceScheduler(driver.Scheduler):
         self.compute_rpcapi.prep_resize(context, image, instance,
                 instance_type, host, reservations)
 
-    def schedule_create_volume(self, context, volume_id, snapshot_id, image_id,
-                               reservations):
+    def schedule_create_volume(self, context, volume_id, snapshot_id,
+                               image_id):
         """Picks a host that is up at random."""
         host = self._schedule(context, FLAGS.volume_topic, None, {})
         driver.cast_to_host(context, FLAGS.volume_topic, host, 'create_volume',
                             volume_id=volume_id, snapshot_id=snapshot_id,
-                            image_id=image_id, reservations=reservations)
+                            image_id=image_id)
diff --git a/nova/scheduler/driver.py b/nova/scheduler/driver.py
index df49acfae..a72f3c26d 100644
--- a/nova/scheduler/driver.py
+++ b/nova/scheduler/driver.py
@@ -207,8 +207,8 @@ class Scheduler(object):
         msg = _("Driver must implement schedule_run_instance")
         raise NotImplementedError(msg)
 
-    def schedule_create_volume(self, context, volume_id, snapshot_id, image_id,
-                               reservations):
+    def schedule_create_volume(self, context, volume_id, snapshot_id,
+                               image_id):
         msg = _("Driver must implement schedule_create_volune")
         raise NotImplementedError(msg)
 
diff --git a/nova/scheduler/manager.py b/nova/scheduler/manager.py
index 377cdaaa2..5ba375978 100644
--- a/nova/scheduler/manager.py
+++ b/nova/scheduler/manager.py
@@ -53,7 +53,7 @@ QUOTAS = quota.QUOTAS
 class SchedulerManager(manager.Manager):
     """Chooses a host to run instances on."""
 
-    RPC_API_VERSION = '2.1'
+    RPC_API_VERSION = '2.2'
 
     def __init__(self, scheduler_driver=None, *args, **kwargs):
         if not scheduler_driver:
@@ -70,10 +70,10 @@ class SchedulerManager(manager.Manager):
                 capabilities)
 
     def create_volume(self, context, volume_id, snapshot_id,
-                      reservations, image_id=None):
+                      reservations=None, image_id=None):
         try:
             self.driver.schedule_create_volume(
-                context, volume_id, snapshot_id, image_id, reservations)
+                context, volume_id, snapshot_id, image_id)
         except Exception as ex:
             with excutils.save_and_reraise_exception():
                 self._set_vm_state_and_notify('create_volume',
diff --git a/nova/scheduler/rpcapi.py b/nova/scheduler/rpcapi.py
index f36578dab..2c280be44 100644
--- a/nova/scheduler/rpcapi.py
+++ b/nova/scheduler/rpcapi.py
@@ -45,6 +45,7 @@ class SchedulerAPI(nova.openstack.common.rpc.proxy.RpcProxy):
 
         2.0 - Remove 1.x backwards compat
         2.1 - Add image_id to create_volume()
+        2.2 - Remove reservations argument to create_volume()
     '''
 
     #
@@ -94,13 +95,12 @@ class SchedulerAPI(nova.openstack.common.rpc.proxy.RpcProxy):
                 disk_over_commit=disk_over_commit, instance=instance_p,
                 dest=dest))
 
-    def create_volume(self, ctxt, volume_id, snapshot_id, image_id,
-                      reservations):
+    def create_volume(self, ctxt, volume_id, snapshot_id, image_id):
         self.cast(ctxt,
                   self.make_msg('create_volume',
                                 volume_id=volume_id, snapshot_id=snapshot_id,
-                                image_id=image_id, reservations=reservations),
-                  version='2.1')
+                                image_id=image_id),
+                  version='2.2')
 
     def update_service_capabilities(self, ctxt, service_name, host,
             capabilities):
diff --git a/nova/scheduler/simple.py b/nova/scheduler/simple.py
index 5eee0b136..48e5ea37d 100644
--- a/nova/scheduler/simple.py
+++ b/nova/scheduler/simple.py
@@ -56,8 +56,8 @@ class SimpleScheduler(chance.ChanceScheduler):
                 request_spec, admin_password, injected_files,
                 requested_networks, is_first_time, filter_properties)
 
-    def schedule_create_volume(self, context, volume_id, snapshot_id, image_id,
-                               reservations):
+    def schedule_create_volume(self, context, volume_id, snapshot_id,
+                               image_id):
         """Picks a host that is up and has the fewest volumes."""
         deprecated.warn(_('nova-volume functionality is deprecated in Folsom '
                 'and will be removed in Grizzly.  Volumes are now handled '
@@ -76,7 +76,7 @@ class SimpleScheduler(chance.ChanceScheduler):
                 raise exception.WillNotSchedule(host=host)
             driver.cast_to_volume_host(context, host, 'create_volume',
                     volume_id=volume_id, snapshot_id=snapshot_id,
-                    image_id=image_id, reservations=reservations)
+                    image_id=image_id)
             return None
 
         results = db.service_get_all_volume_sorted(elevated)
@@ -91,8 +91,7 @@ class SimpleScheduler(chance.ChanceScheduler):
             if utils.service_is_up(service) and not service['disabled']:
                 driver.cast_to_volume_host(context, service['host'],
                         'create_volume', volume_id=volume_id,
-                        snapshot_id=snapshot_id, image_id=image_id,
-                        reservations=reservations)
+                        snapshot_id=snapshot_id, image_id=image_id)
                 return None
         msg = _("Is the appropriate service running?")
         raise exception.NoValidHost(reason=msg)
diff --git a/nova/tests/scheduler/test_rpcapi.py b/nova/tests/scheduler/test_rpcapi.py
index 89e6ccedf..ab97adf5c 100644
--- a/nova/tests/scheduler/test_rpcapi.py
+++ b/nova/tests/scheduler/test_rpcapi.py
@@ -95,4 +95,4 @@ class SchedulerRpcAPITestCase(test.TestCase):
         self._test_scheduler_api('create_volume',
                 rpc_method='cast', volume_id="fake_volume",
                 snapshot_id="fake_snapshots", image_id="fake_image",
-                reservations=list('fake_res'), version='2.1')
+                version='2.2')
diff --git a/nova/volume/api.py b/nova/volume/api.py
index 0342c0ac2..6beb771f3 100644
--- a/nova/volume/api.py
+++ b/nova/volume/api.py
@@ -109,6 +109,8 @@ class API(base.Base):
             msg = (_("Volume size '%s' must be an integer and greater than 0")
                    % size)
             raise exception.InvalidInput(reason=msg)
+
+        reservations = None
         try:
             reservations = QUOTAS.reserve(context, volumes=1, gigabytes=size)
         except exception.OverQuota as e:
@@ -165,12 +167,16 @@ class API(base.Base):
             'metadata': metadata,
             }
         volume = self.db.volume_create(context, options)
+
+        if reservations:
+            QUOTAS.commit(context, reservations)
+
         self._cast_create_volume(context, volume['id'],
-                                 snapshot_id, image_id, reservations)
+                                 snapshot_id, image_id)
         return volume
 
     def _cast_create_volume(self, context, volume_id,
-                            snapshot_id, image_id, reservations):
+                            snapshot_id, image_id):
 
         # NOTE(Rongze Zhu): It is a simple solution for bug 1008866
         # If snapshot_id is set, make the call create volume directly to
@@ -189,19 +195,29 @@ class API(base.Base):
                      {"method": "create_volume",
                       "args": {"volume_id": volume_id,
                                "snapshot_id": snapshot_id,
-                               "reservations": reservations,
                                "image_id": image_id}})
 
         else:
             self.scheduler_rpcapi.create_volume(
-                context, volume_id, snapshot_id, image_id, reservations)
+                context, volume_id, snapshot_id, image_id)
 
     @wrap_check_policy
     def delete(self, context, volume, force=False):
         volume_id = volume['id']
         if not volume['host']:
             # NOTE(vish): scheduling failed, so delete it
+            # Note(zhiteng): update volume quota reservation
+            try:
+                reservations = QUOTAS.reserve(context, volumes=-1,
+                                              gigabytes=-volume['size'])
+            except Exception:
+                reservations = None
+                LOG.exception(_("Failed to update quota for deleting volume."))
+
             self.db.volume_destroy(context, volume_id)
+
+            if reservations:
+                QUOTAS.commit(context, reservations)
             return
         if not force and volume['status'] not in ["available", "error"]:
             msg = _("Volume status must be available or error")
diff --git a/nova/volume/manager.py b/nova/volume/manager.py
index e25f3daf1..fafdcd5be 100644
--- a/nova/volume/manager.py
+++ b/nova/volume/manager.py
@@ -160,14 +160,8 @@ class VolumeManager(manager.SchedulerDependentManager):
             model_update = self.driver.create_export(context, volume_ref)
             if model_update:
                 self.db.volume_update(context, volume_ref['id'], model_update)
-
-            # Commit the reservation
-            if reservations:
-                QUOTAS.commit(context, reservations)
         except Exception:
             with excutils.save_and_reraise_exception():
-                if reservations:
-                    QUOTAS.rollback(context, reservations)
                 self.db.volume_update(context,
                                       volume_ref['id'], {'status': 'error'})