From: Erlon R. Cruz Date: Thu, 18 Dec 2014 18:11:48 +0000 (-0200) Subject: Fix HNAS driver confusing error message (iSCSI driver) X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=450a35e2;p=openstack-build%2Fcinder-build.git Fix HNAS driver confusing error message (iSCSI driver) The error message shown when the parser finds a parser error says, 'file not found' which causes confusion on the user when he/she needs to debug the real cause of the problem. This patch fixes this by testing first if the file exist and then throwing a proper error message. Also corrects an error when appending the config_file_name to the message on the NFS driver and fixes LOG messages according to oslo.i18n guidelines. Closes-Bug: #1403986 Change-Id: Ib6e613796eede751215db54696b353431caaafd1 --- diff --git a/cinder/volume/drivers/hds/iscsi.py b/cinder/volume/drivers/hds/iscsi.py index d33436be2..8a3334ec2 100644 --- a/cinder/volume/drivers/hds/iscsi.py +++ b/cinder/volume/drivers/hds/iscsi.py @@ -17,7 +17,7 @@ """ iSCSI Cinder Volume driver for Hitachi Unified Storage (HUS-HNAS) platform. """ - +import os from xml.etree import ElementTree as ETree from oslo.utils import excutils @@ -25,7 +25,7 @@ from oslo.utils import units from oslo_config import cfg from cinder import exception -from cinder.i18n import _LE, _LI +from cinder.i18n import _LE, _LI, _LW from cinder.openstack.common import log as logging from cinder.volume import driver from cinder.volume.drivers.hds.hnas_backend import HnasBackend @@ -54,7 +54,7 @@ def factory_bend(type): def _loc_info(loc): """Parse info from location string.""" - LOG.info("Parse_loc: %s" % loc) + LOG.info(_LI("Parse_loc: %s"), loc) info = {} tup = loc.split(',') if len(tup) < 5: @@ -70,9 +70,7 @@ def _xml_read(root, element, check=None): try: val = root.findtext(element) - LOG.info(_LI("%(element)s: %(val)s") - % {'element': element, - 'val': val}) + LOG.info(_LI("%(element)s: %(val)s"), {'element': element, 'val': val}) if val: return val.strip() if check: @@ -81,21 +79,24 @@ def _xml_read(root, element, check=None): except ETree.ParseError: if check: with excutils.save_and_reraise_exception(): - LOG.error(_LE("XML exception reading " - "parameter: %s") % element) + LOG.error(_LE("XML exception reading parameter: %s"), element) else: - LOG.info(_LI("XML exception reading parameter: %s") % element) + LOG.info(_LI("XML exception reading parameter: %s"), element) return None def _read_config(xml_config_file): """Read hds driver specific xml config file.""" + if not os.access(xml_config_file, os.R_OK): + raise exception.NotFound(_LE("Can't open config file: %s"), + xml_config_file) + try: root = ETree.parse(xml_config_file).getroot() except Exception: - raise exception.NotFound(message='config file not found: ' - + xml_config_file) + raise exception.ConfigNotFound(_LE("Error parsing config file: %s"), + xml_config_file) # mandatory parameters config = {} @@ -145,7 +146,7 @@ class HDSISCSIDriver(driver.ISCSIDriver): self.type = 'HNAS' self.platform = self.type.lower() - LOG.info(_LI("Backend type: %s") % self.type) + LOG.info(_LI("Backend type: %s"), self.type) self.bend = factory_bend(self.type) def _array_info_get(self): @@ -180,12 +181,8 @@ class HDSISCSIDriver(driver.ISCSIDriver): conf[ip]['ctl'] = ctl conf[ip]['port'] = port conf[ip]['iscsi_port'] = ipp - msg = ('portal: %(ip)s:%(ipp)s, CTL: %(ctl)s, port: %(port)s') - LOG.debug(msg - % {'ip': ip, - 'ipp': ipp, - 'ctl': ctl, - 'port': port}) + msg = "portal: %(ip)s:%(ipp)s, CTL: %(ctl)s, port: %(pt)s" + LOG.debug(msg, {'ip': ip, 'ipp': ipp, 'ctl': ctl, 'pt': port}) return conf @@ -203,9 +200,9 @@ class HDSISCSIDriver(driver.ISCSIDriver): if label not in self.config['services'].keys(): # default works if no match is found label = 'default' - LOG.info(_LI("Using default: instead of %s") % label) - LOG.info(_LI("Available services: %s") - % self.config['services'].keys()) + LOG.info(_LI("Using default: instead of %s"), label) + LOG.info(_LI("Available services: %s"), + self.config['services'].keys()) if label in self.config['services'].keys(): svc = self.config['services'][label] @@ -216,8 +213,7 @@ class HDSISCSIDriver(driver.ISCSIDriver): if self.config['chap_enabled'] == 'True': # it may not exist, create and set secret if 'iscsi_secret' not in svc: - LOG.info(_LI("Retrieving secret for service: %s") - % label) + LOG.info(_LI("Retrieving secret for service: %s"), label) out = self.bend.get_targetsecret(self.config['hnas_cmd'], self.config['mgmt_ip0'], @@ -236,8 +232,8 @@ class HDSISCSIDriver(driver.ISCSIDriver): svc['hdp'], svc['iscsi_secret']) - LOG.info("Set tgt CHAP secret for service: %s" - % (label)) + LOG.info(_LI("Set tgt CHAP secret for service: %s"), + label) else: # We set blank password when the client does not # support CHAP. Later on, if the client tries to create a new @@ -245,12 +241,12 @@ class HDSISCSIDriver(driver.ISCSIDriver): # value and use a temporary dummy password. if 'iscsi_secret' not in svc: # Warns in the first time - LOG.info("CHAP authentication disabled") + LOG.info(_LE("CHAP authentication disabled")) svc['iscsi_secret'] = "" if 'iscsi_target' not in svc: - LOG.info(_LI("Retrieving target for service: %s") % label) + LOG.info(_LI("Retrieving target for service: %s"), label) out = self.bend.get_targetiqn(self.config['hnas_cmd'], self.config['mgmt_ip0'], @@ -267,10 +263,9 @@ class HDSISCSIDriver(driver.ISCSIDriver): svc['port'], svc['hdp'], svc['iscsi_target'], svc['iscsi_secret']) else: - LOG.info(_LI("Available services: %s") - % self.config['services'].keys()) - LOG.error(_LE("No configuration found for service: %s") - % label) + LOG.info(_LI("Available services: %s"), + self.config['services'].keys()) + LOG.error(_LE("No configuration found for service: %s"), label) raise exception.ParameterNotFound(param=label) return service @@ -295,7 +290,8 @@ class HDSISCSIDriver(driver.ISCSIDriver): total_cap += int(size) total_used += int(used) - LOG.info("stats: total: %d used: %d" % (total_cap, total_used)) + LOG.info(_LI("stats: total: %(cap)d used: %(used)d"), + {'cap': total_cap, 'used': total_used}) hnas_stat = {} hnas_stat['total_capacity_gb'] = int(total_cap / units.Ki) # in GB @@ -309,7 +305,7 @@ class HDSISCSIDriver(driver.ISCSIDriver): hnas_stat['QoS_support'] = False hnas_stat['reserved_percentage'] = 0 - LOG.info(_LI("stats: stats: %s") % hnas_stat) + LOG.info(_LI("stats: stats: %s"), hnas_stat) return hnas_stat def _get_hdp_list(self): @@ -332,7 +328,7 @@ class HDSISCSIDriver(driver.ISCSIDriver): hdp_list.extend(inf[1:2]) # returns a list of HDP IDs - LOG.info(_LI("HDP list: %s") % hdp_list) + LOG.info(_LI("HDP list: %s"), hdp_list) return hdp_list def _check_hdp_list(self): @@ -347,7 +343,7 @@ class HDSISCSIDriver(driver.ISCSIDriver): for hdp in lst: if hdp not in hdpl: - LOG.error(_LE("HDP not found: %s") % hdp) + LOG.error(_LE("HDP not found: %s"), hdp) err = "HDP not found: " + hdp raise exception.ParameterNotFound(param=err) # status, verify corresponding status is Normal @@ -383,11 +379,11 @@ class HDSISCSIDriver(driver.ISCSIDriver): self._check_hdp_list() iscsi_info = self._get_iscsi_info() - LOG.info(_LI("do_setup: %s") % iscsi_info) + LOG.info(_LI("do_setup: %s"), iscsi_info) for svc in self.config['services'].keys(): svc_ip = self.config['services'][svc]['iscsi_ip'] if svc_ip in iscsi_info.keys(): - LOG.info(_LI("iSCSI portal found for service: %s") % svc_ip) + LOG.info(_LI("iSCSI portal found for service: %s"), svc_ip) self.config['services'][svc]['port'] = \ iscsi_info[svc_ip]['port'] self.config['services'][svc]['ctl'] = iscsi_info[svc_ip]['ctl'] @@ -395,7 +391,7 @@ class HDSISCSIDriver(driver.ISCSIDriver): iscsi_info[svc_ip]['iscsi_port'] else: # config iscsi address not found on device! LOG.error(_LE("iSCSI portal not found " - "for service: %s") % svc_ip) + "for service: %s"), svc_ip) raise exception.ParameterNotFound(param=svc_ip) def ensure_export(self, context, volume): @@ -408,7 +404,7 @@ class HDSISCSIDriver(driver.ISCSIDriver): """ name = volume['name'] - LOG.debug("create_export %(name)s" % {'name': name}) + LOG.debug("create_export %s", name) pass @@ -420,9 +416,8 @@ class HDSISCSIDriver(driver.ISCSIDriver): provider = volume['provider_location'] name = volume['name'] - LOG.debug("remove_export provider %(provider)s on %(name)s" - % {'provider': provider, - 'name': name}) + LOG.debug("remove_export provider %(provider)s on %(name)s", + {'provider': provider, 'name': name}) pass @@ -441,14 +436,14 @@ class HDSISCSIDriver(driver.ISCSIDriver): '%s' % (int(volume['size']) * units.Ki), volume['name']) - LOG.info(_LI("create_volume: create_lu returns %s") % out) + LOG.info(_LI("create_volume: create_lu returns %s"), out) lun = self.arid + '.' + out.split()[1] sz = int(out.split()[5]) # Example: 92210013.volume-44d7e29b-2aa4-4606-8bc4-9601528149fd - LOG.info(_LI("LUN %(lun)s of size %(sz)s MB is created.") - % {'lun': lun, 'sz': sz}) + LOG.info(_LI("LUN %(lun)s of size %(sz)s MB is created."), + {'lun': lun, 'sz': sz}) return {'provider_location': lun} def create_cloned_volume(self, dst, src): @@ -475,9 +470,8 @@ class HDSISCSIDriver(driver.ISCSIDriver): lun = self.arid + '.' + out.split()[1] size = int(out.split()[5]) - LOG.debug("LUN %(lun)s of size %(size)s MB is cloned." - % {'lun': lun, - 'size': size}) + LOG.debug("LUN %(lun)s of size %(size)s MB is cloned.", + {'lun': lun, 'size': size}) return {'provider_location': lun} def extend_volume(self, volume, new_size): @@ -498,8 +492,8 @@ class HDSISCSIDriver(driver.ISCSIDriver): '%s' % (new_size * units.Ki), volume['name']) - LOG.info(_LI("LUN %(lun)s extended to %(size)s GB.") - % {'lun': lun, 'size': new_size}) + LOG.info(_LI("LUN %(lun)s extended to %(size)s GB."), + {'lun': lun, 'size': new_size}) def delete_volume(self, volume): """Delete an LU on HNAS. @@ -508,12 +502,12 @@ class HDSISCSIDriver(driver.ISCSIDriver): prov_loc = volume['provider_location'] if prov_loc is None: - LOG.error("delete_vol: provider location empty.") + LOG.error(_LE("delete_vol: provider location empty.")) return info = _loc_info(prov_loc) (arid, lun) = info['id_lu'] if 'tgt' in info.keys(): # connected? - LOG.info("delete lun loc %s" % info['tgt']) + LOG.info(_LI("delete lun loc %s"), info['tgt']) # loc = id.lun (_portal, iqn, loc, ctl, port, hlun) = info['tgt'] self.bend.del_iscsi_conn(self.config['hnas_cmd'], @@ -524,9 +518,7 @@ class HDSISCSIDriver(driver.ISCSIDriver): name = self.hnas_name - LOG.debug("delete lun %(lun)s on %(name)s" - % {'lun': lun, - 'name': name}) + LOG.debug("delete lun %(lun)s on %(name)s", {'lun': lun, 'name': name}) service = self._get_service(volume) (_ip, _ipp, _ctl, _port, hdp, target, secret) = service @@ -543,7 +535,8 @@ class HDSISCSIDriver(driver.ISCSIDriver): :param connector: dictionary connector reference """ - LOG.info("initialize volume %s connector %s" % (volume, connector)) + LOG.info(_LI("initialize volume %(vol)s connector %(conn)s"), + {'vol': volume, 'conn': connector}) # connector[ip, host, wwnns, unititator, wwp/ service = self._get_service(volume) @@ -571,7 +564,7 @@ class HDSISCSIDriver(driver.ISCSIDriver): tgt = hnas_portal + ',' + iqn + ',' + loc + ',' + ctl + ',' tgt += port + ',' + hlun - LOG.info("initiate: connection %s" % tgt) + LOG.info(_LI("initiate: connection %s"), tgt) properties = {} properties['provider_location'] = tgt @@ -598,11 +591,11 @@ class HDSISCSIDriver(driver.ISCSIDriver): info = _loc_info(volume['provider_location']) if 'tgt' not in info.keys(): # spurious disconnection - LOG.warn("terminate_conn: provider location empty.") + LOG.warn(_LW("terminate_conn: provider location empty.")) return (arid, lun) = info['id_lu'] (_portal, iqn, loc, ctl, port, hlun) = info['tgt'] - LOG.info("terminate: connection %s" % volume['provider_location']) + LOG.info(_LI("terminate: connection %s"), volume['provider_location']) self.bend.del_iscsi_conn(self.config['hnas_cmd'], self.config['mgmt_ip0'], self.config['username'], @@ -632,8 +625,8 @@ class HDSISCSIDriver(driver.ISCSIDriver): lun = self.arid + '.' + out.split()[1] sz = int(out.split()[5]) - LOG.debug("LUN %(lun)s of size %(sz)s MB is created from snapshot." - % {'lun': lun, 'sz': sz}) + LOG.debug("LUN %(lun)s of size %(sz)s MB is created from snapshot.", + {'lun': lun, 'sz': sz}) return {'provider_location': lun} def create_snapshot(self, snapshot): @@ -656,8 +649,8 @@ class HDSISCSIDriver(driver.ISCSIDriver): lun = self.arid + '.' + out.split()[1] size = int(out.split()[5]) - LOG.debug("LUN %(lun)s of size %(size)s MB is created." - % {'lun': lun, 'size': size}) + LOG.debug("LUN %(lun)s of size %(size)s MB is created.", + {'lun': lun, 'size': size}) return {'provider_location': lun} def delete_snapshot(self, snapshot): @@ -680,9 +673,8 @@ class HDSISCSIDriver(driver.ISCSIDriver): myid = self.arid if arid != myid: - LOG.error(_LE('Array mismatch %(myid)s vs %(arid)s') - % {'myid': myid, - 'arid': arid}) + LOG.error(_LE("Array mismatch %(myid)s vs %(arid)s"), + {'myid': myid, 'arid': arid}) msg = 'Array id mismatch in delete snapshot' raise exception.VolumeBackendAPIException(data=msg) self.bend.delete_lu(self.config['hnas_cmd'], diff --git a/cinder/volume/drivers/hds/nfs.py b/cinder/volume/drivers/hds/nfs.py index b52244605..d265744f3 100644 --- a/cinder/volume/drivers/hds/nfs.py +++ b/cinder/volume/drivers/hds/nfs.py @@ -59,9 +59,7 @@ def _xml_read(root, element, check=None): try: val = root.findtext(element) - LOG.info(_LI("%(element)s: %(val)s") - % {'element': element, - 'val': val}) + LOG.info(_LI("%(element)s: %(val)s"), {'element': element, 'val': val}) if val: return val.strip() if check: @@ -70,9 +68,9 @@ def _xml_read(root, element, check=None): except ETree.ParseError: if check: with excutils.save_and_reraise_exception(): - LOG.error(_LE("XML exception reading parameter: %s") % element) + LOG.error(_LE("XML exception reading parameter: %s"), element) else: - LOG.info(_LI("XML exception reading parameter: %s") % element) + LOG.info(_LI("XML exception reading parameter: %s"), element) return None @@ -83,14 +81,14 @@ def _read_config(xml_config_file): """ if not os.access(xml_config_file, os.R_OK): - raise exception.NotFound(message=_LE('Can\'t open config file: ') - + xml_config_file) + raise exception.NotFound(_LE("Can't open config file: %s"), + xml_config_file) try: root = ETree.parse(xml_config_file).getroot() except Exception: - raise exception.ConfigNotFound( - message=_LE('Error parsing config file: ') + xml_config_file) + raise exception.ConfigNotFound(_LE("Error parsing config file: %s"), + xml_config_file) # mandatory parameters config = {} @@ -188,12 +186,14 @@ class HDSNFSDriver(nfs.NfsDriver): label = 'default' if label in self.config['services'].keys(): svc = self.config['services'][label] - LOG.info("Get service: %s->%s" % (label, svc['fslabel'])) + LOG.info(_LI("Get service: %(lbl)s->%(svc)s"), + {'lbl': label, 'svc': svc['fslabel']}) service = (svc['hdp'], svc['path'], svc['fslabel']) else: - LOG.info(_LI("Available services: %s") - % self.config['services'].keys()) - LOG.error(_LE("No configuration found for service: %s") % label) + LOG.info(_LI("Available services: %s"), + self.config['services'].keys()) + LOG.error(_LE("No configuration found for service: %s"), + label) raise exception.ParameterNotFound(param=label) return service @@ -212,20 +212,20 @@ class HDSNFSDriver(nfs.NfsDriver): path = self._get_volume_path(nfs_mount, volume['name']) # Resize the image file on share to new size. - LOG.debug('Checking file for resize') + LOG.debug("Checking file for resize") if self._is_file_size_equal(path, new_size): return else: - LOG.info(_LI('Resizing file to %sG'), new_size) + LOG.info(_LI("Resizing file to %sG"), new_size) image_utils.resize_image(path, new_size) if self._is_file_size_equal(path, new_size): - LOG.info(_LI("LUN %(id)s extended to %(size)s GB.") - % {'id': volume['id'], 'size': new_size}) + LOG.info(_LI("LUN %(id)s extended to %(size)s GB."), + {'id': volume['id'], 'size': new_size}) return else: raise exception.InvalidResults( - _('Resizing image file failed.')) + _("Resizing image file failed.")) def _is_file_size_equal(self, path, size): """Checks if file size at path is equal to size.""" @@ -241,13 +241,13 @@ class HDSNFSDriver(nfs.NfsDriver): def create_volume_from_snapshot(self, volume, snapshot): """Creates a volume from a snapshot.""" - LOG.debug('create_volume_from %s', volume) + LOG.debug("create_volume_from %s", volume) 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') + msg = _("Cannot create volume of size %(vol_size)s from " + "snapshot of size %(snap_size)s") msg_fmt = {'vol_size': vol_size, 'snap_size': snap_size} raise exception.CinderException(msg % msg_fmt) @@ -380,8 +380,8 @@ class HDSNFSDriver(nfs.NfsDriver): src_vol_size = src_vref['size'] if vol_size != src_vol_size: - msg = _('Cannot create clone of size %(vol_size)s from ' - 'volume of size %(src_vol_size)s') + msg = _("Cannot create clone of size %(vol_size)s from " + "volume of size %(src_vol_size)s") msg_fmt = {'vol_size': vol_size, 'src_vol_size': src_vol_size} raise exception.CinderException(msg % msg_fmt) @@ -425,13 +425,10 @@ class HDSNFSDriver(nfs.NfsDriver): conf[key]['path'] = path conf[key]['hdp'] = hdp conf[key]['fslabel'] = fslabel - msg = _('nfs_info: %(key)s: %(path)s, HDP: \ - %(fslabel)s FSID: %(hdp)s') - LOG.info(msg - % {'key': key, - 'path': path, - 'fslabel': fslabel, - 'hdp': hdp}) + msg = _("nfs_info: %(key)s: %(path)s, HDP: \ + %(fslabel)s FSID: %(hdp)s") + LOG.info(msg, {'key': key, 'path': path, 'fslabel': fslabel, + 'hdp': hdp}) return conf @@ -442,14 +439,15 @@ class HDSNFSDriver(nfs.NfsDriver): self._load_shares_config(getattr(self.configuration, self.driver_prefix + '_shares_config')) - LOG.info("Review shares: %s" % self.shares) + LOG.info(_LI("Review shares: %s"), self.shares) nfs_info = self._get_nfs_info() for share in self.shares: #export = share.split(':')[1] if share in nfs_info.keys(): - LOG.info("share: %s -> %s" % (share, nfs_info[share]['path'])) + LOG.info(_LI("share: %(share)s -> %(info)s"), + {'share': share, 'info': nfs_info[share]['path']}) for svc in self.config['services'].keys(): if share == self.config['services'][svc]['hdp']: @@ -460,17 +458,19 @@ class HDSNFSDriver(nfs.NfsDriver): nfs_info[share]['hdp'] self.config['services'][svc]['fslabel'] = \ nfs_info[share]['fslabel'] - LOG.info("Save service info for %s -> %s, %s" - % (svc, nfs_info[share]['hdp'], - nfs_info[share]['path'])) + LOG.info(_LI("Save service info for" + " %(svc)s -> %(hdp)s, %(path)s"), + {'svc': svc, 'hdp': nfs_info[share]['hdp'], + 'path': nfs_info[share]['path']}) break if share != self.config['services'][svc]['hdp']: - LOG.error("NFS share %s has no service entry: %s -> %s" - % (share, svc, - self.config['services'][svc]['hdp'])) + LOG.error(_LE("NFS share %(share)s has no service entry:" + " %(svc)s -> %(hdp)s"), + {'share': share, 'svc': svc, + 'hdp': self.config['services'][svc]['hdp']}) raise exception.ParameterNotFound(param=svc) else: - LOG.info("share: %s incorrect entry" % share) + LOG.info(_LI("share: %s incorrect entry"), share) def _clone_volume(self, volume_name, clone_name, volume_id): """Clones mounted volume using the HNAS file_clone. @@ -482,8 +482,10 @@ class HDSNFSDriver(nfs.NfsDriver): export_path = self._get_export_path(volume_id) # volume-ID snapshot-ID, /cinder - LOG.info("Cloning with volume_name %s clone_name %s export_path %s" - % (volume_name, clone_name, export_path)) + LOG.info(_LI("Cloning with volume_name %(vname)s clone_name %(cname)s" + " export_path %(epath)s"), {'vname': volume_name, + 'cname': clone_name, + 'epath': export_path}) source_vol = self._id_to_vol(volume_id) # sps; added target