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 4280442fef1b5ebdb072f93bd155d4c5bd827628..59a664c5b8758328271c993b2b932199b60d628c 100644 (file)
--- 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 ac75a77db9beba8d2e2f450e4ec1e711a932a891..111e053e90bd4cfffa7665322b47096cbbc5ecc4 100644 (file)
--- 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 2dc770919aeac91f8802574e356eccb96afc8fc3..db6c547f0e99e8afb183cb5dc9db909e023de420 100644 (file)
--- 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 c898ab051c58257904ac58a7ebf86ab1cd8e5187..0f8a28b18d87f806e91c5bdaa28bf54db13858d0 100644 (file)
--- 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: