Skip to content

Commit

Permalink
Remove usage of locals() for formatting from cinder.volume.*
Browse files Browse the repository at this point in the history 
Using of locals() for formatting string is a nasty thing because:
1) It is not so clear as using explicit dicts
2) It could produce hidden errors during refactoring
3) Changing name of variable causes change in message
4) Creating a lot of unused variables

Fix bug 1171936

Change-Id: I806c530851527db9da251352be45b97c183241a8
  • Loading branch information
Haomai Wang committed Jun 16, 2013
1 parent 0e249f4 commit a80003c
Show file tree
Hide file tree
Showing 10 changed files with 123 additions and 96 deletions.
3 changes: 2 additions & 1 deletion cinder/volume/drivers/emc/emc_smis_common.py
View file
Expand Up @@ -943,7 +943,8 @@ def _get_ecom_server(self, filename=None):
ecomPort = ecomPorts[0].toxml().replace('<EcomServerPort>', '')
ecomPort = ecomPort.replace('</EcomServerPort>', '')
if ecomIp is not None and ecomPort is not None:
LOG.debug(_("Ecom IP: %(ecomIp)s Port: %(ecomPort)s") % (locals()))
LOG.debug(_("Ecom IP: %(ecomIp)s Port: %(ecomPort)s"),
{'ecomIp': ecomIp, 'ecomPort': ecomPort})
return ecomIp, ecomPort
else:
LOG.debug(_("Ecom server not found."))
Expand Down
2 changes: 1 addition & 1 deletion cinder/volume/drivers/glusterfs.py
View file
Expand Up @@ -67,7 +67,7 @@ def do_setup(self, context):
raise exception.GlusterfsException(msg)
if not os.path.exists(config):
msg = (_("Gluster config file at %(config)s doesn't exist") %
locals())
{'config': config})
LOG.warn(msg)
raise exception.GlusterfsException(msg)

Expand Down
129 changes: 72 additions & 57 deletions cinder/volume/drivers/netapp/iscsi.py
View file
Expand Up @@ -122,10 +122,9 @@ def __init__(self, *args, **kwargs):
def _check_fail(self, request, response):
"""Utility routine to handle checking ZAPI failures."""
if 'failed' == response.Status:
name = request.Name
reason = response.Reason
msg = _('API %(name)s failed: %(reason)s')
raise exception.VolumeBackendAPIException(data=msg % locals())
msg_fmt = {'name': request.Name, 'reason': response.Reason}
raise exception.VolumeBackendAPIException(msg % msg_fmt)

def _create_client(self, **kwargs):
"""Instantiate a web services client.
Expand Down Expand Up @@ -281,10 +280,10 @@ def _discover_luns(self):
project, type)
self.discovered_datasets.append(ds)
self._discover_dataset_luns(ds, None)
dataset_count = len(self.discovered_datasets)
lun_count = len(self.discovered_luns)
msg = _("Discovered %(dataset_count)s datasets and %(lun_count)s LUNs")
LOG.debug(msg % locals())
msg = (_("Discovered %(dataset_count)s datasets and %(lun_count)s"
"LUNs") % {'dataset_count': len(self.discovered_datasets),
'lun_count': len(self.discovered_luns)})
LOG.debug(msg)
self.lun_table = {}

def _get_job_progress(self, job_id):
Expand Down Expand Up @@ -464,8 +463,8 @@ def _remove_destroy(self, name, project):
lun = self._lookup_lun_for_volume(name, project)
lun_details = self._get_lun_details(lun.id)
except exception.VolumeBackendAPIException:
msg = _("No entry in LUN table for volume %(name)s.")
LOG.debug(msg % locals())
LOG.debug(_("No entry in LUN table for volume %(name)s."),
{'name': name})
return

member = self.client.factory.create('DatasetMemberParameter')
Expand Down Expand Up @@ -1045,7 +1044,8 @@ def create_volume_from_snapshot(self, volume, snapshot):
if vol_size != snap_size:
msg = _('Cannot create volume of size %(vol_size)s from '
'snapshot of size %(snap_size)s')
raise exception.VolumeBackendAPIException(data=msg % locals())
msg_fmt = {'vol_size': vol_size, 'snap_size': snap_size}
raise exception.VolumeBackendAPIException(msg % msg_fmt)
vol_name = snapshot['volume_name']
snapshot_name = snapshot['name']
project = snapshot['project_id']
Expand All @@ -1057,7 +1057,8 @@ def create_volume_from_snapshot(self, volume, snapshot):
if new_type != old_type:
msg = _('Cannot create volume of type %(new_type)s from '
'snapshot of type %(old_type)s')
raise exception.VolumeBackendAPIException(data=msg % locals())
msg_fmt = {'vol_size': vol_size, 'snap_size': snap_size}
raise exception.VolumeBackendAPIException(msg % msg_fmt)
lun = self._get_lun_details(lun_id)
extra_gb = vol_size
new_size = '+%dg' % extra_gb
Expand All @@ -1078,7 +1079,8 @@ def create_cloned_volume(self, volume, src_vref):
if vol_size != src_vol_size:
msg = _('Cannot create clone of size %(vol_size)s from '
'volume of size %(src_vol_size)s')
raise exception.VolumeBackendAPIException(data=msg % locals())
msg_fmt = {'vol_size': vol_size, 'src_vol_size': src_vol_size}
raise exception.VolumeBackendAPIException(msg % msg_fmt)
src_vol_name = src_vref['name']
project = src_vref['project_id']
lun = self._lookup_lun_for_volume(src_vol_name, project)
Expand All @@ -1087,9 +1089,10 @@ def create_cloned_volume(self, volume, src_vref):
old_type = dataset.type
new_type = self._get_ss_type(volume)
if new_type != old_type:
msg = _('Cannot create clone of type %(new_type)s from '
'volume of type %(old_type)s')
raise exception.VolumeBackendAPIException(data=msg % locals())
msg = (_('Cannot create clone of type %(new_type)s from '
'volume of type %(old_type)s') %
{'new_type': new_type, 'old_type': old_type})
raise exception.VolumeBackendAPIException(data=msg)
lun = self._get_lun_details(lun_id)
extra_gb = vol_size
new_size = '+%dg' % extra_gb
Expand Down Expand Up @@ -1144,8 +1147,9 @@ def get_metadata_property(self, prop):
if prop in self.metadata:
return self.metadata[prop]
name = self.name
msg = _("No metadata property %(prop)s defined for the LUN %(name)s")
LOG.debug(msg % locals())
msg = _("No metadata property %(prop)s defined for the LUN "
"%(name)s") % {'prop': prop, 'name': name}
LOG.debug(msg)

def __str__(self, *args, **kwargs):
return 'NetApp Lun[handle:%s, name:%s, size:%s, metadata:%s]'\
Expand Down Expand Up @@ -1251,8 +1255,8 @@ def delete_volume(self, volume):
name = volume['name']
handle = self._get_lun_handle(name)
if not handle:
msg = _("No entry in LUN table for volume %(name)s.")
LOG.warn(msg % locals())
LOG.warn(_("No entry in LUN table for volume %(name)s."),
{'name': name})
return
self.client.service.DestroyLun(Handle=handle)
LOG.debug(_("Destroyed LUN %s") % handle)
Expand Down Expand Up @@ -1292,16 +1296,17 @@ def initialize_connection(self, volume, connector):
server = self.client.service
server.MapLun(Handle=handle, InitiatorType="iscsi",
InitiatorName=initiator_name)
msg = _("Mapped LUN %(handle)s to the initiator %(initiator_name)s")
LOG.debug(msg % locals())
LOG.debug(_("Mapped LUN %(handle)s to the initiator "
"%(initiator_name)s"),
{'handle': handle, 'initiator_name': initiator_name})

target_details_list = server.GetLunTargetDetails(
Handle=handle,
InitiatorType="iscsi",
InitiatorName=initiator_name)
msg = _("Succesfully fetched target details for LUN %(handle)s and "
"initiator %(initiator_name)s")
LOG.debug(msg % locals())
LOG.debug(_("Succesfully fetched target details for LUN %(handle)s and"
" initiator %(initiator_name)s"),
{'handle': handle, 'initiator_name': initiator_name})

if not target_details_list:
msg = _('Failed to get LUN target details for the LUN %s')
Expand Down Expand Up @@ -1346,8 +1351,9 @@ def terminate_connection(self, volume, connector, **kwargs):
self.client.service.UnmapLun(Handle=handle, InitiatorType="iscsi",
InitiatorName=initiator_name)
msg = _("Unmapped LUN %(handle)s from the initiator "
"%(initiator_name)s")
LOG.debug(msg % locals())
"%(initiator_name)s") % {'handle': handle,
'initiator_name': initiator_name}
LOG.debug(msg)

def create_snapshot(self, snapshot):
"""Driver entry point for creating a snapshot.
Expand All @@ -1366,8 +1372,8 @@ def delete_snapshot(self, snapshot):
name = snapshot['name']
handle = self._get_lun_handle(name)
if not handle:
msg = _("No entry in LUN table for snapshot %(name)s.")
LOG.warn(msg % locals())
LOG.warn(_("No entry in LUN table for snapshot %(name)s."),
{'name': name})
return
self.client.service.DestroyLun(Handle=handle)
LOG.debug(_("Destroyed LUN %s") % handle)
Expand All @@ -1384,7 +1390,8 @@ def create_volume_from_snapshot(self, volume, snapshot):
if vol_size != snap_size:
msg = _('Cannot create volume of size %(vol_size)s from '
'snapshot of size %(snap_size)s')
raise exception.VolumeBackendAPIException(data=msg % locals())
msg_fmt = {'vol_size': vol_size, 'snap_size': snap_size}
raise exception.VolumeBackendAPIException(msg % msg_fmt)
snapshot_name = snapshot['name']
lun = self.lun_table[snapshot_name]
new_name = volume['name']
Expand Down Expand Up @@ -1461,7 +1468,8 @@ def create_cloned_volume(self, volume, src_vref):
if vol_size != src_vol_size:
msg = _('Cannot clone volume of size %(vol_size)s from '
'src volume of size %(src_vol_size)s')
raise exception.VolumeBackendAPIException(data=msg % locals())
msg_fmt = {'vol_size': vol_size, 'src_vol_size': src_vol_size}
raise exception.VolumeBackendAPIException(msg % msg_fmt)
new_name = volume['name']
extra_args = {}
extra_args['OsType'] = 'linux'
Expand Down Expand Up @@ -1586,8 +1594,8 @@ def delete_volume(self, volume):
name = volume['name']
metadata = self._get_lun_attr(name, 'metadata')
if not metadata:
msg = _("No entry in LUN table for volume/snapshot %(name)s.")
LOG.warn(msg % locals())
LOG.warn(_("No entry in LUN table for volume/snapshot %(name)s."),
{'name': name})
return
lun_destroy = NaElement.create_node_with_children(
'lun-destroy',
Expand Down Expand Up @@ -1629,13 +1637,15 @@ def initialize_connection(self, volume, connector):
initiator_name = connector['initiator']
name = volume['name']
lun_id = self._map_lun(name, initiator_name, 'iscsi', None)
msg = _("Mapped LUN %(name)s to the initiator %(initiator_name)s")
LOG.debug(msg % locals())
msg = (_("Mapped LUN %(name)s to the initiator %(initiator_name)s") %
{'name': name, 'initiator_name': initiator_name})
LOG.debug(msg)
iqn = self._get_iscsi_service_details()
target_details_list = self._get_target_details()
msg = _("Succesfully fetched target details for LUN %(name)s and "
"initiator %(initiator_name)s")
LOG.debug(msg % locals())
msg = (_("Succesfully fetched target details for LUN %(name)s and "
"initiator %(initiator_name)s") %
{'name': name, 'initiator_name': initiator_name})
LOG.debug(msg)

if not target_details_list:
msg = _('Failed to get LUN target details for the LUN %s')
Expand Down Expand Up @@ -1700,9 +1710,10 @@ def create_volume_from_snapshot(self, volume, snapshot):
vol_size = volume['size']
snap_size = snapshot['volume_size']
if vol_size != snap_size:
msg = _('Cannot create volume of size %(vol_size)s from '
'snapshot of size %(snap_size)s')
raise exception.VolumeBackendAPIException(data=msg % locals())
msg = (_('Cannot create volume of size %(vol_size)s from '
'snapshot of size %(snap_size)s') %
{'vol_size': vol_size, 'snap_size': snap_size})
raise exception.VolumeBackendAPIException(data=msg)
snapshot_name = snapshot['name']
new_name = volume['name']
self._clone_lun(snapshot_name, new_name, 'true')
Expand All @@ -1718,9 +1729,10 @@ def terminate_connection(self, volume, connector, **kwargs):
metadata = self._get_lun_attr(name, 'metadata')
path = metadata['Path']
self._unmap_lun(path, initiator_name)
msg = _("Unmapped LUN %(name)s from the initiator "
"%(initiator_name)s")
LOG.debug(msg % locals())
msg = (_("Unmapped LUN %(name)s from the initiator "
"%(initiator_name)s") %
{'name': name, 'initiator_name': initiator_name})
LOG.debug(msg)

def _get_ontapi_version(self):
"""Gets the supported ontapi version."""
Expand Down Expand Up @@ -1810,10 +1822,10 @@ def _map_lun(self, name, initiator, initiator_type='iscsi', lun_id=None):
result = self.client.invoke_successfully(lun_map, True)
return result.get_child_content('lun-id-assigned')
except NaApiError as e:
code = e.code
message = e.message
msg = _('Error mapping lun. Code :%(code)s, Message:%(message)s')
LOG.warn(msg % locals())
msg = (_("Error mapping lun. Code :%(code)s,"
" Message:%(message)s") %
{'code': e.code, 'message': e.message})
LOG.warn(msg)
(igroup, lun_id) = self._find_mapped_lun_igroup(path, initiator)
if lun_id is not None:
return lun_id
Expand All @@ -1830,10 +1842,10 @@ def _unmap_lun(self, path, initiator):
try:
self.client.invoke_successfully(lun_unmap, True)
except NaApiError as e:
msg = _("Error unmapping lun. Code :%(code)s, Message:%(message)s")
code = e.code
message = e.message
LOG.warn(msg % locals())
msg = (_("Error unmapping lun. Code :%(code)s,"
" Message:%(message)s") %
{'code': e.code, 'message': e.message})
LOG.warn(msg)
# if the lun is already unmapped
if e.code == '13115' or e.code == '9016':
pass
Expand Down Expand Up @@ -1937,9 +1949,10 @@ def create_cloned_volume(self, volume, src_vref):
src_vol = self.lun_table[src_vref['name']]
src_vol_size = src_vref['size']
if vol_size != src_vol_size:
msg = _('Cannot clone volume of size %(vol_size)s from '
'src volume of size %(src_vol_size)s')
raise exception.VolumeBackendAPIException(data=msg % locals())
msg = (_("Cannot clone volume of size %(vol_size)s from "
"src volume of size %(src_vol_size)s") %
{'vol_size': vol_size, 'src_vol_size': src_vol_size})
raise exception.VolumeBackendAPIException(data=msg)
new_name = volume['name']
self._clone_lun(src_vol.name, new_name, 'true')

Expand Down Expand Up @@ -2379,7 +2392,7 @@ def _get_lun_list(self):
lun_list.extend(luns)
except NaApiError:
LOG.warn(_("Error finding luns for volume %(vol)s."
" Verify volume exists.") % locals())
" Verify volume exists."), {'vol': vol})
else:
luns = self._get_vol_luns(None)
lun_list.extend(luns)
Expand Down Expand Up @@ -2483,10 +2496,12 @@ def _check_clone_status(self, clone_id, vol_uuid, name, new_name):
if clone_ops_info.get_child_content('clone-state')\
== 'completed':
LOG.debug(_("Clone operation with src %(name)s"
" and dest %(new_name)s completed") % locals())
" and dest %(new_name)s completed"),
{'name': name, 'new_name': new_name})
else:
LOG.debug(_("Clone operation with src %(name)s"
" and dest %(new_name)s failed") % locals())
" and dest %(new_name)s failed"),
{'name': name, 'new_name': new_name})
raise NaApiError(
clone_ops_info.get_child_content('error'),
clone_ops_info.get_child_content('reason'))
Expand Down

0 comments on commit a80003c

Please sign in to comment.