From: Navneet Singh <singn@netapp.com>
Date: Sat, 13 Jul 2013 06:55:30 +0000 (+0530)
Subject: Clone_image should return dict of vol properties, clone status.
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=632cdeca7923b7a06c3d7537b89a21d1790f230d;p=openstack-build%2Fcinder-build.git

Clone_image should return dict of vol properties, clone status.

The method will work fine in case of drivers not dependent
on volume properties like provider_location. It will fail to
restart in case of nfs drivers and also leave volume created as
result of clone_image functionality created in the
nfs share in dangling state after deletion as provider_location
is None. This fix requires dict of volume properties, cloned status
to be returned which facilitates passing back provider_location
in case of nfs drivers and hence resolves the issue.

bug 1200708

Change-Id: I590571e52d1c64b6dba7d7e76cd71badd74e51d1
---

diff --git a/cinder/tests/test_rbd.py b/cinder/tests/test_rbd.py
index a36a278b2..8272389c0 100644
--- a/cinder/tests/test_rbd.py
+++ b/cinder/tests/test_rbd.py
@@ -454,7 +454,7 @@ class ManagedRBDTestCase(DriverTestCase):
         afterwards.
         """
         def fake_clone_image(volume, image_location):
-            return True
+            return {'provider_location': None}, True
 
         def fake_clone_error(volume, image_location):
             raise exception.CinderException()
diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py
index a5ce64444..0f95ce0de 100644
--- a/cinder/volume/driver.py
+++ b/cinder/volume/driver.py
@@ -201,9 +201,10 @@ class VolumeDriver(object):
         image service backend in use. The driver should use it
         to determine whether cloning is possible.
 
-        Returns a boolean indicating whether cloning occurred
+        Returns a dict of volume properties eg. provider_location,
+        boolean indicating whether cloning occurred
         """
-        return False
+        return None, False
 
     def backup_volume(self, context, backup, backup_service):
         """Create a new backup from an existing volume."""
diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py
index a905a15db..3b943089b 100644
--- a/cinder/volume/drivers/lvm.py
+++ b/cinder/volume/drivers/lvm.py
@@ -316,7 +316,7 @@ class LVMVolumeDriver(driver.VolumeDriver):
             self.delete_snapshot(temp_snapshot)
 
     def clone_image(self, volume, image_location):
-        return False
+        return None, False
 
     def backup_volume(self, context, backup, backup_service):
         """Create a new backup from an existing volume."""
diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py
index 288742b08..7252e349c 100644
--- a/cinder/volume/drivers/rbd.py
+++ b/cinder/volume/drivers/rbd.py
@@ -486,7 +486,7 @@ class RBDDriver(driver.VolumeDriver):
         _, pool, image, snapshot = self._parse_location(image_location)
         self._clone(volume, pool, image, snapshot)
         self._resize(volume)
-        return True
+        return {'provider_location': None}, True
 
     def _ensure_tmp_exists(self):
         tmp_dir = self.configuration.volume_tmp_dir
diff --git a/cinder/volume/drivers/scality.py b/cinder/volume/drivers/scality.py
index 03e97fe63..3a6a49813 100644
--- a/cinder/volume/drivers/scality.py
+++ b/cinder/volume/drivers/scality.py
@@ -253,6 +253,7 @@ class ScalityDriver(driver.VolumeDriver):
         image service backend in use. The driver should use it
         to determine whether cloning is possible.
 
-        Returns a boolean indicating whether cloning occurred
+        Returns a dict of volume properties eg. provider_location,
+        boolean indicating whether cloning occurred
         """
-        return False
+        return None, False
diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py
index 7c8500797..32ed8981b 100644
--- a/cinder/volume/manager.py
+++ b/cinder/volume/manager.py
@@ -185,7 +185,11 @@ class VolumeManager(manager.SchedulerDependentManager):
                                       {'bootable': True})
         else:
             # create the volume from an image
-            cloned = self.driver.clone_image(volume_ref, image_location)
+            # NOTE (singn): two params need to be returned
+            # dict containing provider_location for cloned volume
+            # and clone status
+            model_update, cloned = self.driver.clone_image(
+                volume_ref, image_location)
             if not cloned:
                 model_update = self.driver.create_volume(volume_ref)