From 8c38aa5ddf9efe82cd44c00b666748c69ba60f68 Mon Sep 17 00:00:00 2001
From: Shay Halsband <shay.halsband@emc.com>
Date: Mon, 22 Jun 2015 09:33:11 +0300
Subject: [PATCH] XtremIO driver fix array snapshot problem

Use create snapshot v2, to resolve a potential bug
when 2 such requests are processed in the same second.

Change-Id: If2801cb737e8a3d1847c68c9e19265c2123e6687
Closes-Bug: #1467386
---
 cinder/tests/unit/test_emc_xtremio.py | 24 +++++++++++++
 cinder/volume/drivers/emc/xtremio.py  | 50 ++++++++++++++++++++-------
 2 files changed, 62 insertions(+), 12 deletions(-)

diff --git a/cinder/tests/unit/test_emc_xtremio.py b/cinder/tests/unit/test_emc_xtremio.py
index c0e4b50a0..97a2a787c 100644
--- a/cinder/tests/unit/test_emc_xtremio.py
+++ b/cinder/tests/unit/test_emc_xtremio.py
@@ -173,6 +173,22 @@ def xms_bad_request(object_type='volumes', request_typ='GET', data=None,
         raise exception.VolumeBackendAPIException('Failed to create ig')
 
 
+def xms_failed_rename_snapshot_request(object_type='volumes',
+                                       request_typ='GET', data=None,
+                                       name=None, idx=None):
+    if request_typ == 'POST':
+        xms_data['volumes'][27] = {}
+        return {
+            "links": [
+                {
+                    "href": "https://host/api/json/v2/types/snapshots/27",
+                    "rel": "self"}]}
+    elif request_typ == 'PUT':
+        raise exception.VolumeBackendAPIException(msg='Failed to delete')
+    elif request_typ == 'DELETE':
+        del xms_data['volumes'][27]
+
+
 class D(dict):
     def update(self, *args, **kwargs):
         self.__dict__.update(*args, **kwargs)
@@ -267,9 +283,17 @@ class EMCXIODriverISCSITestCase(test.TestCase):
         clean_xms_data()
         self.driver.create_volume(self.data.test_volume)
         self.driver.create_snapshot(self.data.test_snapshot)
+        self.assertEqual(self.data.test_snapshot['id'],
+                         xms_data['volumes'][3]['name'])
         self.driver.delete_snapshot(self.data.test_snapshot)
         self.driver.delete_volume(self.data.test_volume)
 
+    def test_failed_rename_snapshot(self, req):
+        req.side_effect = xms_failed_rename_snapshot_request
+        self.driver.create_snapshot(self.data.test_snapshot)
+        self.assertIn(27, xms_data['volumes'])
+        clean_xms_data()
+
     def test_volume_from_snapshot(self, req):
         req.side_effect = xms_request
         clean_xms_data()
diff --git a/cinder/volume/drivers/emc/xtremio.py b/cinder/volume/drivers/emc/xtremio.py
index d4ec195e5..38efc48cf 100644
--- a/cinder/volume/drivers/emc/xtremio.py
+++ b/cinder/volume/drivers/emc/xtremio.py
@@ -152,6 +152,17 @@ class XtremIOClient(object):
     def get_cluster(self):
         return self.req('clusters', idx=1)['content']
 
+    def create_snapshot(self, src, dest, ro=False):
+        """Create a snapshot of a volume on the array.
+
+        XtreamIO array snapshots are also volumes.
+
+        :src: name of the source volume to be cloned
+        :dest: name for the new snapshot
+        :ro: new snapshot type ro/regular. only applicable to Client4
+        """
+        raise NotImplementedError()
+
 
 class XtremIOClient3(XtremIOClient):
     def __init__(self, configuration, cluster_id):
@@ -195,6 +206,11 @@ class XtremIOClient3(XtremIOClient):
 
         return self._portals
 
+    def create_snapshot(self, src, dest, ro=False):
+        data = {'snap-vol-name': dest, 'ancestor-vol-id': src}
+
+        self.req('snapshots', 'POST', data)
+
 
 class XtremIOClient4(XtremIOClient):
     def __init__(self, configuration, cluster_id):
@@ -234,6 +250,25 @@ class XtremIOClient4(XtremIOClient):
 
         return self.req('clusters', name=self.cluster_id)['content']
 
+    def create_snapshot(self, src, dest, ro=False):
+        data = {'snapshot-set-name': dest, 'snap-suffix': dest,
+                'volume-list': [src],
+                'snapshot-type': 'readonly' if ro else 'regular'}
+
+        res = self.req('snapshots', 'POST', data, ver='v2')
+        typ, idx = res['links'][0]['href'].split('/')[-2:]
+
+        # rename the snapshot
+        data = {'name': dest}
+        try:
+            self.req(typ, 'PUT', data, idx=int(idx))
+        except exception.VolumeBackendAPIException:
+            # reverting
+            msg = _LE('Failed to rename the created snapshot, reverting.')
+            LOG.error(msg)
+            self.req(typ, 'DELETE', idx=int(idx))
+            raise
+
 
 class XtremIOVolumeDriver(san.SanDriver):
     """Executes commands relating to Volumes."""
@@ -292,17 +327,11 @@ class XtremIOVolumeDriver(san.SanDriver):
 
     def create_volume_from_snapshot(self, volume, snapshot):
         """Creates a volume from a snapshot."""
-        data = {'snap-vol-name': volume['id'],
-                'ancestor-vol-id': snapshot.id}
-
-        self.client.req('snapshots', 'POST', data)
+        self.client.create_snapshot(snapshot.id, volume['id'])
 
     def create_cloned_volume(self, volume, src_vref):
         """Creates a clone of the specified volume."""
-        data = {'snap-vol-name': volume['id'],
-                'ancestor-vol-id': src_vref['id']}
-
-        self.client.req('snapshots', 'POST', data)
+        self.client.create_snapshot(src_vref['id'], volume['id'])
 
     def delete_volume(self, volume):
         """Deletes a volume."""
@@ -313,10 +342,7 @@ class XtremIOVolumeDriver(san.SanDriver):
 
     def create_snapshot(self, snapshot):
         """Creates a snapshot."""
-        data = {'snap-vol-name': snapshot.id,
-                'ancestor-vol-id': snapshot.volume_id}
-
-        self.client.req('snapshots', 'POST', data)
+        self.client.create_snapshot(snapshot.volume_id, snapshot.id, True)
 
     def delete_snapshot(self, snapshot):
         """Deletes a snapshot."""
-- 
2.45.2