From: Mitsuhiro Tanino <mitsuhiro.tanino@hds.com>
Date: Wed, 18 Mar 2015 15:48:25 +0000 (-0400)
Subject: Return updated volume object to the caller of _attach_volume()
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=bbf1b4950fed90338aa38cd93f41fc86d5a66f99;p=openstack-build%2Fcinder-build.git

Return updated volume object to the caller of _attach_volume()

Currently, migrate volume between two back-ends using LIO scsi
target fails due to the error of "Unable to terminate volume
connection: 'NoneType' object has no attribute 'split'".

The root cause of this error is _detach_volume() during volume
migration does not use updated volume object which is updated
during _attach_volume() by create_export().
As a result terminate_connection() which is called from
_detach_volume() tries to split volume['provider_location']
even if the entry is None and causes above error.

Also same error would be happened following methods.

- copy_volume_data(-> volume migration)
- backup_volume
- restore_backup
- copy_image_to_volume
- copy_volume_to_image

This change adds updated volume as an additional return value
for _attach_volume().

Change-Id: I7b76941d8b2e761c2c0147b13a201583d7e0a1c4
Closes-Bug: #1433360
---

diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index bcf20c2df..353dde1c4 100644
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -4740,7 +4740,7 @@ class GenericVolumeDriverTestCase(DriverTestCase):
             get_connector_properties(root_helper, CONF.my_ip, False, False).\
             AndReturn(properties)
         self.volume.driver._attach_volume(self.context, vol, properties).\
-            AndReturn(attach_info)
+            AndReturn((attach_info, vol))
         os.getuid()
         utils.execute('chown', None, '/dev/null', run_as_root=True)
         f = fileutils.file_open('/dev/null').AndReturn(file('/dev/null'))
@@ -4773,7 +4773,7 @@ class GenericVolumeDriverTestCase(DriverTestCase):
             get_connector_properties(root_helper, CONF.my_ip, False, False).\
             AndReturn(properties)
         self.volume.driver._attach_volume(self.context, vol, properties).\
-            AndReturn(attach_info)
+            AndReturn((attach_info, vol))
         os.getuid()
         utils.execute('chown', None, '/dev/null', run_as_root=True)
         f = fileutils.file_open('/dev/null', 'wb').AndReturn(file('/dev/null'))
diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py
index a4b77f297..8ccf46a66 100644
--- a/cinder/volume/driver.py
+++ b/cinder/volume/driver.py
@@ -492,10 +492,11 @@ class BaseVD(object):
         dest_remote = True if remote in ['dest', 'both'] else False
         dest_orig_status = dest_vol['status']
         try:
-            dest_attach_info = self._attach_volume(context,
-                                                   dest_vol,
-                                                   properties,
-                                                   remote=dest_remote)
+            dest_attach_info, dest_vol = self._attach_volume(
+                context,
+                dest_vol,
+                properties,
+                remote=dest_remote)
         except Exception:
             with excutils.save_and_reraise_exception():
                 msg = _("Failed to attach volume %(vol)s")
@@ -506,10 +507,10 @@ class BaseVD(object):
         src_remote = True if remote in ['src', 'both'] else False
         src_orig_status = src_vol['status']
         try:
-            src_attach_info = self._attach_volume(context,
-                                                  src_vol,
-                                                  properties,
-                                                  remote=src_remote)
+            src_attach_info, src_vol = self._attach_volume(context,
+                                                           src_vol,
+                                                           properties,
+                                                           remote=src_remote)
         except Exception:
             with excutils.save_and_reraise_exception():
                 msg = _("Failed to attach volume %(vol)s")
@@ -549,7 +550,7 @@ class BaseVD(object):
         enforce_multipath = self.configuration.enforce_multipath_for_image_xfer
         properties = utils.brick_get_connector_properties(use_multipath,
                                                           enforce_multipath)
-        attach_info = self._attach_volume(context, volume, properties)
+        attach_info, volume = self._attach_volume(context, volume, properties)
 
         try:
             image_utils.fetch_to_raw(context,
@@ -569,7 +570,7 @@ class BaseVD(object):
         enforce_multipath = self.configuration.enforce_multipath_for_image_xfer
         properties = utils.brick_get_connector_properties(use_multipath,
                                                           enforce_multipath)
-        attach_info = self._attach_volume(context, volume, properties)
+        attach_info, volume = self._attach_volume(context, volume, properties)
 
         try:
             image_utils.upload_volume(context,
@@ -675,7 +676,7 @@ class BaseVD(object):
                     LOG.error(err_msg)
                     raise exception.VolumeBackendAPIException(data=ex_msg)
                 raise exception.VolumeBackendAPIException(data=err_msg)
-        return self._connect_device(conn)
+        return (self._connect_device(conn), volume)
 
     def _connect_device(self, conn):
         # Use Brick's code to do attach/detach
@@ -718,7 +719,7 @@ class BaseVD(object):
         enforce_multipath = self.configuration.enforce_multipath_for_image_xfer
         properties = utils.brick_get_connector_properties(use_multipath,
                                                           enforce_multipath)
-        attach_info = self._attach_volume(context, volume, properties)
+        attach_info, volume = self._attach_volume(context, volume, properties)
 
         try:
             volume_path = attach_info['device']['path']
@@ -746,7 +747,7 @@ class BaseVD(object):
         enforce_multipath = self.configuration.enforce_multipath_for_image_xfer
         properties = utils.brick_get_connector_properties(use_multipath,
                                                           enforce_multipath)
-        attach_info = self._attach_volume(context, volume, properties)
+        attach_info, volume = self._attach_volume(context, volume, properties)
 
         try:
             volume_path = attach_info['device']['path']