openstack/cinder
Code Issues Proposed changes

IBM Storwize driver: Add local variable assignment to "ctxt"

Browse Source 
* The method get_vdisk_params in helpers.py is missing a local variable
assignment for "ctxt", causing "UnboundLocalError: local variable
'ctxt' referenced before assignment. Adding the assignment should
resolve this issue.

* Add the unit tests coverage for get_vdisk_params.

Change-Id: I564b1ef8cd1b6504d5ac8c9af0bb11bf29767d9a
closes-bug: #1379654
This commit is contained in:
Vincent Hou 2014-10-10 15:46:36 +08:00
parent 83d7bd0cb7
commit afcbfe053c
2 changed files with 132 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

136
cinder/tests/test_storwize_svc.py
View File

@ -39,6 +39,7 @@ from cinder.volume import configuration as conf
from cinder.volume.drivers.ibm import storwize_svc
from cinder.volume.drivers.ibm.storwize_svc import helpers
from cinder.volume.drivers.ibm.storwize_svc import ssh
from cinder.volume import qos_specs
from cinder.volume import volume_types
LOG = logging.getLogger(__name__)
@ -1513,7 +1514,8 @@ class StorwizeSVCDriverTestCase(test.TestCase):
'storwize_svc_flashcopy_timeout': 20,
# Test ignore capitalization
'storwize_svc_connection_protocol': 'iScSi',
'storwize_svc_multipath_enabled': False}
'storwize_svc_multipath_enabled': False,
'storwize_svc_allow_tenant_qos': True}
wwpns = [str(random.randint(0, 9999999999999999)).zfill(16),
str(random.randint(0, 9999999999999999)).zfill(16)]
initiator = 'test.initiator.%s' % str(random.randint(10000, 99999))
@ -1535,6 +1537,7 @@ class StorwizeSVCDriverTestCase(test.TestCase):
# Test ignore capitalization
'storwize_svc_connection_protocol': 'iScSi',
'storwize_svc_multipath_enabled': False,
'storwize_svc_allow_tenant_qos': True,
'ssh_conn_timeout': 0}
config_group = self.driver.configuration.config_group
self.driver.configuration.set_override('rootwrap_config',
@ -1697,7 +1700,9 @@ class StorwizeSVCDriverTestCase(test.TestCase):
'protocol': 'iSCSI',
'multipath': False,
'iogrp': 0,
'qos': None}
'qos': None,
'replication': False,
'stretched_cluster': None}
return opt
@mock.patch.object(helpers.StorwizeHelpers, 'add_vdisk_qos')
@ -2442,6 +2447,107 @@ class StorwizeSVCDriverTestCase(test.TestCase):
self.driver.migrate_volume(ctxt, volume, host)
self._delete_volume(volume)
def test_storwize_svc_get_vdisk_params(self):
self.driver.do_setup(None)
fake_qos = {'qos:IOThrottling': 5000}
expected_qos = {'IOThrottling': 5000}
fake_opts = self._get_default_opts()
# The parameters retured should be the same to the default options,
# if the QoS is empty.
vol_type_empty_qos = self._create_volume_type_qos(True, None)
type_id = vol_type_empty_qos['id']
params = self.driver._get_vdisk_params(type_id,
volume_type=vol_type_empty_qos,
volume_metadata=None)
self.assertEqual(fake_opts, params)
volume_types.destroy(self.ctxt, type_id)
# If the QoS is set via the qos association with the volume type,
# qos value should be set in the retured parameters.
vol_type_qos = self._create_volume_type_qos(False, fake_qos)
type_id = vol_type_qos['id']
# If type_id is not none and volume_type is none, it should work fine.
params = self.driver._get_vdisk_params(type_id, volume_type=None,
volume_metadata=None)
self.assertEqual(expected_qos, params['qos'])
# If type_id is not none and volume_type is not none, it should
# work fine.
params = self.driver._get_vdisk_params(type_id,
volume_type=vol_type_qos,
volume_metadata=None)
self.assertEqual(expected_qos, params['qos'])
# If type_id is none and volume_type is not none, it should work fine.
params = self.driver._get_vdisk_params(None,
volume_type=vol_type_qos,
volume_metadata=None)
self.assertEqual(expected_qos, params['qos'])
# If both type_id and volume_type are none, no qos will be returned
# in the parameter.
params = self.driver._get_vdisk_params(None, volume_type=None,
volume_metadata=None)
self.assertEqual(None, params['qos'])
qos_spec = volume_types.get_volume_type_qos_specs(type_id)
volume_types.destroy(self.ctxt, type_id)
qos_specs.delete(self.ctxt, qos_spec['qos_specs']['id'])
# If the QoS is set via the extra specs in the volume type,
# qos value should be set in the retured parameters.
vol_type_qos = self._create_volume_type_qos(True, fake_qos)
type_id = vol_type_qos['id']
# If type_id is not none and volume_type is none, it should work fine.
params = self.driver._get_vdisk_params(type_id, volume_type=None,
volume_metadata=None)
self.assertEqual(expected_qos, params['qos'])
# If type_id is not none and volume_type is not none,
# it should work fine.
params = self.driver._get_vdisk_params(type_id,
volume_type=vol_type_qos,
volume_metadata=None)
self.assertEqual(expected_qos, params['qos'])
# If type_id is none and volume_type is not none,
# it should work fine.
params = self.driver._get_vdisk_params(None,
volume_type=vol_type_qos,
volume_metadata=None)
self.assertEqual(expected_qos, params['qos'])
# If both type_id and volume_type are none, no qos will be returned
# in the parameter.
params = self.driver._get_vdisk_params(None, volume_type=None,
volume_metadata=None)
self.assertEqual(None, params['qos'])
volume_types.destroy(self.ctxt, type_id)
# If the QoS is set in the volume metadata,
# qos value should be set in the retured parameters.
metadata = [{'key': 'qos:IOThrottling', 'value': 4000}]
expected_qos_metadata = {'IOThrottling': 4000}
params = self.driver._get_vdisk_params(None, volume_type=None,
volume_metadata=metadata)
self.assertEqual(expected_qos_metadata, params['qos'])
# If the QoS is set both in the metadata and the volume type, the one
# in the volume type will take effect.
vol_type_qos = self._create_volume_type_qos(True, fake_qos)
type_id = vol_type_qos['id']
params = self.driver._get_vdisk_params(type_id, volume_type=None,
volume_metadata=metadata)
self.assertEqual(expected_qos, params['qos'])
volume_types.destroy(self.ctxt, type_id)
# If the QoS is set both via the qos association and the
# extra specs, the one from the qos association will take effect.
fake_qos_associate = {'qos:IOThrottling': 6000}
expected_qos_associate = {'IOThrottling': 6000}
vol_type_qos = self._create_volume_type_qos_both(fake_qos,
fake_qos_associate)
type_id = vol_type_qos['id']
params = self.driver._get_vdisk_params(type_id, volume_type=None,
volume_metadata=None)
self.assertEqual(expected_qos_associate, params['qos'])
qos_spec = volume_types.get_volume_type_qos_specs(type_id)
volume_types.destroy(self.ctxt, type_id)
qos_specs.delete(self.ctxt, qos_spec['qos_specs']['id'])
@mock.patch.object(helpers.StorwizeHelpers, 'disable_vdisk_qos')
@mock.patch.object(helpers.StorwizeHelpers, 'update_vdisk_qos')
def test_storwize_svc_retype_no_copy(self, update_vdisk_qos,
@ -3095,6 +3201,29 @@ class StorwizeSVCDriverTestCase(test.TestCase):
self.assertEqual(term_data, term_ret)
def _create_volume_type_qos(self, extra_specs, fake_qos):
# Generate a QoS volume type for volume.
if extra_specs:
spec = fake_qos
type_ref = volume_types.create(self.ctxt, "qos_extra_specs", spec)
else:
type_ref = volume_types.create(self.ctxt, "qos_associate", None)
if fake_qos:
qos_ref = qos_specs.create(self.ctxt, 'qos-specs', fake_qos)
qos_specs.associate_qos_with_type(self.ctxt, qos_ref['id'],
type_ref['id'])
qos_type = volume_types.get_volume_type(self.ctxt, type_ref['id'])
return qos_type
def _create_volume_type_qos_both(self, fake_qos, fake_qos_associate):
type_ref = volume_types.create(self.ctxt, "qos_extra_specs", fake_qos)
qos_ref = qos_specs.create(self.ctxt, 'qos-specs', fake_qos_associate)
qos_specs.associate_qos_with_type(self.ctxt, qos_ref['id'],
type_ref['id'])
qos_type = volume_types.get_volume_type(self.ctxt, type_ref['id'])
return qos_type
def _create_replication_volume_type(self, enable):
# Generate a volume type for volume repliation.
if enable:
@ -3335,6 +3464,3 @@ class StorwizeHelpersTestCase(test.TestCase):
with mock.patch.object(ssh.StorwizeSSH, 'lslicense') as lslicense:
lslicense.return_value = fake_license
self.assertTrue(self.helpers.compression_enabled())
def test_get_vdisk_params(self):
pass

2
cinder/volume/drivers/ibm/storwize_svc/helpers.py
View File

@ -547,8 +547,8 @@ class StorwizeHelpers(object):
Takes volume type and defaults from config options into account.
"""
opts = self.build_default_opts(config)
ctxt = context.get_admin_context()
if volume_type is None and type_id is not None:
ctxt = context.get_admin_context()
volume_type = volume_types.get_volume_type(ctxt, type_id)
if volume_type:
qos_specs_id = volume_type.get('qos_specs_id')