From: Bob-OpenStack <295988511@qq.com> Date: Mon, 19 Jan 2015 01:05:14 +0000 (-0800) Subject: Huawei driver check before associating LUN to a LUN group X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=9b11c4e0329db814eaf3ac18246bfe502f13504d;p=openstack-build%2Fcinder-build.git Huawei driver check before associating LUN to a LUN group This commit fixes the following problems: * Change in huawei_t.py removes the duplicated wwns returned from the array. * Change in rest_common.py adds log print for response data from the array. * Change in rest_common.py does a check before associating a LUN to a LUN group. * Change in rest_common.py fixes the REST request. * Change in ssh_common.py checks whether the hostlunid have already been assigned. Closes-Bug: #1411904 Change-Id: I862ed5d71780fb7b8cbc5ed257072ea1113fead7 --- diff --git a/cinder/tests/test_huawei_18000.py b/cinder/tests/test_huawei_18000.py index 370a4c826..512afd75e 100644 --- a/cinder/tests/test_huawei_18000.py +++ b/cinder/tests/test_huawei_18000.py @@ -341,6 +341,7 @@ class Fake18000Common(rest_common.RestCommon): rest_common.RestCommon.__init__(self, configuration) self.test_normal = True self.other_flag = True + self.associate_flag = True self.connect_flag = False self.delete_flag = False self.terminateFlag = False @@ -442,7 +443,9 @@ class Fake18000Common(rest_common.RestCommon): data = data_lun(url, method) if url == "lungroup/associate?ID=11"\ - "&ASSOCIATEOBJTYPE=11&ASSOCIATEOBJID=11": + "&ASSOCIATEOBJTYPE=11&ASSOCIATEOBJID=11"\ + or url == "lungroup/associate?ID=12"\ + "&ASSOCIATEOBJTYPE=11&ASSOCIATEOBJID=12": data = '{"error":{"code":0}}' self.terminateFlag = True @@ -494,9 +497,18 @@ class Fake18000Common(rest_common.RestCommon): if url == "mappingview" or url == "mappingview?range=[0-65535]": data = find_data_mappingview(method, self.other_flag) - if url == ("lun/associate?ID=1&TYPE=11&" - "ASSOCIATEOBJTYPE=21&ASSOCIATEOBJID=0"): - data = '{"error":{"code":0}}' + if (url == ("lun/associate?ID=1&TYPE=11&" + "ASSOCIATEOBJTYPE=21&ASSOCIATEOBJID=0") + or url == ("lun/associate?TYPE=11&ASSOCIATEOBJTYPE=256" + "&ASSOCIATEOBJID=11") + or (url == ("lun/associate?TYPE=11&ASSOCIATEOBJTYPE=256" + "&ASSOCIATEOBJID=12") + and not self.associate_flag)): + data = '{"error":{"code":0},"data":[{"ID":"11"}]}' + if ((url == ("lun/associate?TYPE=11&ASSOCIATEOBJTYPE=256" + "&ASSOCIATEOBJID=12")) + and self.associate_flag): + data = '{"error":{"code":0},"data":[{"ID":"12"}]}' if url == "fc_initiator?ISFREE=true&range=[0-1000]": data = self.fc_initiator_data() @@ -524,11 +536,10 @@ class Fake18000Common(rest_common.RestCommon): else: data = '{"error":{"code":31755596}}' - if url == "lun/11": - if method == "GET": - data = """{"error":{"code":0},"data":{"ID":"11", - "IOCLASSID":"11", - "NAME":"5mFHcBv4RkCcD+JyrWc0SA"}}""" + if (url == "lun/11") and (method == "GET"): + data = """{"error":{"code":0},"data":{"ID":"11", + "IOCLASSID":"11", + "NAME":"5mFHcBv4RkCcD+JyrWc0SA"}}""" res_json = json.loads(data) return res_json @@ -690,6 +701,23 @@ class Huawei18000ISCSIDriverTestCase(test.TestCase): result = self.driver.common._get_wait_interval('LUNReadyWaitInterval') self.assertEqual('2', result) + def test_lun_is_associated_to_lungroup(self): + self.driver.common.login() + self.driver.common._associate_lun_to_lungroup('11', '11') + result = self.driver.common._is_lun_associated_to_lungroup('11', '11') + self.assertTrue(result) + + def test_lun_is_not_associated_to_lun_group(self): + self.driver.common.login() + self.driver.common._associate_lun_to_lungroup('12', '12') + self.driver.common.associate_flag = True + result = self.driver.common._is_lun_associated_to_lungroup('12', '12') + self.assertTrue(result) + self.driver.common._remove_lun_from_lungroup('12', '12') + self.driver.common.associate_flag = False + result = self.driver.common._is_lun_associated_to_lungroup('12', '12') + self.assertFalse(result) + def create_fake_conf_file(self): """Create a fake Config file @@ -898,6 +926,23 @@ class Huawei18000FCDriverTestCase(test.TestCase): result = self.driver.common._get_wait_interval('LUNReadyWaitInterval') self.assertEqual('2', result) + def test_lun_is_associated_to_lungroup(self): + self.driver.common.login() + self.driver.common._associate_lun_to_lungroup('11', '11') + result = self.driver.common._is_lun_associated_to_lungroup('11', '11') + self.assertTrue(result) + + def test_lun_is_not_associated_to_lun_group(self): + self.driver.common.login() + self.driver.common._associate_lun_to_lungroup('12', '12') + self.driver.common.associate_flag = True + result = self.driver.common._is_lun_associated_to_lungroup('12', '12') + self.assertTrue(result) + self.driver.common._remove_lun_from_lungroup('12', '12') + self.driver.common.associate_flag = False + result = self.driver.common._is_lun_associated_to_lungroup('12', '12') + self.assertFalse(result) + def create_fake_conf_file(self): """Create a fake Config file diff --git a/cinder/volume/drivers/huawei/huawei_t.py b/cinder/volume/drivers/huawei/huawei_t.py index 356dcf298..afb13a97d 100644 --- a/cinder/volume/drivers/huawei/huawei_t.py +++ b/cinder/volume/drivers/huawei/huawei_t.py @@ -21,6 +21,7 @@ import re import time from oslo_log import log as logging +from oslo_utils import excutils from cinder import exception from cinder.i18n import _, _LE, _LW @@ -108,7 +109,7 @@ class HuaweiTISCSIDriver(driver.ISCSIDriver): 'ini': connector['initiator']}) self.common._update_login_info() - (iscsi_iqn, target_ip, port_ctr) =\ + (iscsi_iqn, target_ip, port_ctr) = \ self._get_iscsi_params(connector['initiator']) # First, add a host if not added before. @@ -120,7 +121,12 @@ class HuaweiTISCSIDriver(driver.ISCSIDriver): # Finally, map the volume to the host. volume_id = volume['provider_location'] - hostlun_id = self.common.map_volume(host_id, volume_id) + try: + hostlun_id = self.common.map_volume(host_id, volume_id) + except Exception: + with excutils.save_and_reraise_exception(): + # Remove the iSCSI port from the host if the map failed. + self._remove_iscsi_port(host_id, connector) # Change LUN ctr for better performance, just for single path. lun_details = self.common.get_lun_details(volume_id) @@ -467,7 +473,12 @@ class HuaweiTFCDriver(driver.FibreChannelDriver): # Finally, map the volume to the host. volume_id = volume['provider_location'] - hostlun_id = self.common.map_volume(host_id, volume_id) + try: + hostlun_id = self.common.map_volume(host_id, volume_id) + except Exception: + with excutils.save_and_reraise_exception(): + # Remove the FC port from the host if the map failed. + self._remove_fc_ports(host_id, connector) # Change LUN ctr for better performance, just for single path. if len(tgt_wwns) == 1: @@ -502,7 +513,7 @@ class HuaweiTFCDriver(driver.FibreChannelDriver): if (tmp_line[1] == 'FC') and (tmp_line[4] == 'Connected'): wwns.append(tmp_line[0]) - return wwns + return list(set(wwns)) def _add_fc_port_to_host(self, hostid, wwn, multipathtype=0): """Add a FC port to host.""" diff --git a/cinder/volume/drivers/huawei/rest_common.py b/cinder/volume/drivers/huawei/rest_common.py index 672cf84eb..9653fe1d9 100644 --- a/cinder/volume/drivers/huawei/rest_common.py +++ b/cinder/volume/drivers/huawei/rest_common.py @@ -78,20 +78,22 @@ class RestCommon(): if "xx/sessions" not in url: LOG.info(_LI('\n\n\n\nRequest URL: %(url)s\n\n' 'Call Method: %(method)s\n\n' - 'Request Data: %(data)s\n\n'), {'url': url, - 'method': method, - 'data': data}) + 'Request Data: %(data)s\n\n' + 'Response Data:%(res)s\n\n'), {'url': url, + 'method': method, + 'data': data, + 'res': res}) except Exception as err: LOG.error(_LE('\nBad response from server: %s.') % err) - raise err + raise try: res_json = json.loads(res) except Exception as err: err_msg = (_LE('JSON transfer error: %s.') % err) LOG.error(err_msg) - raise err + raise return res_json @@ -277,7 +279,7 @@ class RestCommon(): root = tree.getroot() except Exception as err: LOG.error(_LE('_read_xml: %s') % err) - raise err + raise return root def _encode_name(self, name): @@ -515,9 +517,9 @@ class RestCommon(): except Exception as ex: res = False LOG.debug('_wait_for_condition: %(func_name)s ' - 'failed for %(exception)s.' - % {'func_name': func.__name__, - 'exception': ex.message}) + 'failed for %(exception)s.', + {'func_name': func.__name__, + 'exception': ex}) if res: raise loopingcall.LoopingCallDone() @@ -645,9 +647,9 @@ class RestCommon(): if hostgroup_id is None: hostgroup_id = self._create_hostgroup(host_group_name) - isAssociate = self._is_host_associate_to_hostgroup(hostgroup_id, - host_id) - if isAssociate is False: + is_associated = self._is_host_associate_to_hostgroup(hostgroup_id, + host_id) + if is_associated is False: self._associate_host_to_hostgroup(hostgroup_id, host_id) return hostgroup_id @@ -672,7 +674,10 @@ class RestCommon(): # Create lungroup and add LUN into to lungroup. if lungroup_id is None: lungroup_id = self._create_lungroup(lungroup_name) - self._associate_lun_to_lungroup(lungroup_id, lun_id) + is_associated = self._is_lun_associated_to_lungroup(lungroup_id, + lun_id) + if not is_associated: + self._associate_lun_to_lungroup(lungroup_id, lun_id) if view_id is None: view_id = self._add_mapping_view(mapping_view_name) @@ -923,7 +928,7 @@ class RestCommon(): except Exception as err: msg = (_LE("JSON transfer data error. %s") % err) LOG.error(msg) - raise err + raise return host_lun_id def _find_host(self, hostname): @@ -971,6 +976,22 @@ class RestCommon(): return False + def _is_lun_associated_to_lungroup(self, lungroup_id, lun_id): + """Check whether the lun is associated to the lungroup.""" + url_subfix = ("/lun/associate?TYPE=11&" + "ASSOCIATEOBJTYPE=256&ASSOCIATEOBJID=%s" % lungroup_id) + + url = self.url + url_subfix + result = self.call(url, None, "GET") + self._assert_rest_result(result, 'Check lungroup associate error.') + + if "data" in result: + for item in result['data']: + if lun_id == item['ID']: + return True + + return False + def _associate_host_to_hostgroup(self, hostgroup_id, host_id): url = self.url + "/hostgroup/associate" data = json.dumps({"TYPE": "14", @@ -1003,9 +1024,8 @@ class RestCommon(): def _initiator_is_added_to_array(self, ininame): """Check whether the initiator is already added on the array.""" - url = self.url + "/iscsi_initiator" - data = json.dumps({"TYPE": "222", "ID": ininame}) - result = self.call(url, data, "GET") + url = self.url + "/iscsi_initiator?range=[0-65535]" + result = self.call(url, None, "GET") self._assert_rest_result(result, 'Check initiator added to array error.') @@ -1017,9 +1037,8 @@ class RestCommon(): def _is_initiator_associated_to_host(self, ininame): """Check whether the initiator is associated to the host.""" - url = self.url + "/iscsi_initiator" - data = json.dumps({"TYPE": "222", "ID": ininame}) - result = self.call(url, data, "GET") + url = self.url + "/iscsi_initiator?range=[0-65535]" + result = self.call(url, None, "GET") self._assert_rest_result(result, 'Check initiator associated to host error.') diff --git a/cinder/volume/drivers/huawei/ssh_common.py b/cinder/volume/drivers/huawei/ssh_common.py index 1aa0ef2ad..6688f3734 100644 --- a/cinder/volume/drivers/huawei/ssh_common.py +++ b/cinder/volume/drivers/huawei/ssh_common.py @@ -43,6 +43,7 @@ LOG = logging.getLogger(__name__) HOST_GROUP_NAME = 'HostGroup_OpenStack' HOST_NAME_PREFIX = 'Host_' VOL_AND_SNAP_NAME_PREFIX = 'OpenStack_' +HOST_LUN_ERR_MSG = 'host LUN is mapped or does not exist' def ssh_read(user, channel, cmd, timeout): @@ -122,14 +123,14 @@ class TseriesCommon(): LOG.error(err_msg) raise exception.InvalidInput(reason=err_msg) - # make sure storage pool is set + # Make sure storage pool is set. if not huawei_utils.is_xml_item_exist(root, 'LUN/StoragePool', 'Name'): err_msg = _('_check_conf_file: Config file invalid. ' 'StoragePool must be set.') LOG.error(err_msg) raise exception.InvalidInput(reason=err_msg) - # If setting os type, make sure it valid + # If setting os type, make sure it valid. if huawei_utils.is_xml_item_exist(root, 'Host', 'OSType'): os_list = huawei_utils.os_type.keys() if not huawei_utils.is_xml_item_valid(root, 'Host', os_list, @@ -147,10 +148,10 @@ class TseriesCommon(): filename = self.configuration.cinder_huawei_conf_file tree = ET.parse(filename) root = tree.getroot() - logininfo['ControllerIP0'] =\ - root.findtext('Storage/ControllerIP0').strip() - logininfo['ControllerIP1'] =\ - root.findtext('Storage/ControllerIP1').strip() + logininfo['ControllerIP0'] = ( + root.findtext('Storage/ControllerIP0')).strip() + logininfo['ControllerIP1'] = ( + root.findtext('Storage/ControllerIP1')).strip() need_encode = False for key in ['UserName', 'UserPassword']: @@ -958,6 +959,17 @@ class TseriesCommon(): 'lunid': volume_id, 'hostlunid': new_hostlun_id}) out = self._execute_cli(cli_cmd) + # Check whether the hostlunid has already been assigned. + condition = re.search(HOST_LUN_ERR_MSG, out) + while condition: + new_hostlun_id = new_hostlun_id + 1 + cli_cmd = ('addhostmap -host %(host_id)s -devlun %(lunid)s ' + '-hostlun %(hostlunid)s' + % {'host_id': host_id, + 'lunid': volume_id, + 'hostlunid': new_hostlun_id}) + out = self._execute_cli(cli_cmd) + condition = re.search(HOST_LUN_ERR_MSG, out) msg = ('Failed to map LUN %s to host %s. host LUN ID: %s' % (volume_id, host_id, new_hostlun_id))