From: John Griffith Date: Fri, 19 Dec 2014 19:25:30 +0000 (+0000) Subject: Fix format errors in brick/iscsi LOG messages X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=3e1ea2c99859d1304a1db2562d931e85dbfb9093;p=openstack-build%2Fcinder-build.git Fix format errors in brick/iscsi LOG messages Currently in cinder.brick.iscsi.iscs:LioADM.create_iscsi_target The Log message for the exception is: LOG.error("%s" % e), This rightfully results in: *** UnicodeError: UnicodeError(u'Message objects do not support str() because they may contain non-ascii characters. Please use unicode() or translate() instead.',) In some cases this causes the Volume service to stop and doesn't help a ton with debug. While looking at this also noticed a number of other similar cases where invalid LOG messages were set up. Following the i8n guidelines here: http://docs.openstack.org/developer/oslo.i18n/guidelines.html Went ahead and cleaned the bulk of the LOG messages in this file up to adhere to the guidelines as well as fixing the UnicodeError described in the bug. Change-Id: I33adf99ca25a2d8f869de5bfa85b4ca8429be05e --- diff --git a/cinder/brick/iscsi/iscsi.py b/cinder/brick/iscsi/iscsi.py index 4f6822661..934cdc290 100644 --- a/cinder/brick/iscsi/iscsi.py +++ b/cinder/brick/iscsi/iscsi.py @@ -165,13 +165,11 @@ class TgtAdm(TargetAdmin): 'logicalunit', '--tid', tid, '--lun', '1', '-b', path, run_as_root=True) - LOG.debug('StdOut from recreate backing lun: %s' % out) - LOG.debug('StdErr from recreate backing lun: %s' % err) except putils.ProcessExecutionError as e: LOG.error(_LE("Failed to recover attempt to create " - "iscsi backing lun for volume " - "id:%(vol_id)s: %(e)s") - % {'vol_id': name, 'e': e}) + "iscsi backing lun for Volume " + "ID: %(vol_id)s: %(e)s"), + {'vol_id': name, 'e': e}) def _get_target_chap_auth(self, name): volumes_dir = self.volumes_dir @@ -185,14 +183,14 @@ class TgtAdm(TargetAdmin): # NOTE(jdg): Debug is ok here because the caller # will just generate the CHAP creds and create the # file based on the None return - LOG.debug('Failed to open config for %(vol_id)s: %(e)s' - % {'vol_id': vol_id, 'e': six.text_type(e)}) + LOG.debug('Failed to open config for %(vol_id)s: %(e)s', + {'vol_id': vol_id, 'e': six.text_type(e)}) return None m = re.search('incominguser (\w+) (\w+)', volume_conf) if m: return (m.group(1), m.group(2)) - LOG.debug('Failed to find CHAP auth from config for %s' % vol_id) + LOG.debug('Failed to find CHAP auth from config for %s', vol_id) return None def create_iscsi_target(self, name, tid, lun, path, @@ -212,7 +210,7 @@ class TgtAdm(TargetAdmin): path, chap_str, write_cache) - LOG.info(_LI('Creating iscsi_target for: %s') % vol_id) + LOG.info(_LI('Creating iscsi_target for: %s'), vol_id) volumes_dir = self.volumes_dir volume_path = os.path.join(volumes_dir, vol_id) @@ -220,8 +218,8 @@ class TgtAdm(TargetAdmin): f.write(volume_conf) f.close() LOG.debug('Created volume path %(vp)s,\n' - 'content: %(vc)s' - % {'vp': volume_path, 'vc': volume_conf}) + 'content: %(vc)s', + {'vp': volume_path, 'vc': volume_conf}) old_persist_file = None old_name = kwargs.get('old_name', None) @@ -235,8 +233,6 @@ class TgtAdm(TargetAdmin): # created. (out, err) = self._run('tgt-admin', '--update', name, run_as_root=True) - LOG.debug("StdOut from tgt-admin --update: %s", out) - LOG.debug("StdErr from tgt-admin --update: %s", err) # Grab targets list for debug # Consider adding a check for lun 0 and 1 for tgtadm @@ -249,12 +245,13 @@ class TgtAdm(TargetAdmin): '--mode', 'target', run_as_root=True) - LOG.debug("Targets after update: %s" % out) + LOG.debug("Targets after update: %s", out) except putils.ProcessExecutionError as e: LOG.warning(_LW("Failed to create iscsi target for volume " - "id:%(vol_id)s: %(e)s") - % {'vol_id': vol_id, 'e': e}) + "id:%(vol_id)s: %(e)s"), + {'vol_id': vol_id, 'e': e.stderr}) if "target already exists" in e.stderr: + LOG.warning(_LW('Create iscsi target failed for ' 'target already exists')) # NOTE(jdg): We've run into some cases where the cmd being @@ -273,9 +270,9 @@ class TgtAdm(TargetAdmin): iqn = '%s%s' % (self.iscsi_target_prefix, vol_id) tid = self._get_target(iqn) if tid is None: - LOG.error(_LE("Failed to create iscsi target for volume " - "id:%(vol_id)s. Please ensure your tgtd config file " - "contains 'include %(volumes_dir)s/*'") % { + LOG.error(_LE("Failed to create iscsi target for Volume " + "ID: %(vol_id)s. Ensure the tgtd config file " + "contains 'include %(volumes_dir)s/*'"), { 'vol_id': vol_id, 'volumes_dir': volumes_dir, }) raise exception.NotFound() @@ -302,12 +299,12 @@ class TgtAdm(TargetAdmin): return tid def remove_iscsi_target(self, tid, lun, vol_id, vol_name, **kwargs): - LOG.info(_LI('Removing iscsi_target for: %s') % vol_id) + LOG.info(_LI('Removing iscsi_target for: %s'), vol_id) vol_uuid_file = vol_name volume_path = os.path.join(self.volumes_dir, vol_uuid_file) if not os.path.exists(volume_path): LOG.warning(_LW('Volume path %s does not exist, ' - 'nothing to remove.') % volume_path) + 'nothing to remove.'), volume_path) return if os.path.isfile(volume_path): @@ -325,9 +322,9 @@ class TgtAdm(TargetAdmin): run_as_root=True, attempts=CONF.num_shell_tries) except putils.ProcessExecutionError as e: - LOG.error(_LE("Failed to remove iscsi target for volume " - "id:%(vol_id)s: %(e)s") - % {'vol_id': vol_id, 'e': e}) + LOG.error(_LE("Failed to remove iscsi target for Volume " + "ID: %(vol_id)s: %(e)s"), + {'vol_id': vol_id, 'e': e}) raise exception.ISCSITargetRemoveFailed(volume_id=vol_id) # NOTE(jdg): There's a bug in some versions of tgt that @@ -349,9 +346,9 @@ class TgtAdm(TargetAdmin): iqn, run_as_root=True) except putils.ProcessExecutionError as e: - LOG.error(_LE("Failed to remove iscsi target for volume " - "id:%(vol_id)s: %(e)s") - % {'vol_id': vol_id, 'e': e}) + LOG.error(_LE("Failed to remove iscsi target for Volume " + "ID: %(vol_id)s: %(e)s"), + {'vol_id': vol_id, 'e': e}) raise exception.ISCSITargetRemoveFailed(volume_id=vol_id) # NOTE(jdg): This *should* be there still but incase @@ -362,7 +359,7 @@ class TgtAdm(TargetAdmin): os.unlink(volume_path) else: LOG.debug('Volume path %s not found at end, ' - 'of remove_iscsi_target.' % volume_path) + 'of remove_iscsi_target.', volume_path) def show_target(self, tid, iqn=None, **kwargs): if iqn is None: @@ -420,14 +417,14 @@ class IetAdm(TargetAdmin): f.close() except putils.ProcessExecutionError as e: vol_id = name.split(':')[1] - LOG.error(_LE("Failed to create iscsi target for volume " - "id:%(vol_id)s: %(e)s") - % {'vol_id': vol_id, 'e': e}) + LOG.error(_LE("Failed to create iscsi target for Volume " + "ID: %(vol_id)s: %(e)s"), + {'vol_id': vol_id, 'e': e}) raise exception.ISCSITargetCreateFailed(volume_id=vol_id) return tid def remove_iscsi_target(self, tid, lun, vol_id, vol_name, **kwargs): - LOG.info(_LI('Removing iscsi_target for volume: %s') % vol_id) + LOG.info(_LI('Removing iscsi_target for volume: %s'), vol_id) self._delete_logicalunit(tid, lun, **kwargs) self._delete_target(tid, **kwargs) vol_uuid_file = vol_name @@ -540,7 +537,7 @@ class LioAdm(TargetAdmin): vol_id = name.split(':')[1] - LOG.info(_LI('Creating iscsi_target for volume: %s') % vol_id) + LOG.info(_LI('Creating iscsi_target for volume: %s'), vol_id) if chap_auth is not None: (chap_auth_userid, chap_auth_password) = chap_auth.split(' ')[1:] @@ -553,23 +550,23 @@ class LioAdm(TargetAdmin): chap_auth_password] self._run('cinder-rtstool', *command_args, run_as_root=True) except putils.ProcessExecutionError as e: - LOG.error(_LE("Failed to create iscsi target for volume " - "id:%s.") % vol_id) - LOG.error("%s" % e) + LOG.error(_LE("Failed to create iscsi target for Volume " + "ID: %(vol_id)s, Error: %(err)s."), + {'vol_id': vol_id, 'err': e.stderr}) raise exception.ISCSITargetCreateFailed(volume_id=vol_id) iqn = '%s%s' % (self.iscsi_target_prefix, vol_id) tid = self._get_target(iqn) if tid is None: - LOG.error(_LE("Failed to create iscsi target for volume " - "id:%s.") % vol_id) + LOG.error(_LE("Failed to create iscsi target for Volume " + "ID: %s."), vol_id) raise exception.NotFound() return tid def remove_iscsi_target(self, tid, lun, vol_id, vol_name, **kwargs): - LOG.info(_LI('Removing iscsi_target: %s') % vol_id) + LOG.info(_LI('Removing iscsi_target: %s'), vol_id) vol_uuid_name = vol_name iqn = '%s%s' % (self.iscsi_target_prefix, vol_uuid_name) @@ -579,9 +576,9 @@ class LioAdm(TargetAdmin): iqn, run_as_root=True) except putils.ProcessExecutionError as e: - LOG.error(_LE("Failed to remove iscsi target for volume " - "id:%s.") % vol_id) - LOG.error("%s" % e) + LOG.error(_LE("Failed to remove iscsi target for Volume " + "ID: %(vol_id)s, Error: %(err)s."), + {'vol_id': vol_id, 'err': e.stderr}) raise exception.ISCSITargetRemoveFailed(volume_id=vol_id) def show_target(self, tid, iqn=None, **kwargs): @@ -608,7 +605,7 @@ class LioAdm(TargetAdmin): connector['initiator'], run_as_root=True) except putils.ProcessExecutionError: - LOG.error(_LE("Failed to add initiator iqn %s to target.") % + LOG.error(_LE("Failed to add initiator iqn %s to target."), connector['initiator']) raise exception.ISCSITargetAttachFailed(volume_id=volume['id']) @@ -622,7 +619,7 @@ class LioAdm(TargetAdmin): connector['initiator'], run_as_root=True) except putils.ProcessExecutionError: - LOG.error(_LE("Failed to delete initiator iqn %s to target.") % + LOG.error(_LE("Failed to delete initiator iqn %s to target."), connector['initiator']) raise exception.ISCSITargetAttachFailed(volume_id=volume['id'])