From: Bill Owen <billowen@us.ibm.com>
Date: Wed, 18 Sep 2013 21:30:07 +0000 (-0700)
Subject: GPFS Driver missing clone depth limit for snapshots
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=cbb057169094e9d633162cc5645fa19a82325d49;p=openstack-build%2Fcinder-build.git

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 85bf212ea..befe7b213 100644
--- 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 9b156f0e5..5f6d00b82 100644
--- 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,