From a3848ed80d7306833a87b1f7cfa3f89b58ab744a Mon Sep 17 00:00:00 2001 From: zhangchao010 Date: Tue, 26 Mar 2013 22:10:19 +0800 Subject: [PATCH] Change format of some judgments Because "not volume" will be True if volume id is 0.It can not distinguish 0 from None.The same to some other judgments where variable value may be 0. This patch also does the following changes: Deletes check storagepool in function _check_conf_file and adds check default iscsi target IP in function initialize_connection. Moves create host group from do_setup to initialize_connection. Changes _get_login_info to be _update_login_info. Fixes bug: 1160340 Change-Id: I0434b587d57c783f10d5724c4c66aea5df0b32e5 --- cinder/volume/drivers/huawei/huawei_iscsi.py | 144 ++++++++++--------- 1 file changed, 76 insertions(+), 68 deletions(-) diff --git a/cinder/volume/drivers/huawei/huawei_iscsi.py b/cinder/volume/drivers/huawei/huawei_iscsi.py index 956c2730d1b..ad2961eba32 100644 --- a/cinder/volume/drivers/huawei/huawei_iscsi.py +++ b/cinder/volume/drivers/huawei/huawei_iscsi.py @@ -41,7 +41,7 @@ HOST_GROUP_NAME = 'HostGroup_OpenStack' HOST_NAME_PREFIX = 'Host_' HOST_PORT_PREFIX = 'HostPort_' -VOL_AND_SNAP_NAME_FREFIX = 'OpenStack_' +VOL_AND_SNAP_NAME_PREFIX = 'OpenStack_' READBUFFERSIZE = 8192 @@ -117,13 +117,6 @@ def do_setup(self, context): LOG.debug(_('do_setup.')) self._check_conf_file() - # Create hostgroup. - self.login_info = self._get_login_info() - hostgroup_name = HOST_GROUP_NAME - self.hostgroup_id = self._find_hostgroup(hostgroup_name) - if not self.hostgroup_id: - self._create_hostgroup(hostgroup_name) - self.hostgroup_id = self._find_hostgroup(hostgroup_name) def check_for_setup_error(self): """Try to connect with device and get device type.""" @@ -148,6 +141,17 @@ def check_for_setup_error(self): LOG.error(err_msg) raise exception.VolumeBackendAPIException(data=err_msg) + # Check whether storage pools are configured. + # Dorado2100 G2 needn't to configure this. + if self.device_type['type'] != 'Dorado2100 G2': + root = self._read_xml() + pool_node = root.findall('LUN/StoragePool') + if not pool_node: + err_msg = (_('_get_device_type: Storage Pool must be' + 'configured.')) + LOG.error(err_msg) + raise exception.VolumeBackendAPIException(data=err_msg) + def create_volume(self, volume): """Create a new volume.""" volume_name = self._name_translate(volume['name']) @@ -156,11 +160,11 @@ def create_volume(self, volume): self.login_info = self._get_login_info() if int(volume['size']) == 0: - volume_sise = '100M' + volume_size = '100M' else: - volume_sise = '%sG' % volume['size'] + volume_size = '%sG' % volume['size'] - self._create_volume(volume_name, volume_sise) + self._create_volume(volume_name, volume_size) def delete_volume(self, volume): """Delete a volume.""" @@ -170,7 +174,7 @@ def delete_volume(self, volume): self.login_info = self._get_login_info() volume_id = self._find_lun(volume_name) - if volume_id: + if volume_id is not None: self._delete_volume(volume_name, volume_id) else: err_msg = (_('delete_volume:No need to delete volume.' @@ -185,7 +189,7 @@ def create_export(self, context, volume): LOG.debug(_('create_export: volume name:%s') % volume['name']) lun_id = self._find_lun(volume_name) - if not lun_id: + if lun_id is None: err_msg = (_('create_export:Volume %(name)s does not exist.') % {'name': volume_name}) LOG.error(err_msg) @@ -211,9 +215,42 @@ def initialize_connection(self, volume, connector): % {'volume': volume_name, 'ini': initiator_name}) + self.login_info = self._get_login_info() + # Get target iSCSI iqn. + iscsi_conf = self._get_iscsi_info() + target_ip = None + for ini in iscsi_conf['Initiator']: + if ini['Name'] == initiator_name: + target_ip = ini['TargetIP'] + break + if not target_ip: + if not iscsi_conf['DefaultTargetIP']: + err_msg = (_('initialize_connection:Failed to find target ip' + 'for initiator:%(initiatorname)s,' + 'please check config file.') + % {'initiatorname': initiator_name}) + LOG.error(err_msg) + raise exception.VolumeBackendAPIException(data=err_msg) + target_ip = iscsi_conf['DefaultTargetIP'] + + target_iqn = self._get_tgt_iqn(target_ip) + if not target_iqn: + err_msg = (_('initialize_connection:Failed to find target iSCSI' + 'iqn. Target IP:%(ip)s') + % {'ip': target_ip}) + LOG.error(err_msg) + raise exception.VolumeBackendAPIException(data=err_msg) + + # Create hostgroup and host. + hostgroup_name = HOST_GROUP_NAME + self.hostgroup_id = self._find_hostgroup(hostgroup_name) + if self.hostgroup_id is None: + self._create_hostgroup(hostgroup_name) + self.hostgroup_id = self._find_hostgroup(hostgroup_name) + host_name = HOST_NAME_PREFIX + str(hash(initiator_name)) host_id = self._find_host_in_hostgroup(host_name, self.hostgroup_id) - if not host_id: + if host_id is None: self._add_host(host_name, self.hostgroup_id) host_id = self._find_host_in_hostgroup(host_name, self.hostgroup_id) @@ -243,26 +280,9 @@ def initialize_connection(self, volume, connector): 'ini': initiator_name, 'port': port_name}) - # Get target iSCSI iqn. - iscsi_conf = self._get_iscsi_info() - target_ip = None - for ini in iscsi_conf['Initiator']: - if ini['Name'] == initiator_name: - target_ip = ini['TargetIP'] - break - if not target_ip: - target_ip = iscsi_conf['DefaultTargetIP'] - - target_iqn = self._get_tgt_iqn(target_ip) - if not target_iqn: - err_msg = (_('initialize_connection:' - 'Failed to find target iSCSI iqn.')) - LOG.error(err_msg) - raise exception.VolumeBackendAPIException(data=err_msg) - # Map a LUN to a host if not mapped. lun_id = self._find_lun(volume_name) - if not lun_id: + if lun_id is None: err_msg = (_('initialize_connection:Failed to find the ' 'given volume. ' 'volume name:%(volume)s.') @@ -319,7 +339,7 @@ def terminate_connection(self, volume, connector, **kwargs): self.login_info = self._get_login_info() host_name = HOST_NAME_PREFIX + str(hash(initiator_name)) host_id = self._find_host_in_hostgroup(host_name, self.hostgroup_id) - if not host_id: + if host_id is None: err_msg = (_('terminate_connection:Host does not exist. ' 'Host name:%(host)s.') % {'host': host_name}) @@ -328,7 +348,7 @@ def terminate_connection(self, volume, connector, **kwargs): # Delete host map. lun_id = self._find_lun(volume_name) - if not lun_id: + if lun_id is None: err_msg = (_('terminate_connection:volume does not exist.' 'volume name:%(volume)s') % {'volume': volume_name}) @@ -344,7 +364,7 @@ def terminate_connection(self, volume, connector, **kwargs): if map['devlunid'] == lun_id: map_id = map['mapid'] break - if map_id: + if map_id is not None: self._delete_map(map_id) mapnum = mapnum - 1 else: @@ -398,7 +418,7 @@ def create_snapshot(self, snapshot): raise exception.VolumeBackendAPIException(data=err_msg) lun_id = self._find_lun(volume_name) - if not lun_id: + if lun_id is None: err_msg = (_('create_snapshot:Volume does not exist.' 'Volume name:%(name)s') % {'name': volume_name}) @@ -432,7 +452,7 @@ def delete_snapshot(self, snapshot): raise exception.VolumeBackendAPIException(data=err_msg) snapshot_id = self._find_snapshot(snapshot_name) - if snapshot_id: + if snapshot_id is not None: self._disable_snapshot(snapshot_id) self._delete_snapshot(snapshot_id) else: @@ -467,7 +487,7 @@ def create_volume_from_snapshot(self, volume, snapshot): raise exception.VolumeBackendAPIException(data=err_msg) snapshot_id = self._find_snapshot(snapshot_name) - if not snapshot_id: + if snapshot_id is None: err_msg = (_('create_volume_from_snapshot:Snapshot does not exist.' 'Snapshot name:%(name)s') % {'name': snapshot_name}) @@ -514,22 +534,13 @@ def _check_conf_file(self): IP2 = root.findtext('Storage/ControllerIP1') username = root.findtext('Storage/UserName') pwd = root.findtext('Storage/UserPassword') - pool = root.findall('LUN/StoragePool') isconfwrong = False if ((not IP1 and not IP2) or (not username) or - (not pwd) or - (not pool)): - isconfwrong = True - - elif not pool[0].attrib['Name']: - isconfwrong = True - - if isconfwrong is True: - err_msg = (_('Config file is wrong. ' - 'Controler IP, UserName, UserPassword and ' - 'StoragePool must be set.')) + (not pwd)): + err_msg = (_('Config file is wrong. Controler IP, ' + 'UserName and UserPassword must be set.')) LOG.error(err_msg) raise exception.InvalidInput(reason=err_msg) @@ -560,7 +571,7 @@ def _get_login_info(self): logininfo['UserPassword'] = root.findtext('Storage/UserPassword') except Exception as err: - LOG.debug(_('_get_login_info error. %s') % err) + LOG.error(_('_get_login_info error. %s') % err) raise exception.VolumeBackendAPIException(data=err) return logininfo @@ -581,7 +592,7 @@ def _get_lun_set_info(self): luntype = root.findtext('LUN/LUNType') if luntype in ['Thick', 'Thin']: lunsetinfo['LUNType'] = luntype - elif luntype is not '' or luntype is not None: + elif luntype: err_msg = (_('Config file is wrong. LUNType must be "Thin"' ' or "Thick". LUNType:%(type)s') % {'type': luntype}) @@ -590,20 +601,19 @@ def _get_lun_set_info(self): # Here we do not judge whether the parameters are right. # CLI will return error responses if the parameters not right. stripunitsize = root.findtext('LUN/StripUnitSize') - if stripunitsize is not '' and stripunitsize is not None: + if stripunitsize: lunsetinfo['StripUnitSize'] = stripunitsize writetype = root.findtext('LUN/WriteType') - if writetype is not '' and writetype is not None: + if writetype: lunsetinfo['WriteType'] = writetype mirrorswitch = root.findtext('LUN/MirrorSwitch') - if mirrorswitch is not '' and mirrorswitch is not None: + if mirrorswitch: lunsetinfo['MirrorSwitch'] = mirrorswitch if self.device_type['type'] == 'Tseries': - pooltype = luntype + pooltype = lunsetinfo['LUNType'] prefetch = root.find('LUN/Prefetch') - if ((prefetch is not None) and - (prefetch.attrib['Type'] != '')): + if prefetch and prefetch.attrib['Type']: lunsetinfo['PrefetchType'] = prefetch.attrib['Type'] if lunsetinfo['PrefetchType'] == '1': lunsetinfo['PrefetchValue'] = prefetch.attrib['Value'] @@ -678,7 +688,7 @@ def _get_maximum_pool(self, poolinconf, poolindev, luntype): maxpoolid = pooldetail[0] maxpoolsize = int(float(pooldetail[sizeindex])) break - if maxpoolid: + if maxpoolid is not None: return maxpoolid else: err_msg = (_('_get_maximum_pool:maxpoolid is None.' @@ -751,7 +761,7 @@ def _execute_cli(self, cmd): def _name_translate(self, name): """Form new names because of the 32-character limit on names.""" - newname = VOL_AND_SNAP_NAME_FREFIX + str(hash(name)) + newname = VOL_AND_SNAP_NAME_PREFIX + str(hash(name)) LOG.debug(_('_name_translate:Name in cinder: %(old)s, ' 'new name in storage system: %(new)s') @@ -776,7 +786,7 @@ def _find_lun(self, name): d = 0 for i in range(6, len(en) - 2): - r = en[i].split() + r = en[i].replace('Not format', 'Notformat').split() if r[6 - d] == name: return r[0] return None @@ -994,7 +1004,7 @@ def _map_lun(self, lunid, hostid, new_hostlun_id): Here we give the hostlun ID which starts from 1. """ - cli_cmd = ('addhostmap -host %(hostid)s -devlun %(lunid)s' + cli_cmd = ('addhostmap -host %(hostid)s -devlun %(lunid)s ' '-hostlun %(hostlunid)s' % {'hostid': hostid, 'lunid': lunid, @@ -1057,7 +1067,7 @@ def _delete_host(self, hostid): % {'hostid': hostid}) out = self._execute_cli(cli_cmd) if not re.search('command operates successfully', out): - err_msg = (_('Error: Failed delete host,host id: %(hostid)s.' + err_msg = (_('_delete_host: Failed delete host.host id:%(hostid)s.' 'out:%(out)s') % {'hostid': hostid, 'out': out}) @@ -1407,14 +1417,12 @@ def _update_volume_status(self): def _get_free_capacity(self): """Get total free capacity of pools.""" self.login_info = self._get_login_info() - self.device_type = self._get_device_type() - root = self._read_xml() lun_type = root.findtext('LUN/LUNType') - if (self.device_type['type'] == 'Dorado5100' or not lun_type): - lun_type = 'Thick' - elif self.device_type['type'] == 'Dorado2100 G2': + if self.device_type['type'] == 'Dorado2100 G2': lun_type = 'Thin' + elif (self.device_type['type'] == 'Dorado5100' or not lun_type): + lun_type = 'Thick' poolinfo_dev = self._find_pool_info(lun_type) pools_conf = root.findall('LUN/StoragePool') total_free_capacity = 0