GPFS Driver missing clone depth limit for snapshots

GPFS driver is fixed to handle snapshot clones correctly. Previously,
these were allowed to grow without respecting limit defined in config
flag gpfs_max_clone_depth.  This change adds the depth check operation
in create_snapshot.

To ensure that all clone files are cleaned up, the delete_snapshot
method now marks snapshots to be deleted with ts file extension
and attempts to delete the snapshot.  If the snapshot cannot be
deleted because it has clone children, it will be deleted when the
child is deleted.

Closes-Bug: #1227366

Change-Id: I4fb2a720b55dbe033159e6fb341f6e2f1508776e

diff --git a/cinder/tests/test_gpfs.py b/cinder/tests/test_gpfs.py
index 85bf212ea899aa935e9bc82003d1e6505d0b7828..befe7b21309335db48061a1ccbbaeec1429829de 100644 (file)
--- a/cinder/tests/test_gpfs.py
+++ b/cinder/tests/test_gpfs.py
@@ -202,8 +202,8 @@ class GPFSDriverTestCase(test.TestCase):
         snapCount = len(db.snapshot_get_all_for_volume(self.context,
                                                        volume_src['id']))
         self.assertTrue(snapCount == 1)
-        self.volume.delete_volume(self.context, volume_src['id'])
         self.volume.delete_snapshot(self.context, snapshot_id)
+        self.volume.delete_volume(self.context, volume_src['id'])
         self.assertFalse(os.path.exists(os.path.join(self.volumes_path,
                                                      snapshot['name'])))
         snapCount = len(db.snapshot_get_all_for_volume(self.context,
@@ -230,8 +230,8 @@ class GPFSDriverTestCase(test.TestCase):
                          volume_dst['id']).snapshot_id)
         self.volume.delete_volume(self.context, volume_dst['id'])
 
-        self.volume.delete_volume(self.context, volume_src['id'])
         self.volume.delete_snapshot(self.context, snapshot_id)
+        self.volume.delete_volume(self.context, volume_src['id'])
 
     def test_create_cloned_volume(self):
         volume_src = test_utils.create_volume(self.context, host=CONF.host)
@@ -268,9 +268,9 @@ class GPFSDriverTestCase(test.TestCase):
         volumepath = os.path.join(self.volumes_path, volume_dst['name'])
         self.assertTrue(os.path.exists(volumepath))
 
+        self.volume.delete_snapshot(self.context, snapshot_id)
         self.volume.delete_volume(self.context, volume_dst['id'])
         self.volume.delete_volume(self.context, volume_src['id'])
-        self.volume.delete_snapshot(self.context, snapshot_id)
 
     def test_clone_image_to_volume_with_copy_on_write_mode(self):
         """Test the function of copy_image_to_volume

diff --git a/cinder/volume/drivers/gpfs.py b/cinder/volume/drivers/gpfs.py
index 9b156f0e53d08416aef35719f9eea4b03f296a70..5f6d00b826ec504e04bcb70ec58f32f1863a3568 100644 (file)
--- a/cinder/volume/drivers/gpfs.py
+++ b/cinder/volume/drivers/gpfs.py
@@ -323,7 +323,7 @@ class GPFSDriver(driver.VolumeDriver):
         # would succeed and the snapshot is deleted.
         if not os.path.exists(fchild) and fparent:
             fpbase = os.path.basename(fparent)
-            if (fpbase.startswith('snapshot-') or fpbase.endswith('.snap')):
+            if (fpbase.endswith('.snap') or fpbase.endswith('.ts')):
                 self._delete_gpfs_file(fparent)
 
     def delete_volume(self, volume):
@@ -381,11 +381,20 @@ class GPFSDriver(driver.VolumeDriver):
         volume_path = os.path.join(self.configuration.gpfs_mount_point_base,
                                    snapshot['volume_name'])
         self._create_gpfs_snap(src=volume_path, dest=snapshot_path)
+        self._gpfs_redirect(volume_path)
 
     def delete_snapshot(self, snapshot):
         """Deletes a GPFS snapshot."""
-        # A snapshot file is deleted as a part of delete_volume when
-        # all volumes derived from it are deleted.
+        # Rename the deleted snapshot to indicate it no longer exists in
+        # cinder db. Attempt to delete the snapshot.  If the snapshot has
+        # clone children, the delete will fail silently. When volumes that
+        # are clone children are deleted in the future, the remaining ts
+        # snapshots will also be deleted.
+        snapshot_path = self.local_path(snapshot)
+        snapshot_ts_path = '%s.ts' % snapshot_path
+        os.rename(snapshot_path, snapshot_ts_path)
+        self._execute('rm', '-f', snapshot_ts_path,
+                      check_exit_code=False, run_as_root=True)
 
     def local_path(self, volume):
         return os.path.join(self.configuration.gpfs_mount_point_base,