openstack
/
cinder
Code Issues Proposed changes

Forbidden to revert volume to a different size snapshot 

As mentioned from original design:
"As we support extend volumes at present, we could meet the
case that snapshot is smaller than volume when reverting to
snapshot and it's obviously safe to revert in that case. But we
will still restrict to revert the volume which current volume size
is equal to snapshot, cause we don't support shrink volume yet
(that ability will be used in the generic method, if the driver
don't support revert to snapshot)."

So, we need add a size check before we revert a volume to a
snapshot. If the volume size is not equal to its latest
snapshot size, it will be raise a HTTPBadRequest(400).

Change-Id: Ib3cc95a10870822b9f597d66007699fb40908b61
Closes-bug: #1798503
(cherry picked from commit bfc27c9aba)
This commit is contained in:
Yikun Jiang 2018-10-18 11:47:29 +08:00 committed by Ivan Kolodyazhny
parent c118e8f92c
commit 8ef380c8ae
2 changed files with 30 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
cinder/api/v3/volumes.py
View File

@ -190,6 +190,12 @@ class VolumeController(volumes_v2.VolumeController):
"the latest one of volume %(v_id)s.")
raise exc.HTTPBadRequest(explanation=msg % {'s_id': snapshot_id,
'v_id': volume.id})
if volume.size != l_snap.volume_size:
msg = _("Can't revert volume %(v_id)s to its latest snapshot "
"%(s_id)s. The volume size must be equal to the snapshot "
"size.")
raise exc.HTTPBadRequest(explanation=msg % {'s_id': snapshot_id,
'v_id': volume.id})
try:
msg = 'Reverting volume %(v_id)s to snapshot %(s_id)s.'
LOG.info(msg, {'v_id': volume.id,

28
cinder/tests/unit/api/v3/test_volumes.py
View File

@ -758,20 +758,22 @@ class VolumeApiTest(test.TestCase):
else:
self.assertNotIn('provider_id', res_dict['volume'])
def _fake_create_volume(self):
def _fake_create_volume(self, size=1):
vol = {
'display_name': 'fake_volume1',
'status': 'available'
'status': 'available',
'size': size
}
volume = objects.Volume(context=self.ctxt, **vol)
volume.create()
return volume
def _fake_create_snapshot(self, volume_id):
def _fake_create_snapshot(self, volume_id, volume_size=1):
snap = {
'display_name': 'fake_snapshot1',
'status': 'available',
'volume_id': volume_id
'volume_id': volume_id,
'volume_size': volume_size
}
snapshot = objects.Snapshot(context=self.ctxt, **snap)
snapshot.create()
@ -842,6 +844,24 @@ class VolumeApiTest(test.TestCase):
req, fake_volume['id'], {'revert': {'snapshot_id':
fake_snapshot['id']}})
@mock.patch.object(objects.Volume, 'get_latest_snapshot')
@mock.patch.object(volume_api.API, 'get_volume')
def test_volume_revert_with_not_equal_size(self, mock_volume,
mock_latest):
fake_volume = self._fake_create_volume(size=2)
fake_snapshot = self._fake_create_snapshot(fake_volume['id'],
volume_size=1)
mock_volume.return_value = fake_volume
mock_latest.return_value = fake_snapshot
req = fakes.HTTPRequest.blank('/v3/volumes/%s/revert'
% fake_volume['id'])
req.headers = mv.get_mv_header(mv.VOLUME_REVERT)
req.api_version_request = mv.get_api_version(
mv.VOLUME_REVERT)
self.assertRaises(webob.exc.HTTPBadRequest, self.controller.revert,
req, fake_volume['id'],
{'revert': {'snapshot_id': fake_snapshot['id']}})
def test_view_get_attachments(self):
fake_volume = self._fake_create_volume()
fake_volume['attach_status'] = fields.VolumeAttachStatus.ATTACHING