From b068ae93e26d21cb94504ac2f70231361acd38e6 Mon Sep 17 00:00:00 2001 From: Anthony Lee Date: Fri, 26 Jun 2015 10:38:33 -0700 Subject: [PATCH] Improve 3PAR driver VLUN creation and deletion This patch improves the creation and deletion logic for VLUNs for the 3PAR drivers. The changes will help to prevent VLUNs from being left behind after volume detaches. It also prevents extra VLUNs being created when initialize_connection is called multiple times for the same volume and host. Closes-Bug: #1469816 Change-Id: I497564cb4bd661252570eb2e69962f7d5f3c07da --- cinder/tests/unit/test_hp3par.py | 33 +++++++----- .../volume/drivers/san/hp/hp_3par_common.py | 54 +++++++++++++++---- cinder/volume/drivers/san/hp/hp_3par_fc.py | 36 +++++++++---- cinder/volume/drivers/san/hp/hp_3par_iscsi.py | 12 +++-- 4 files changed, 97 insertions(+), 38 deletions(-) diff --git a/cinder/tests/unit/test_hp3par.py b/cinder/tests/unit/test_hp3par.py index 1d331c584..6eb61195e 100644 --- a/cinder/tests/unit/test_hp3par.py +++ b/cinder/tests/unit/test_hp3par.py @@ -2096,6 +2096,7 @@ class HP3PARBaseDriver(object): self.VOLUME_3PAR_NAME, None, self.FAKE_HOST), + mock.call.getHostVLUNs(self.FAKE_HOST), mock.call.deleteHost(self.FAKE_HOST), mock.call.removeVolumeMetaData( self.VOLUME_3PAR_NAME, CHAP_USER_KEY), @@ -2997,6 +2998,8 @@ 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, @@ -3023,6 +3026,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.createVLUN( self.VOLUME_3PAR_NAME, auto=True, @@ -3069,6 +3073,8 @@ 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, @@ -3109,6 +3115,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.getPorts(), mock.call.createVLUN( self.VOLUME_3PAR_NAME, @@ -3154,6 +3161,8 @@ 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, @@ -3180,6 +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.createVLUN( self.VOLUME_3PAR_NAME, auto=True, @@ -3203,6 +3213,7 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase): effects = [ [{'active': True, 'volumeName': self.VOLUME_3PAR_NAME, 'lun': None, 'type': 0}], + hpexceptions.HTTPNotFound, hpexceptions.HTTPNotFound] mock_client.getHostVLUNs.side_effect = effects @@ -3220,6 +3231,7 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase): self.VOLUME_3PAR_NAME, None, self.FAKE_HOST), + mock.call.getHostVLUNs(self.FAKE_HOST), mock.call.deleteHost(self.FAKE_HOST), mock.call.getHostVLUNs(self.FAKE_HOST), mock.call.getPorts()] @@ -3282,6 +3294,7 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase): effects = [ [{'active': True, 'volumeName': self.VOLUME_3PAR_NAME, 'lun': None, 'type': 0}], + hpexceptions.HTTPNotFound, hpexceptions.HTTPNotFound] mock_client.queryHost.return_value = { @@ -3299,6 +3312,7 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase): self.VOLUME_3PAR_NAME, None, self.FAKE_HOST), + mock.call.getHostVLUNs(self.FAKE_HOST), mock.call.deleteHost(self.FAKE_HOST), mock.call.getHostVLUNs(self.FAKE_HOST), mock.call.getPorts()] @@ -3368,6 +3382,7 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase): self.VOLUME_3PAR_NAME, None, self.FAKE_HOST), + mock.call.getHostVLUNs(self.FAKE_HOST), mock.call.getHostVLUNs(self.FAKE_HOST)] with mock.patch.object(hpcommon.HP3PARCommon, @@ -3739,6 +3754,7 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase): '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}} location = ("%(volume_name)s,%(lun_id)s,%(host)s,%(nsp)s" % @@ -3761,13 +3777,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.createVLUN( - self.VOLUME_3PAR_NAME, - auto=True, - hostname='fakehost', - portPos={'node': 8, 'slot': 1, 'cardPort': 1}), - mock.call.getHostVLUNs(self.FAKE_HOST)] + mock.call.getVLUN(self.VOLUME_3PAR_NAME)] mock_client.assert_has_calls( self.standard_login + @@ -3795,6 +3805,7 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase): '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}} location = ("%(volume_name)s,%(lun_id)s,%(host)s,%(nsp)s" % @@ -3817,13 +3828,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.createVLUN( - self.VOLUME_3PAR_NAME, - auto=True, - hostname='fakehost', - portPos={'node': 8, 'slot': 1, 'cardPort': 1}), - mock.call.getHostVLUNs(self.FAKE_HOST)] + mock.call.getVLUN(self.VOLUME_3PAR_NAME)] 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 120e49665..ac75a77db 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_common.py +++ b/cinder/volume/drivers/san/hp/hp_3par_common.py @@ -178,10 +178,11 @@ class HP3PARCommon(object): 2.0.43 - Report the capability of supporting multiattach 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 """ - VERSION = "2.0.45" + VERSION = "2.0.46" stats = {} @@ -794,24 +795,57 @@ class HP3PARCommon(object): volume_name = self._get_3par_vol_name(volume['id']) vluns = self.client.getHostVLUNs(hostname) + # Find all the VLUNs associated with the volume. The VLUNs will then + # be split into groups based on the active status of the VLUN. If there + # are active VLUNs detected a delete will be attempted on them. If + # there are no active VLUNs but there are inactive VLUNs, then the + # inactive VLUNs will be deleted. The inactive VLUNs are the templates + # on the 3PAR backend. + active_volume_vluns = [] + inactive_volume_vluns = [] + volume_vluns = [] + for vlun in vluns: if volume_name in vlun['volumeName']: - break - else: - LOG.info(_LI("3PAR vlun for volume %(name)s not found on host " - "%(host)s"), {'name': volume_name, 'host': hostname}) + if vlun['active']: + active_volume_vluns.append(vlun) + else: + inactive_volume_vluns.append(vlun) + if active_volume_vluns: + volume_vluns = active_volume_vluns + elif inactive_volume_vluns: + volume_vluns = inactive_volume_vluns + + if not volume_vluns: + msg = ( + _LW("3PAR vlun for volume %(name)s not found on " + "host %(host)s"), {'name': volume_name, 'host': hostname}) + LOG.warning(msg) return # VLUN Type of MATCHED_SET 4 requires the port to be provided - if self.VLUN_TYPE_MATCHED_SET == vlun['type']: - self.client.deleteVLUN(volume_name, vlun['lun'], hostname, - vlun['portPos']) - else: - self.client.deleteVLUN(volume_name, vlun['lun'], hostname) + removed_luns = [] + for vlun in volume_vluns: + if self.VLUN_TYPE_MATCHED_SET == vlun['type']: + self.client.deleteVLUN(volume_name, vlun['lun'], hostname, + vlun['portPos']) + else: + # This is HOST_SEES or a type that is not MATCHED_SET. + # By deleting one VLUN, all the others should be deleted, too. + if vlun['lun'] not in removed_luns: + self.client.deleteVLUN(volume_name, vlun['lun'], hostname) + removed_luns.append(vlun['lun']) # Determine if there are other volumes attached to the host. # This will determine whether we should try removing host from host set # and deleting the host. + vluns = [] + try: + vluns = self.client.getHostVLUNs(hostname) + except hpexceptions.HTTPNotFound: + LOG.debug("All VLUNs removed from host %s", hostname) + pass + for vlun in vluns: if volume_name not in vlun['volumeName']: # Found another volume diff --git a/cinder/volume/drivers/san/hp/hp_3par_fc.py b/cinder/volume/drivers/san/hp/hp_3par_fc.py index 8740dda1c..2dc770919 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_fc.py +++ b/cinder/volume/drivers/san/hp/hp_3par_fc.py @@ -78,10 +78,11 @@ class HP3PARFCDriver(cinder.volume.driver.FibreChannelDriver): 2.0.14 - Removed usage of host name cache #1398914 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 """ - VERSION = "2.0.16" + VERSION = "2.0.17" def __init__(self, *args, **kwargs): super(HP3PARFCDriver, self).__init__(*args, **kwargs) @@ -228,17 +229,30 @@ class HP3PARFCDriver(cinder.volume.driver.FibreChannelDriver): target_wwns, init_targ_map, numPaths = \ self._build_initiator_target_map(common, connector) - # now that we have a host, create the VLUN - if self.lookup_service is not None and numPaths == 1: - nsp = None - active_fc_port_list = common.get_active_fc_target_ports() - for port in active_fc_port_list: - if port['portWWN'].lower() == target_wwns[0].lower(): - nsp = port['nsp'] - break - vlun = common.create_vlun(volume, host, nsp) + # 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 + + vlun = None + if not existing_vlun or host['name'] != existing_vlun['hostname']: + # now that we have a host, create the VLUN + if self.lookup_service is not None and numPaths == 1: + nsp = None + active_fc_port_list = common.get_active_fc_target_ports() + for port in active_fc_port_list: + if port['portWWN'].lower() == target_wwns[0].lower(): + nsp = port['nsp'] + break + vlun = common.create_vlun(volume, host, nsp) + else: + vlun = common.create_vlun(volume, host) else: - vlun = common.create_vlun(volume, host) + vlun = existing_vlun info = {'driver_volume_type': 'fibre_channel', 'data': {'target_lun': vlun['lun'], diff --git a/cinder/volume/drivers/san/hp/hp_3par_iscsi.py b/cinder/volume/drivers/san/hp/hp_3par_iscsi.py index cbf35f470..c898ab051 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_iscsi.py +++ b/cinder/volume/drivers/san/hp/hp_3par_iscsi.py @@ -87,10 +87,11 @@ class HP3PARISCSIDriver(cinder.volume.driver.ISCSIDriver): 2.0.15 - Added support for updated detach_volume attachment. 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 """ - VERSION = "2.0.17" + VERSION = "2.0.18" def __init__(self, *args, **kwargs): super(HP3PARISCSIDriver, self).__init__(*args, **kwargs) @@ -290,6 +291,7 @@ 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) @@ -309,8 +311,12 @@ class HP3PARISCSIDriver(cinder.volume.driver.ISCSIDriver): common, host['name']) - # now that we have a host, create the VLUN - vlun = common.create_vlun(volume, host, least_used_nsp) + vlun = None + if not existing_vlun or host['name'] != existing_vlun['hostname']: + # now that we have a host, create the VLUN + vlun = common.create_vlun(volume, host, least_used_nsp) + else: + vlun = existing_vlun if least_used_nsp is None: LOG.warning(_LW("Least busy iSCSI port not found, " -- 2.45.2