From: Anthony Lee Date: Thu, 16 Jul 2015 02:25:58 +0000 (-0700) Subject: Fix 3PAR driver handling of existing VLUNs X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=a07502f697fc7e45871d0099889841d03bfdf773;p=openstack-build%2Fcinder-build.git Fix 3PAR driver handling of existing VLUNs Changes the 3PAR FC and iSCSI drivers to check for existing VLUNs by taking into consideration both the volume and the host when querying the backend. Also pulled out some common code from the FC and iSCSI driver to reduce duplicate code relating to the finding of existing VLUNs. Closes-Bug: #1475064 Change-Id: Id8fbdb5ae79274fb3d074f674df39f56db06f34d --- diff --git a/cinder/tests/unit/test_hp3par.py b/cinder/tests/unit/test_hp3par.py index 4280442fe..59a664c5b 100644 --- a/cinder/tests/unit/test_hp3par.py +++ b/cinder/tests/unit/test_hp3par.py @@ -2995,12 +2995,13 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase): 'name': self.FAKE_HOST }] } - mock_client.getVLUN.side_effect = [ - hpexceptions.HTTPNotFound('fake')] - mock_client.getHostVLUNs.return_value = [ - {'active': True, - 'volumeName': self.VOLUME_3PAR_NAME, - 'lun': 90, 'type': 0}] + + mock_client.getHostVLUNs.side_effect = [ + hpexceptions.HTTPNotFound('fake'), + [{'active': True, + 'volumeName': self.VOLUME_3PAR_NAME, + 'lun': 90, 'type': 0}]] + location = ("%(volume_name)s,%(lun_id)s,%(host)s,%(nsp)s" % {'volume_name': self.VOLUME_3PAR_NAME, 'lun_id': 90, @@ -3023,7 +3024,7 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase): '123456789054321']), mock.call.getHost(self.FAKE_HOST), mock.call.getPorts(), - mock.call.getVLUN(self.VOLUME_3PAR_NAME), + mock.call.getHostVLUNs(self.FAKE_HOST), mock.call.createVLUN( self.VOLUME_3PAR_NAME, auto=True, @@ -3070,12 +3071,13 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase): 'name': self.FAKE_HOST }] } - mock_client.getVLUN.side_effect = [ - hpexceptions.HTTPNotFound('fake')] - mock_client.getHostVLUNs.return_value = [ - {'active': True, - 'volumeName': self.VOLUME_3PAR_NAME, - 'lun': 90, 'type': 0}] + + mock_client.getHostVLUNs.side_effect = [ + hpexceptions.HTTPNotFound('fake'), + [{'active': True, + 'volumeName': self.VOLUME_3PAR_NAME, + 'lun': 90, 'type': 0}]] + location = ("%(volume_name)s,%(lun_id)s,%(host)s,%(nsp)s" % {'volume_name': self.VOLUME_3PAR_NAME, 'lun_id': 90, @@ -3112,7 +3114,7 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase): mock.ANY, mock.call.getHost(self.FAKE_HOST), mock.call.getPorts(), - mock.call.getVLUN(self.VOLUME_3PAR_NAME), + mock.call.getHostVLUNs(self.FAKE_HOST), mock.call.getPorts(), mock.call.createVLUN( self.VOLUME_3PAR_NAME, @@ -3158,12 +3160,13 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase): 'name': self.FAKE_HOST }] } - mock_client.getVLUN.side_effect = [ - hpexceptions.HTTPNotFound('fake')] - mock_client.getHostVLUNs.return_value = [ - {'active': True, - 'volumeName': self.VOLUME_3PAR_NAME, - 'lun': 90, 'type': 0}] + + mock_client.getHostVLUNs.side_effect = [ + hpexceptions.HTTPNotFound('fake'), + [{'active': True, + 'volumeName': self.VOLUME_3PAR_NAME, + 'lun': 90, 'type': 0}]] + location = ("%(volume_name)s,%(lun_id)s,%(host)s,%(nsp)s" % {'volume_name': self.VOLUME_3PAR_NAME, 'lun_id': 90, @@ -3186,7 +3189,7 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase): '123456789054321']), mock.call.getHost(self.FAKE_HOST), mock.call.getPorts(), - mock.call.getVLUN(self.VOLUME_3PAR_NAME), + mock.call.getHostVLUNs(self.FAKE_HOST), mock.call.createVLUN( self.VOLUME_3PAR_NAME, auto=True, @@ -3746,14 +3749,16 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase): 'name': self.FAKE_HOST }] } - mock_client.getHostVLUNs.return_value = [ - {'active': True, - 'volumeName': self.VOLUME_3PAR_NAME, - 'lun': self.TARGET_LUN, 'type': 0}] - mock_client.getVLUN.return_value = { - 'hostname': self.FAKE_HOST, - 'lun': self.TARGET_LUN, - 'portPos': {'node': 8, 'slot': 1, 'cardPort': 1}} + + mock_client.getHostVLUNs.side_effect = [ + [{'hostname': self.FAKE_HOST, + 'volumeName': self.VOLUME_3PAR_NAME, + 'lun': self.TARGET_LUN, + 'portPos': {'node': 8, 'slot': 1, 'cardPort': 1}}], + [{'active': True, + 'volumeName': self.VOLUME_3PAR_NAME, + 'lun': self.TARGET_LUN, 'type': 0}]] + location = ("%(volume_name)s,%(lun_id)s,%(host)s,%(nsp)s" % {'volume_name': self.VOLUME_3PAR_NAME, 'lun_id': self.TARGET_LUN, @@ -3774,7 +3779,7 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase): mock.call.getHost(self.FAKE_HOST), mock.call.queryHost(iqns=['iqn.1993-08.org.debian:01:222']), mock.call.getHost(self.FAKE_HOST), - mock.call.getVLUN(self.VOLUME_3PAR_NAME)] + mock.call.getHostVLUNs(self.FAKE_HOST)] mock_client.assert_has_calls( self.standard_login + @@ -3797,14 +3802,16 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase): 'name': self.FAKE_HOST }] } - mock_client.getHostVLUNs.return_value = [ - {'active': True, - 'volumeName': self.VOLUME_3PAR_NAME, - 'lun': self.TARGET_LUN, 'type': 0}] - mock_client.getVLUN.return_value = { - 'hostname': self.FAKE_HOST, - 'lun': self.TARGET_LUN, - 'portPos': {'node': 8, 'slot': 1, 'cardPort': 1}} + + mock_client.getHostVLUNs.side_effect = [ + [{'hostname': self.FAKE_HOST, + 'volumeName': self.VOLUME_3PAR_NAME, + 'lun': self.TARGET_LUN, + 'portPos': {'node': 8, 'slot': 1, 'cardPort': 1}}], + [{'active': True, + 'volumeName': self.VOLUME_3PAR_NAME, + 'lun': self.TARGET_LUN, 'type': 0}]] + location = ("%(volume_name)s,%(lun_id)s,%(host)s,%(nsp)s" % {'volume_name': self.VOLUME_3PAR_NAME, 'lun_id': self.TARGET_LUN, @@ -3825,7 +3832,7 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase): mock.call.getHost(self.FAKE_HOST), mock.call.queryHost(iqns=['iqn.1993-08.org.debian:01:222']), mock.call.getHost(self.FAKE_HOST), - mock.call.getVLUN(self.VOLUME_3PAR_NAME)] + mock.call.getHostVLUNs(self.FAKE_HOST)] mock_client.assert_has_calls( self.standard_login + diff --git a/cinder/volume/drivers/san/hp/hp_3par_common.py b/cinder/volume/drivers/san/hp/hp_3par_common.py index ac75a77db..111e053e9 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_common.py +++ b/cinder/volume/drivers/san/hp/hp_3par_common.py @@ -179,10 +179,11 @@ class HP3PARCommon(object): 2.0.44 - Update help strings to reduce the 3PAR user role requirements 2.0.45 - Python 3 fixes 2.0.46 - Improved VLUN creation and deletion logic. #1469816 + 2.0.47 - Changed initialize_connection to use getHostVLUNs. #1475064 """ - VERSION = "2.0.46" + VERSION = "2.0.47" stats = {} @@ -2089,6 +2090,30 @@ class HP3PARCommon(object): return self._retype_from_old_to_new(volume, new_type, old_volume_settings, host) + def find_existing_vlun(self, volume, host): + """Finds an existing VLUN for a volume on a host. + + Returns an existing VLUN's information. If no existing VLUN is found, + None is returned. + + :param volume: A dictionary describing a volume. + :param host: A dictionary describing a host. + """ + existing_vlun = None + try: + vol_name = self._get_3par_vol_name(volume['id']) + host_vluns = self.client.getHostVLUNs(host['name']) + + # The first existing VLUN found will be returned. + for vlun in host_vluns: + if vlun['volumeName'] == vol_name: + existing_vlun = vlun + break + except hpexceptions.HTTPNotFound: + # ignore, no existing VLUNs were found + pass + return existing_vlun + class TaskWaiter(object): """TaskWaiter waits for task to be not active and returns status.""" diff --git a/cinder/volume/drivers/san/hp/hp_3par_fc.py b/cinder/volume/drivers/san/hp/hp_3par_fc.py index 2dc770919..db6c547f0 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_fc.py +++ b/cinder/volume/drivers/san/hp/hp_3par_fc.py @@ -79,10 +79,11 @@ class HP3PARFCDriver(cinder.volume.driver.FibreChannelDriver): 2.0.15 - Added support for updated detach_volume attachment. 2.0.16 - Added encrypted property to initialize_connection #1439917 2.0.17 - Improved VLUN creation and deletion logic. #1469816 + 2.0.18 - Changed initialize_connection to use getHostVLUNs. #1475064 """ - VERSION = "2.0.17" + VERSION = "2.0.18" def __init__(self, *args, **kwargs): super(HP3PARFCDriver, self).__init__(*args, **kwargs) @@ -230,16 +231,10 @@ class HP3PARFCDriver(cinder.volume.driver.FibreChannelDriver): self._build_initiator_target_map(common, connector) # check if a VLUN already exists for this host - existing_vlun = None - try: - vol_name = common._get_3par_vol_name(volume['id']) - existing_vlun = common.client.getVLUN(vol_name) - except hpexceptions.HTTPNotFound: - # ignore, vlun will be created later - pass + existing_vlun = common.find_existing_vlun(volume, host) vlun = None - if not existing_vlun or host['name'] != existing_vlun['hostname']: + if existing_vlun is None: # now that we have a host, create the VLUN if self.lookup_service is not None and numPaths == 1: nsp = None diff --git a/cinder/volume/drivers/san/hp/hp_3par_iscsi.py b/cinder/volume/drivers/san/hp/hp_3par_iscsi.py index c898ab051..0f8a28b18 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_iscsi.py +++ b/cinder/volume/drivers/san/hp/hp_3par_iscsi.py @@ -88,10 +88,11 @@ class HP3PARISCSIDriver(cinder.volume.driver.ISCSIDriver): 2.0.16 - Added encrypted property to initialize_connection #1439917 2.0.17 - Python 3 fixes 2.0.18 - Improved VLUN creation and deletion logic. #1469816 + 2.0.19 - Changed initialize_connection to use getHostVLUNs. #1475064 """ - VERSION = "2.0.18" + VERSION = "2.0.19" def __init__(self, *args, **kwargs): super(HP3PARISCSIDriver, self).__init__(*args, **kwargs) @@ -291,20 +292,17 @@ class HP3PARISCSIDriver(cinder.volume.driver.ISCSIDriver): connector) least_used_nsp = None - existing_vlun = None - try: - vol_name = common._get_3par_vol_name(volume['id']) - existing_vlun = common.client.getVLUN(vol_name) + # check if a VLUN already exists for this host + existing_vlun = common.find_existing_vlun(volume, host) + + if existing_vlun: # We override the nsp here on purpose to force the # volume to be exported out the same IP as it already is. # This happens during nova live-migration, we want to # disable the picking of a different IP that we export # the volume to, or nova complains. least_used_nsp = common.build_nsp(existing_vlun['portPos']) - except hpexceptions.HTTPNotFound: - # ignore this error, as we will create the vlun later - pass if not least_used_nsp: least_used_nsp = self._get_least_used_nsp_for_host( @@ -312,7 +310,7 @@ class HP3PARISCSIDriver(cinder.volume.driver.ISCSIDriver): host['name']) vlun = None - if not existing_vlun or host['name'] != existing_vlun['hostname']: + if existing_vlun is None: # now that we have a host, create the VLUN vlun = common.create_vlun(volume, host, least_used_nsp) else: