openstack/cinder
Code Issues Proposed changes

IBM Storwize driver: Retype the volume with correct empty QoS

Browse Source 
* Currently for Storwzie driver, if the new type does not have QoS
configurations, the old QoS configurations remain in the volume after
retyping it. It should be retyped into a volume with empty QoS for the
Storwize driver.
* Refactor three dicts into one for better maintainance of the QoS keys
for Storwize driver.

DocImpact

Change-Id: I2b2801a4ef72ef02c11392ed00b56f5263a8a7e4
Closes-Bug: #1368595
This commit is contained in:
Vincent Hou 2014-09-12 16:10:02 +08:00
parent bfd861ab82
commit 26de1b1d82
3 changed files with 159 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

126
cinder/tests/test_storwize_svc.py
View File

@ -1716,7 +1716,7 @@ class StorwizeSVCDriverTestCase(test.TestCase):
# If the qos is not empty, chvdisk should be called
# for create_volume.
fake_opts['qos'] = {'rate': 5000}
fake_opts['qos'] = {'IOThrottling': 5000}
get_vdisk_params.return_value = fake_opts
self.driver.create_volume(vol)
self._assert_vol_exists(vol['name'], True)
@ -1854,7 +1854,7 @@ class StorwizeSVCDriverTestCase(test.TestCase):
# If the qos is not empty, chvdisk should be called
# for create_volume_from_snapshot.
fake_opts['qos'] = {'rate': 5000}
fake_opts['qos'] = {'IOThrottling': 5000}
get_vdisk_params.return_value = fake_opts
self.driver.create_volume_from_snapshot(vol2, snap1)
self._assert_vol_exists(vol2['name'], True)
@ -1876,7 +1876,7 @@ class StorwizeSVCDriverTestCase(test.TestCase):
# If the qos is not empty, chvdisk should be called
# for create_volume_from_snapshot.
fake_opts['qos'] = {'rate': 5000}
fake_opts['qos'] = {'IOThrottling': 5000}
get_vdisk_params.return_value = fake_opts
self.driver.create_cloned_volume(vol3, vol2)
self._assert_vol_exists(vol3['name'], True)
@ -2442,8 +2442,10 @@ class StorwizeSVCDriverTestCase(test.TestCase):
self.driver.migrate_volume(ctxt, volume, host)
self._delete_volume(volume)
@mock.patch.object(helpers.StorwizeHelpers, 'add_vdisk_qos')
def test_storwize_svc_retype_no_copy(self, add_vdisk_qos):
@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,
disable_vdisk_qos):
self.driver.do_setup(None)
loc = ('StorwizeSVCDriver:' + self.driver._state['system_id'] +
':openstack')
@ -2475,24 +2477,62 @@ class StorwizeSVCDriverTestCase(test.TestCase):
self.driver.delete_volume(volume)
fake_opts = self._get_default_opts()
fake_opts_old = self._get_default_opts()
fake_opts_old['qos'] = {'IOThrottling': 4000}
fake_opts_qos = self._get_default_opts()
fake_opts_qos['qos'] = {'IOThrottling': 5000}
self.driver.create_volume(volume)
with mock.patch.object(storwize_svc.StorwizeSVCDriver,
'_get_vdisk_params') as get_vdisk_params:
# If qos is empty, chvdisk will not be called for retype.
get_vdisk_params.return_value = fake_opts
# If qos is empty for both the source and target volumes,
# add_vdisk_qos and disable_vdisk_qos will not be called for
# retype.
get_vdisk_params.side_effect = [fake_opts, fake_opts]
self.driver.retype(ctxt, volume, new_type, diff, host)
self.assertFalse(add_vdisk_qos.called)
self.assertFalse(update_vdisk_qos.called)
self.assertFalse(disable_vdisk_qos.called)
self.driver.delete_volume(volume)
self.driver.create_volume(volume)
add_vdisk_qos.reset_mock()
update_vdisk_qos.reset_mock()
with mock.patch.object(storwize_svc.StorwizeSVCDriver,
'_get_vdisk_params') as get_vdisk_params:
# If qos is not empty, chvdisk will be called for retype.
fake_opts['qos'] = {'rate': 5000}
get_vdisk_params.return_value = fake_opts
# If qos is specified for both source and target volumes,
# add_vdisk_qos will be called for retype, and disable_vdisk_qos
# will not be called.
get_vdisk_params.side_effect = [fake_opts_old, fake_opts_qos]
self.driver.retype(ctxt, volume, new_type, diff, host)
add_vdisk_qos.assert_called_with(volume['name'], fake_opts['qos'])
update_vdisk_qos.assert_called_with(volume['name'],
fake_opts_qos['qos'])
self.assertFalse(disable_vdisk_qos.called)
self.driver.delete_volume(volume)
self.driver.create_volume(volume)
update_vdisk_qos.reset_mock()
with mock.patch.object(storwize_svc.StorwizeSVCDriver,
'_get_vdisk_params') as get_vdisk_params:
# If qos is empty for source and speficied for target volume,
# add_vdisk_qos will be called for retype, and disable_vdisk_qos
# will not be called.
get_vdisk_params.side_effect = [fake_opts, fake_opts_qos]
self.driver.retype(ctxt, volume, new_type, diff, host)
update_vdisk_qos.assert_called_with(volume['name'],
fake_opts_qos['qos'])
self.assertFalse(disable_vdisk_qos.called)
self.driver.delete_volume(volume)
self.driver.create_volume(volume)
update_vdisk_qos.reset_mock()
with mock.patch.object(storwize_svc.StorwizeSVCDriver,
'_get_vdisk_params') as get_vdisk_params:
# If qos is empty for target volume and specified for source
# volume, add_vdisk_qos will not be called for retype, and
# disable_vdisk_qos will be called.
get_vdisk_params.side_effect = [fake_opts_qos, fake_opts]
self.driver.retype(ctxt, volume, new_type, diff, host)
self.assertFalse(update_vdisk_qos.called)
disable_vdisk_qos.assert_called_with(volume['name'],
fake_opts_qos['qos'])
self.driver.delete_volume(volume)
def test_storwize_svc_retype_only_change_iogrp(self):
@ -2525,8 +2565,10 @@ class StorwizeSVCDriverTestCase(test.TestCase):
'failed')
self.driver.delete_volume(volume)
@mock.patch.object(helpers.StorwizeHelpers, 'add_vdisk_qos')
def test_storwize_svc_retype_need_copy(self, add_vdisk_qos):
@mock.patch.object(helpers.StorwizeHelpers, 'disable_vdisk_qos')
@mock.patch.object(helpers.StorwizeHelpers, 'update_vdisk_qos')
def test_storwize_svc_retype_need_copy(self, update_vdisk_qos,
disable_vdisk_qos):
self.driver.do_setup(None)
loc = ('StorwizeSVCDriver:' + self.driver._state['system_id'] +
':openstack')
@ -2558,24 +2600,62 @@ class StorwizeSVCDriverTestCase(test.TestCase):
self.driver.delete_volume(volume)
fake_opts = self._get_default_opts()
fake_opts_old = self._get_default_opts()
fake_opts_old['qos'] = {'IOThrottling': 4000}
fake_opts_qos = self._get_default_opts()
fake_opts_qos['qos'] = {'IOThrottling': 5000}
self.driver.create_volume(volume)
with mock.patch.object(storwize_svc.StorwizeSVCDriver,
'_get_vdisk_params') as get_vdisk_params:
# If qos is empty, chvdisk will not be called for retype.
get_vdisk_params.return_value = fake_opts
# If qos is empty for both the source and target volumes,
# add_vdisk_qos and disable_vdisk_qos will not be called for
# retype.
get_vdisk_params.side_effect = [fake_opts, fake_opts]
self.driver.retype(ctxt, volume, new_type, diff, host)
self.assertFalse(add_vdisk_qos.called)
self.assertFalse(update_vdisk_qos.called)
self.assertFalse(disable_vdisk_qos.called)
self.driver.delete_volume(volume)
add_vdisk_qos.reset_mock()
self.driver.create_volume(volume)
update_vdisk_qos.reset_mock()
with mock.patch.object(storwize_svc.StorwizeSVCDriver,
'_get_vdisk_params') as get_vdisk_params:
# If qos is not empty, chvdisk will be called for retype.
fake_opts['qos'] = {'rate': 5000}
get_vdisk_params.return_value = fake_opts
# If qos is specified for both source and target volumes,
# add_vdisk_qos will be called for retype, and disable_vdisk_qos
# will not be called.
get_vdisk_params.side_effect = [fake_opts_old, fake_opts_qos]
self.driver.retype(ctxt, volume, new_type, diff, host)
add_vdisk_qos.assert_called_with(volume['name'], fake_opts['qos'])
update_vdisk_qos.assert_called_with(volume['name'],
fake_opts_qos['qos'])
self.assertFalse(disable_vdisk_qos.called)
self.driver.delete_volume(volume)
self.driver.create_volume(volume)
update_vdisk_qos.reset_mock()
with mock.patch.object(storwize_svc.StorwizeSVCDriver,
'_get_vdisk_params') as get_vdisk_params:
# If qos is empty for source and speficied for target volume,
# add_vdisk_qos will be called for retype, and disable_vdisk_qos
# will not be called.
get_vdisk_params.side_effect = [fake_opts, fake_opts_qos]
self.driver.retype(ctxt, volume, new_type, diff, host)
update_vdisk_qos.assert_called_with(volume['name'],
fake_opts_qos['qos'])
self.assertFalse(disable_vdisk_qos.called)
self.driver.delete_volume(volume)
self.driver.create_volume(volume)
update_vdisk_qos.reset_mock()
with mock.patch.object(storwize_svc.StorwizeSVCDriver,
'_get_vdisk_params') as get_vdisk_params:
# If qos is empty for target volume and specified for source
# volume, add_vdisk_qos will not be called for retype, and
# disable_vdisk_qos will be called.
get_vdisk_params.side_effect = [fake_opts_qos, fake_opts]
self.driver.retype(ctxt, volume, new_type, diff, host)
self.assertFalse(update_vdisk_qos.called)
disable_vdisk_qos.assert_called_with(volume['name'],
fake_opts_qos['qos'])
self.driver.delete_volume(volume)
def test_set_storage_code_level_success(self):

10
cinder/volume/drivers/ibm/storwize_svc/__init__.py
View File

@ -920,9 +920,13 @@ class StorwizeSVCDriver(san.SanDriver):
self._helpers.change_vdisk_options(volume['name'], vdisk_changes,
new_opts, self._state)
qos = new_opts['qos'] or old_opts['qos']
if qos:
self._helpers.add_vdisk_qos(volume['name'], qos)
if new_opts['qos']:
# Add the new QoS setting to the volume. If the volume has an
# old QoS setting, it will be overwritten.
self._helpers.update_vdisk_qos(volume['name'], new_opts['qos'])
elif old_opts['qos']:
# If the old_opts contain QoS keys, disable them.
self._helpers.disable_vdisk_qos(volume['name'], old_opts['qos'])
# Add replica if needed
if not old_type_replication and new_type_replication:

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

@ -38,8 +38,14 @@ LOG = logging.getLogger(__name__)
class StorwizeHelpers(object):
svc_qos_keys = {'IOThrottling': int}
svc_qos_param_dict = {'IOThrottling': 'rate'}
# All the supported QoS key are saved in this dict. When a new
# key is going to add, three values MUST be set:
# 'default': to indicate the value, when the parameter is disabled.
# 'param': to indicate the corresponding parameter in the command.
# 'type': to indicate the type of this value.
svc_qos_keys = {'IOThrottling': {'default': '0',
'param': 'rate',
'type': int}}
def __init__(self, run_ssh):
self.ssh = storwize_ssh.StorwizeSSH(run_ssh)
@ -487,12 +493,13 @@ class StorwizeHelpers(object):
# Add the QoS.
if scope and scope == 'qos':
type_fn = self.svc_qos_keys[key]
try:
value = type_fn(value)
qos[self.svc_qos_param_dict[key]] = value
except ValueError:
continue
if key in self.svc_qos_keys.keys():
try:
type_fn = self.svc_qos_keys[key]['type']
value = type_fn(value)
qos[key] = value
except ValueError:
continue
# Any keys that the driver should look at should have the
# 'drivers' scope.
@ -525,10 +532,10 @@ class StorwizeHelpers(object):
# Add the QoS.
if scope and scope == 'qos':
if key in self.svc_qos_keys.keys():
type_fn = self.svc_qos_keys[key]
try:
type_fn = self.svc_qos_keys[key]['type']
value = type_fn(value)
qos[self.svc_qos_param_dict[key]] = value
qos[key] = value
except ValueError:
continue
return qos
@ -876,8 +883,39 @@ class StorwizeHelpers(object):
return dest_pool
def add_vdisk_qos(self, vdisk, qos):
"""Add the QoS configuration to the volume."""
for key, value in qos.iteritems():
self.ssh.chvdisk(vdisk, ['-' + key, str(value)])
if key in self.svc_qos_keys.keys():
param = self.svc_qos_keys[key]['param']
self.ssh.chvdisk(vdisk, ['-' + param, str(value)])
def update_vdisk_qos(self, vdisk, qos):
"""Update all the QoS in terms of a key and value.
svc_qos_keys saves all the supported QoS parameters. Going through
this dict, we set the new values to all the parameters. If QoS is
available in the QoS configuration, the value is taken from it;
if not, the value will be set to default.
"""
for key, value in self.svc_qos_keys.iteritems():
param = value['param']
if key in qos.keys():
# If the value is set in QoS, take the value from
# the QoS configuration.
v = qos[key]
else:
# If not, set the value to default.
v = value['default']
self.ssh.chvdisk(vdisk, ['-' + param, str(v)])
def disable_vdisk_qos(self, vdisk, qos):
"""Disable the QoS."""
for key, value in qos.iteritems():
if key in self.svc_qos_keys.keys():
param = self.svc_qos_keys[key]['param']
# Take the default value.
value = self.svc_qos_keys[key]['default']
self.ssh.chvdisk(vdisk, ['-' + param, value])
def change_vdisk_options(self, vdisk, changes, opts, state):
if 'warning' in opts: