From 4296ea130e1055d608b11b44cbff21358804ae7d Mon Sep 17 00:00:00 2001
From: Helen Walsh <helen.walsh@emc.com>
Date: Thu, 12 Mar 2020 15:30:20 +0000
Subject: [PATCH] PowerMax Driver - Legacy volume not found

The equals check is too stringent for legacy pre Pike volumes
after an upgrade.  Pre PIke is SMI-S, Pike and later is REST
and identifier name is persisted differently.

Change-Id: I963ad219c2e3b1c9c5edaa8f7e0d3dea911ecd8e
Closes-Bug: #1867163
---
 .../dell_emc/powermax/powermax_data.py        |  9 +++++++++
 .../dell_emc/powermax/test_powermax_rest.py   | 20 +++++++++++++++++++
 .../volume/drivers/dell_emc/powermax/rest.py  |  6 +++++-
 .../bug-fix-1867163-27afa39ac77b9e15.yaml     |  5 +++++
 4 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/bug-fix-1867163-27afa39ac77b9e15.yaml

diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py
index 4827434d06a..29d7ddd5b2a 100644
--- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py
+++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py
@@ -648,6 +648,15 @@ class PowerMaxData(object):
          'storageGroupId': [
              rdf_managed_async_grp, storagegroup_name_f + '-RA']})
 
+    volume_details_legacy = {'cap_gb': 2,
+                             'num_of_storage_groups': 1,
+                             'volumeId': device_id,
+                             'volume_identifier': test_volume.id,
+                             'wwn': volume_wwn,
+                             'snapvx_target': 'false',
+                             'snapvx_source': 'false',
+                             'storageGroupId': []}
+
     volume_list = [
         {'id': '6b70de13-98c5-46b2-8f24-e4e96a8988fa',
          'count': 2,
diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py
index 7f5cb9beef8..be092e8b96c 100644
--- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py
+++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py
@@ -671,6 +671,26 @@ class PowerMaxRestTest(test.TestCase):
             name_id=self.data.test_clone_volume._name_id)
         self.assertEqual(self.data.device_id, found_dev_id)
 
+    def test_check_volume_device_id_legacy_case(self):
+        element_name = self.utils.get_volume_element_name(
+            self.data.test_volume.id)
+        with mock.patch.object(self.rest, 'get_volume',
+                               return_value=self.data.volume_details_legacy):
+            found_dev_id = self.rest.check_volume_device_id(
+                self.data.array, self.data.device_id, element_name)
+        self.assertEqual(self.data.device_id, found_dev_id)
+
+    def test_check_volume_device_id_legacy_case_no_match(self):
+        element_name = self.utils.get_volume_element_name(
+            self.data.test_volume.id)
+        volume_details_no_match = deepcopy(self.data.volume_details_legacy)
+        volume_details_no_match['volume_identifier'] = 'no_match'
+        with mock.patch.object(self.rest, 'get_volume',
+                               return_value=volume_details_no_match):
+            found_dev_id = self.rest.check_volume_device_id(
+                self.data.array, self.data.device_id, element_name)
+        self.assertIsNone(found_dev_id)
+
     def test_find_mv_connections_for_vol(self):
         device_id = self.data.device_id
         ref_lun_id = int(
diff --git a/cinder/volume/drivers/dell_emc/powermax/rest.py b/cinder/volume/drivers/dell_emc/powermax/rest.py
index d953e03528e..117c2b57421 100644
--- a/cinder/volume/drivers/dell_emc/powermax/rest.py
+++ b/cinder/volume/drivers/dell_emc/powermax/rest.py
@@ -1018,8 +1018,12 @@ class PowerMaxRest(object):
                       'Device id = %(di)s',
                       {'en': element_name, 'vi': vol_identifier,
                        'di': device_id})
-            if vol_identifier == element_name:
+            if vol_identifier in element_name:
                 found_device_id = device_id
+                if vol_identifier != element_name:
+                    LOG.debug("Device %(di)s is a legacy volume created using "
+                              "SMI-S.",
+                              {'di': device_id})
             elif name_id:
                 # This may be host-assisted migration case
                 element_name = self.utils.get_volume_element_name(name_id)
diff --git a/releasenotes/notes/bug-fix-1867163-27afa39ac77b9e15.yaml b/releasenotes/notes/bug-fix-1867163-27afa39ac77b9e15.yaml
new file mode 100644
index 00000000000..f10eb92d9ac
--- /dev/null
+++ b/releasenotes/notes/bug-fix-1867163-27afa39ac77b9e15.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    PowerMax Driver - Issue with upgrades from pre Pike to Pike and later.
+    The device is not found when trying to snapshot a legacy volume.