Skip to content

Commit

Permalink
Snapshot reservation sync calls wrong resource.
Browse files Browse the repository at this point in the history
The snapshot reservations code isn't calling the
correct resource on sync (it's calling volumes).  There's
also some problems with the logic being used on the delete/clean up
that are fixed here as well.

Fixes bug: 1157506
Fixes bug: 1157982

Change-Id: I91327b8043ab63aa35ea8a91b6de544bf5bf6c61
  • Loading branch information
j-griffith committed Mar 22, 2013
1 parent 65e2910 commit b450eef
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 13 deletions.
2 changes: 1 addition & 1 deletion cinder/db/api.py
Expand Up @@ -314,7 +314,7 @@ def snapshot_data_get_for_project(context, project_id, session=None):
"""Get count and gigabytes used for snapshots for specified project."""
return IMPL.snapshot_data_get_for_project(context,
project_id,
session=None)
session)


def snapshot_get_active_by_window(context, begin, end=None, project_id=None):
Expand Down
5 changes: 4 additions & 1 deletion cinder/flags.py
Expand Up @@ -238,6 +238,9 @@ def _get_my_ip():
default=None,
help='A list of backend names to use. These backend names '
'should be backed by a unique [CONFIG] group '
'with its options'), ]
'with its options'),
cfg.BoolOpt('no_snapshot_gb_quota',
default=False,
help='Whether snapshots count against GigaByte quota'), ]

FLAGS.register_opts(global_opts)
6 changes: 3 additions & 3 deletions cinder/quota.py
Expand Up @@ -738,9 +738,9 @@ def _sync_volumes(context, project_id, session):

def _sync_snapshots(context, project_id, session):
return dict(zip(('snapshots', 'gigabytes'),
db.volume_data_get_for_project(context,
project_id,
session=session)))
db.snapshot_data_get_for_project(context,
project_id,
session=session)))


QUOTAS = QuotaEngine()
Expand Down
11 changes: 6 additions & 5 deletions cinder/tests/test_quota.py
Expand Up @@ -68,13 +68,14 @@ def _create_volume(self, size=10):
vol['user_id'] = self.user_id
vol['project_id'] = self.project_id
vol['size'] = size
return db.volume_create(self.context, vol)['id']
vol['status'] = 'available'
return db.volume_create(self.context, vol)

def test_too_many_volumes(self):
volume_ids = []
for i in range(FLAGS.quota_volumes):
volume_id = self._create_volume()
volume_ids.append(volume_id)
vol_ref = self._create_volume()
volume_ids.append(vol_ref['id'])
self.assertRaises(exception.QuotaError,
volume.API().create,
self.context, 10, '', '', None)
Expand All @@ -83,8 +84,8 @@ def test_too_many_volumes(self):

def test_too_many_gigabytes(self):
volume_ids = []
volume_id = self._create_volume(size=20)
volume_ids.append(volume_id)
vol_ref = self._create_volume(size=20)
volume_ids.append(vol_ref['id'])
self.assertRaises(exception.QuotaError,
volume.API().create,
self.context, 10, '', '', None)
Expand Down
7 changes: 5 additions & 2 deletions cinder/volume/api.py
Expand Up @@ -491,8 +491,11 @@ def _create_snapshot(self, context,
raise exception.InvalidVolume(reason=msg)

try:
reservations = QUOTAS.reserve(context, snapshots=1,
gigabytes=volume['size'])
if FLAGS.no_snapshot_gb_quota:
reservations = QUOTAS.reserve(context, snapshots=1)
else:
reservations = QUOTAS.reserve(context, snapshots=1,
gigabytes=volume['size'])
except exception.OverQuota as e:
overs = e.kwargs['overs']
usages = e.kwargs['usages']
Expand Down
16 changes: 16 additions & 0 deletions cinder/volume/manager.py
Expand Up @@ -489,9 +489,25 @@ def delete_snapshot(self, context, snapshot_id):
snapshot_ref['id'],
{'status': 'error_deleting'})

# Get reservations
try:
if CONF.no_snapshot_gb_quota:
reservations = QUOTAS.reserve(context, snapshots=-1)
else:
reservations = QUOTAS.reserve(
context,
snapshots=-1,
gigabytes=-snapshot_ref['volume_size'])
except Exception:
reservations = None
LOG.exception(_("Failed to update usages deleting snapshot"))
self.db.volume_glance_metadata_delete_by_snapshot(context, snapshot_id)
self.db.snapshot_destroy(context, snapshot_id)
LOG.info(_("snapshot %s: deleted successfully"), snapshot_ref['name'])

# Commit the reservations
if reservations:
QUOTAS.commit(context, reservations)
return True

def attach_volume(self, context, volume_id, instance_uuid, mountpoint):
Expand Down
4 changes: 3 additions & 1 deletion etc/cinder/cinder.conf.sample
Expand Up @@ -44,6 +44,8 @@
# syslog facility to receive log lines (string value)
#syslog_log_facility=LOG_USER

# Do not count snapshots against gigabytes quota (bool value)
#no_snapshot_gb_quota=False

#
# Options defined in cinder.exception
Expand Down Expand Up @@ -1168,4 +1170,4 @@
#volume_driver=cinder.volume.driver.FakeISCSIDriver


# Total option count: 254
# Total option count: 255

0 comments on commit b450eef

Please sign in to comment.