openstack
/
cinder
Code Issues Proposed changes

VMAX driver - Retrieve volume from snapshot 

The VMAX driver retrieves volume id from snapshot['volume_name'] in
create_snapshot and delete_snapshot and then gets volume from db using
the volume id.  If the volume_name is changed, volume id may not be in
it any more.  This is not a reliable way of getting volume id.
Also a driver should not access db directly. In this patch, the source
volume is retrieved from snapshot without parsing the volume name for
volume id and accessing db.

Change-Id: I99ebecb27d82d5a967369a04bf2c34158e6670ec
Closes-Bug: #1616133
This commit is contained in:
Helen Walsh 2016-08-31 10:54:46 +01:00
parent a3832a8d42
commit 0d241780ee
3 changed files with 31 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
cinder/tests/unit/volume/drivers/emc/test_emc_vmax.py
View File

@ -490,8 +490,17 @@ class EMCVMAXCommonData(object):
test_snapshot = {'name': 'myCG1',
'id': '12345abcde',
'status': 'available',
'host': fake_host
'host': fake_host,
'volume': test_source_volume,
'provider_location': six.text_type(provider_location)
}
test_snapshot_v3 = {'name': 'myCG1',
'id': '12345abcde',
'status': 'available',
'host': fake_host_v3,
'volume': test_source_volume_v3,
'provider_location': six.text_type(provider_location)
}
test_CG_snapshot = {'name': 'testSnap',
'id': '12345abcde',
'consistencygroup_id': '123456789',
@ -3316,18 +3325,13 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
emc_vmax_utils.EMCVMAXUtils,
'get_volume_meta_head',
return_value=[EMCVMAXCommonData.test_volume])
@mock.patch.object(
FakeDB,
'volume_get',
return_value=EMCVMAXCommonData.test_source_volume)
@mock.patch.object(
volume_types,
'get_volume_type_extra_specs',
return_value={'volume_backend_name': 'ISCSINoFAST'})
def test_create_snapshot_different_sizes_meta_no_fast_success(
self, mock_volume_type, mock_volume,
self, mock_volume_type,
mock_meta, mock_size, mock_pool):
self.data.test_volume['volume_name'] = "vmax-1234567"
common = self.driver.common
volumeDict = {'classname': u'Symm_StorageVolume',
'keybindings': EMCVMAXCommonData.keybindings}
@ -3335,13 +3339,13 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
mock.Mock(return_value=(volumeDict, 0)))
common.provision.get_volume_dict_from_job = (
mock.Mock(return_value=volumeDict))
self.driver.create_snapshot(self.data.test_volume)
self.driver.create_snapshot(self.data.test_snapshot)
def test_create_snapshot_no_fast_failed(self):
self.data.test_volume['volume_name'] = "vmax-1234567"
self.assertRaises(exception.VolumeBackendAPIException,
self.driver.create_snapshot,
self.data.test_volume)
self.data.test_snapshot)
@unittest.skip("Skip until bug #1578986 is fixed")
@mock.patch.object(
@ -4231,16 +4235,12 @@ class EMCVMAXISCSIDriverFastTestCase(test.TestCase):
emc_vmax_utils.EMCVMAXUtils,
'get_volume_meta_head',
return_value=[EMCVMAXCommonData.test_volume])
@mock.patch.object(
FakeDB,
'volume_get',
return_value=EMCVMAXCommonData.test_source_volume)
@mock.patch.object(
volume_types,
'get_volume_type_extra_specs',
return_value={'volume_backend_name': 'ISCSIFAST'})
def test_create_snapshot_different_sizes_meta_fast_success(
self, mock_volume_type, mock_volume,
self, mock_volume_type,
mock_meta, mock_size, mock_pool, mock_policy):
self.data.test_volume['volume_name'] = "vmax-1234567"
common = self.driver.common
@ -4253,13 +4253,13 @@ class EMCVMAXISCSIDriverFastTestCase(test.TestCase):
mock.Mock(return_value=volumeDict))
common.fast.is_volume_in_default_SG = (
mock.Mock(return_value=True))
self.driver.create_snapshot(self.data.test_volume)
self.driver.create_snapshot(self.data.test_snapshot)
def test_create_snapshot_fast_failed(self):
self.data.test_volume['volume_name'] = "vmax-1234567"
self.assertRaises(exception.VolumeBackendAPIException,
self.driver.create_snapshot,
self.data.test_volume)
self.data.test_snapshot)
@unittest.skip("Skip until bug #1578986 is fixed")
@mock.patch.object(
@ -5457,18 +5457,13 @@ class EMCVMAXFCDriverFastTestCase(test.TestCase):
emc_vmax_utils.EMCVMAXUtils,
'get_volume_meta_head',
return_value=[EMCVMAXCommonData.test_volume])
@mock.patch.object(
FakeDB,
'volume_get',
return_value=EMCVMAXCommonData.test_source_volume)
@mock.patch.object(
volume_types,
'get_volume_type_extra_specs',
return_value={'volume_backend_name': 'FCFAST'})
def test_create_snapshot_different_sizes_meta_fast_success(
self, mock_volume_type, mock_volume,
self, mock_volume_type,
mock_meta, mock_size, mock_pool, mock_policy):
self.data.test_volume['volume_name'] = "vmax-1234567"
common = self.driver.common
volumeDict = {'classname': u'Symm_StorageVolume',
@ -5479,7 +5474,7 @@ class EMCVMAXFCDriverFastTestCase(test.TestCase):
mock.Mock(return_value=volumeDict))
common.fast.is_volume_in_default_SG = (
mock.Mock(return_value=True))
self.driver.create_snapshot(self.data.test_volume)
self.driver.create_snapshot(self.data.test_snapshot)
@mock.patch.object(
emc_vmax_common.EMCVMAXCommon,
@ -5489,7 +5484,7 @@ class EMCVMAXFCDriverFastTestCase(test.TestCase):
self.data.test_volume['volume_name'] = "vmax-1234567"
self.assertRaises(exception.VolumeBackendAPIException,
self.driver.create_snapshot,
self.data.test_volume)
self.data.test_snapshot)
@unittest.skip("Skip until bug #1578986 is fixed")
@mock.patch.object(
@ -6065,13 +6060,8 @@ class EMCV3DriverTestCase(test.TestCase):
volume_types,
'get_volume_type_extra_specs',
return_value={'volume_backend_name': 'V3_BE'})
@mock.patch.object(
FakeDB,
'volume_get',
return_value=EMCVMAXCommonData.test_source_volume_v3)
def test_create_snapshot_v3_success(
self, mock_volume_db, mock_type, moke_pool):
self.data.test_volume_v3['volume_name'] = "vmax-1234567"
self, mock_type, mock_pool):
common = self.driver.common
common.provisionv3.utils.get_v3_default_sg_instance_name = mock.Mock(
return_value=(None, None, self.data.default_sg_instance_name))
@ -6079,21 +6069,17 @@ class EMCV3DriverTestCase(test.TestCase):
mock.Mock(return_value=True))
common._initial_setup = mock.Mock(
return_value=self.default_extraspec())
self.driver.create_snapshot(self.data.test_volume_v3)
self.driver.create_snapshot(self.data.test_snapshot_v3)
@mock.patch.object(
FakeDB,
'volume_get',
return_value=EMCVMAXCommonData.test_source_volume_v3)
@mock.patch.object(
volume_types,
'get_volume_type_extra_specs',
return_value={'volume_backend_name': 'V3_BE'})
def test_delete_snapshot_v3_success(self, mock_volume_type, mock_db):
def test_delete_snapshot_v3_success(self, mock_volume_type):
self.data.test_volume_v3['volume_name'] = "vmax-1234567"
self.driver.common._initial_setup = mock.Mock(
return_value=self.default_extraspec())
self.driver.delete_snapshot(self.data.test_volume_v3)
self.driver.delete_snapshot(self.data.test_snapshot_v3)
@unittest.skip("Skip until bug #1578986 is fixed")
@mock.patch.object(

18
cinder/volume/drivers/emc/emc_vmax_fc.py
View File

@ -18,7 +18,6 @@ import ast
from oslo_log import log as logging
import six
from cinder import context
from cinder.i18n import _LW
from cinder import interface
from cinder.volume import driver
@ -123,13 +122,8 @@ class EMCVMAXFCDriver(driver.FibreChannelDriver):
def create_snapshot(self, snapshot):
"""Creates a snapshot."""
ctxt = context.get_admin_context()
volumename = snapshot['volume_name']
index = volumename.index('-')
volumeid = volumename[index + 1:]
volume = self.db.volume_get(ctxt, volumeid)
volpath = self.common.create_snapshot(snapshot, volume)
src_volume = snapshot['volume']
volpath = self.common.create_snapshot(snapshot, src_volume)
model_update = {}
snapshot['provider_location'] = six.text_type(volpath)
@ -138,13 +132,9 @@ class EMCVMAXFCDriver(driver.FibreChannelDriver):
def delete_snapshot(self, snapshot):
"""Deletes a snapshot."""
ctxt = context.get_admin_context()
volumename = snapshot['volume_name']
index = volumename.index('-')
volumeid = volumename[index + 1:]
volume = self.db.volume_get(ctxt, volumeid)
src_volume = snapshot['volume']
self.common.delete_snapshot(snapshot, volume)
self.common.delete_snapshot(snapshot, src_volume)
def ensure_export(self, context, volume):
"""Driver entry point to get the export info for an existing volume."""

18
cinder/volume/drivers/emc/emc_vmax_iscsi.py
View File

@ -21,7 +21,6 @@ import os
from oslo_log import log as logging
import six
from cinder import context
from cinder import exception
from cinder.i18n import _, _LE, _LI
from cinder import interface
@ -132,13 +131,8 @@ class EMCVMAXISCSIDriver(driver.ISCSIDriver):
def create_snapshot(self, snapshot):
"""Creates a snapshot."""
ctxt = context.get_admin_context()
volumename = snapshot['volume_name']
index = volumename.index('-')
volumeid = volumename[index + 1:]
volume = self.db.volume_get(ctxt, volumeid)
volpath = self.common.create_snapshot(snapshot, volume)
src_volume = snapshot['volume']
volpath = self.common.create_snapshot(snapshot, src_volume)
model_update = {}
snapshot['provider_location'] = six.text_type(volpath)
@ -147,13 +141,9 @@ class EMCVMAXISCSIDriver(driver.ISCSIDriver):
def delete_snapshot(self, snapshot):
"""Deletes a snapshot."""
ctxt = context.get_admin_context()
volumename = snapshot['volume_name']
index = volumename.index('-')
volumeid = volumename[index + 1:]
volume = self.db.volume_get(ctxt, volumeid)
src_volume = snapshot['volume']
self.common.delete_snapshot(snapshot, volume)
self.common.delete_snapshot(snapshot, src_volume)
def ensure_export(self, context, volume):
"""Driver entry point to get the export info for an existing volume."""