Merge "Remove duplicated code in volume manager and base driver"

cinder/tests/unit/test_hpe_xp_fc.py

@@ -567,38 +567,6 @@ class HPEXPFCDriverTest(test.TestCase):
         rc = self.driver.get_volume_stats(True)
         self.assertEqual({}, rc)
 
-    @mock.patch.object(driver.FibreChannelDriver, 'copy_volume_data')
-    def test_copy_volume_data(self, arg1):
-        """Test copy_volume_data."""
-        volume = fake_volume.fake_db_volume(**self._VOLUME)
-        rc_vol = self.driver.create_volume(volume)
-        volume['provider_location'] = rc_vol['provider_location']
-
-        volume2 = fake_volume.fake_db_volume(**self._VOLUME2)
-        rc_vol2 = self.driver.create_volume(volume2)
-        volume2['provider_location'] = rc_vol2['provider_location']
-
-        self.driver.copy_volume_data(None, volume, volume2, None)
-
-        arg1.assert_called_with(None, volume, volume2, None)
-
-    @mock.patch.object(driver.FibreChannelDriver, 'copy_volume_data',
-                       side_effect=exception.CinderException)
-    def test_copy_volume_data_error(self, arg1):
-        """Test copy_volume_data is error."""
-        volume = fake_volume.fake_db_volume(**self._VOLUME)
-        rc_vol = self.driver.create_volume(volume)
-        volume['provider_location'] = rc_vol['provider_location']
-
-        volume2 = fake_volume.fake_db_volume(**self._VOLUME2)
-        volume2['provider_location'] = '2'
-
-        self.assertRaises(exception.CinderException,
-                          self.driver.copy_volume_data,
-                          None, volume, volume2, None)
-
-        arg1.assert_called_with(None, volume, volume2, None)
-
     @mock.patch.object(driver.FibreChannelDriver, 'copy_image_to_volume')
     def test_copy_image_to_volume(self, arg1):
         """Test copy_image_to_volume."""

cinder/tests/unit/test_volume.py

@@ -6516,74 +6516,6 @@ class GenericVolumeDriverTestCase(DriverTestCase):
             backup_obj = objects.Backup.get_by_id(self.context, backup.id)
             self.assertEqual(temp_vol.id, backup_obj.temp_volume_id)
 
-    @mock.patch.object(utils, 'brick_get_connector_properties')
-    @mock.patch.object(cinder.volume.driver.VolumeDriver, '_attach_volume')
-    @mock.patch.object(cinder.volume.driver.VolumeDriver, '_detach_volume')
-    @mock.patch.object(volutils, 'copy_volume')
-    @mock.patch.object(volume_rpcapi.VolumeAPI, 'get_capabilities')
-    def test_copy_volume_data(self,
-                              mock_get_capabilities,
-                              mock_copy,
-                              mock_detach,
-                              mock_attach,
-                              mock_get_connector):
-
-        src_vol = tests_utils.create_volume(self.context, size=1,
-                                            host=CONF.host)
-        dest_vol = tests_utils.create_volume(self.context, size=1,
-                                             host=CONF.host)
-        mock_get_connector.return_value = {}
-        self.volume.driver._throttle = mock.MagicMock()
-
-        attach_expected = [
-            mock.call(self.context, dest_vol, {}, remote=False),
-            mock.call(self.context, src_vol, {}, remote=False)]
-
-        detach_expected = [
-            mock.call(self.context, {'device': {'path': 'bar'}},
-                      dest_vol, {}, force=False, remote=False),
-            mock.call(self.context, {'device': {'path': 'foo'}},
-                      src_vol, {}, force=False, remote=False)]
-
-        attach_volume_returns = [
-            ({'device': {'path': 'bar'}}, dest_vol),
-            ({'device': {'path': 'foo'}}, src_vol),
-        ]
-
-        #  Test case for sparse_copy_volume = False
-        mock_attach.side_effect = attach_volume_returns
-        mock_get_capabilities.return_value = {}
-        self.volume.driver.copy_volume_data(self.context,
-                                            src_vol,
-                                            dest_vol)
-
-        self.assertEqual(attach_expected, mock_attach.mock_calls)
-        mock_copy.assert_called_with(
-            'foo', 'bar', 1024, '1M',
-            throttle=self.volume.driver._throttle,
-            sparse=False)
-        self.assertEqual(detach_expected, mock_detach.mock_calls)
-
-        #  Test case for sparse_copy_volume = True
-        mock_attach.reset_mock()
-        mock_detach.reset_mock()
-        mock_attach.side_effect = attach_volume_returns
-        mock_get_capabilities.return_value = {'sparse_copy_volume': True}
-        self.volume.driver.copy_volume_data(self.context,
-                                            src_vol,
-                                            dest_vol)
-
-        self.assertEqual(attach_expected, mock_attach.mock_calls)
-        mock_copy.assert_called_with(
-            'foo', 'bar', 1024, '1M',
-            throttle=self.volume.driver._throttle,
-            sparse=True)
-        self.assertEqual(detach_expected, mock_detach.mock_calls)
-
-        # cleanup resource
-        db.volume_destroy(self.context, src_vol['id'])
-        db.volume_destroy(self.context, dest_vol['id'])
-
     @mock.patch.object(utils, 'brick_get_connector_properties')
     @mock.patch.object(cinder.volume.manager.VolumeManager, '_attach_volume')
     @mock.patch.object(cinder.volume.manager.VolumeManager, '_detach_volume')

cinder/volume/driver.py

@@ -32,7 +32,6 @@ from cinder import objects
 from cinder import utils
 from cinder.volume import rpcapi as volume_rpcapi
 from cinder.volume import throttling
-from cinder.volume import utils as volume_utils
 
 LOG = logging.getLogger(__name__)
 
@@ -736,77 +735,6 @@ class BaseVD(object):
             data["pools"].append(single_pool)
         self._stats = data
 
-    def copy_volume_data(self, context, src_vol, dest_vol, remote=None):
-        """Copy data from src_vol to dest_vol."""
-        LOG.debug('copy_data_between_volumes %(src)s -> %(dest)s.', {
-            'src': src_vol['name'], 'dest': dest_vol['name']})
-
-        use_multipath = self.configuration.use_multipath_for_image_xfer
-        enforce_multipath = self.configuration.enforce_multipath_for_image_xfer
-        properties = utils.brick_get_connector_properties(use_multipath,
-                                                          enforce_multipath)
-        dest_remote = True if remote in ['dest', 'both'] else False
-        dest_orig_status = dest_vol['status']
-        try:
-            dest_attach_info, dest_vol = self._attach_volume(
-                context,
-                dest_vol,
-                properties,
-                remote=dest_remote)
-        except Exception:
-            with excutils.save_and_reraise_exception():
-                LOG.error(_LE("Failed to attach volume %(vol)s"),
-                          {'vol': dest_vol['id']})
-                self.db.volume_update(context, dest_vol['id'],
-                                      {'status': dest_orig_status})
-
-        src_remote = True if remote in ['src', 'both'] else False
-        src_orig_status = src_vol['status']
-        try:
-            src_attach_info, src_vol = self._attach_volume(context,
-                                                           src_vol,
-                                                           properties,
-                                                           remote=src_remote)
-        except Exception:
-            with excutils.save_and_reraise_exception():
-                LOG.error(_LE("Failed to attach volume %(vol)s"),
-                          {'vol': src_vol['id']})
-                self.db.volume_update(context, src_vol['id'],
-                                      {'status': src_orig_status})
-                self._detach_volume(context, dest_attach_info, dest_vol,
-                                    properties, force=True, remote=dest_remote)
-
-        # Check the backend capabilities of migration destination host.
-        rpcapi = volume_rpcapi.VolumeAPI()
-        capabilities = rpcapi.get_capabilities(context, dest_vol['host'],
-                                               False)
-        sparse_copy_volume = bool(capabilities and
-                                  capabilities.get('sparse_copy_volume',
-                                                   False))
-
-        copy_error = True
-        try:
-            size_in_mb = int(src_vol['size']) * 1024    # vol size is in GB
-            volume_utils.copy_volume(
-                src_attach_info['device']['path'],
-                dest_attach_info['device']['path'],
-                size_in_mb,
-                self.configuration.volume_dd_blocksize,
-                throttle=self._throttle,
-                sparse=sparse_copy_volume)
-            copy_error = False
-        except Exception:
-            with excutils.save_and_reraise_exception():
-                LOG.error(_LE("Failed to copy volume %(src)s to %(dest)s."),
-                          {'src': src_vol['id'], 'dest': dest_vol['id']})
-        finally:
-            self._detach_volume(context, dest_attach_info, dest_vol,
-                                properties, force=copy_error,
-                                remote=dest_remote)
-            self._detach_volume(context, src_attach_info, src_vol,
-                                properties, force=copy_error,
-                                remote=src_remote)
-
     def copy_image_to_volume(self, context, volume, image_service, image_id):
         """Fetch the image from image_service and write it to the volume."""
         LOG.debug('copy_image_to_volume %s.', volume['name'])
@@ -845,6 +773,22 @@ class BaseVD(object):
         finally:
             self._detach_volume(context, attach_info, volume, properties)
 
+    def before_volume_copy(self, context, src_vol, dest_vol, remote=None):
+        """Driver-specific actions before copyvolume data.
+
+        This method will be called before _copy_volume_data during volume
+        migration
+        """
+        pass
+
+    def after_volume_copy(self, context, src_vol, dest_vol, remote=None):
+        """Driver-specific actions after copyvolume data.
+
+        This method will be called after _copy_volume_data during volume
+        migration
+        """
+        pass
+
     def get_filter_function(self):
         """Get filter_function string.
 

cinder/volume/drivers/hitachi/hbsd_fc.py

@@ -482,12 +482,6 @@ class HBSDFCDriver(cinder.volume.driver.FibreChannelDriver):
     def remove_export(self, context, volume):
         pass
 
-    def copy_volume_data(self, context, src_vol, dest_vol, remote=None):
-        self.do_setup_status.wait()
-        super(HBSDFCDriver, self).copy_volume_data(context, src_vol,
-                                                   dest_vol, remote)
-        self.discard_zero_page(dest_vol)
-
     def copy_image_to_volume(self, context, volume, image_service, image_id):
         self.do_setup_status.wait()
         super(HBSDFCDriver, self).copy_image_to_volume(context, volume,
@@ -505,6 +499,22 @@ class HBSDFCDriver(cinder.volume.driver.FibreChannelDriver):
                                                        image_service,
                                                        image_meta)
 
+    def before_volume_copy(self, context, src_vol, dest_vol, remote=None):
+        """Driver-specific actions before copyvolume data.
+
+        This method will be called before _copy_volume_data during volume
+        migration
+        """
+        self.do_setup_status.wait()
+
+    def after_volume_copy(self, context, src_vol, dest_vol, remote=None):
+        """Driver-specific actions after copyvolume data.
+
+        This method will be called after _copy_volume_data during volume
+        migration
+        """
+        self.discard_zero_page(dest_vol)
+
     def restore_backup(self, context, backup, volume, backup_service):
         self.do_setup_status.wait()
         super(HBSDFCDriver, self).restore_backup(context, backup,

cinder/volume/drivers/hpe/hpe_xp_fc.py

@@ -74,16 +74,6 @@ class HPEXPFCDriver(driver.FibreChannelDriver):
         """Get volume stats."""
         return self.common.get_volume_stats(refresh)
 
-    def copy_volume_data(self, context, src_vol, dest_vol, remote=None):
-        """Copy data from src_vol to dest_vol.
-
-        Call copy_volume_data() of super class and
-        carry out original postprocessing.
-        """
-        super(HPEXPFCDriver, self).copy_volume_data(
-            context, src_vol, dest_vol, remote)
-        self.common.copy_volume_data(context, src_vol, dest_vol, remote)
-
     def copy_image_to_volume(self, context, volume, image_service, image_id):
         """Fetch the image from image_service and write it to the volume.
 
@@ -95,6 +85,14 @@ class HPEXPFCDriver(driver.FibreChannelDriver):
         self.common.copy_image_to_volume(
             context, volume, image_service, image_id)
 
+    def after_volume_copy(self, context, src_vol, dest_vol, remote=None):
+        """Driver-specific actions after copyvolume data.
+
+        This method will be called after _copy_volume_data during volume
+        migration
+        """
+        self.common.copy_volume_data(context, src_vol, dest_vol, remote)
+
     def restore_backup(self, context, backup, volume, backup_service):
         """Restore an existing backup to a new or existing volume.
 

cinder/volume/manager.py

@@ -1743,7 +1743,13 @@ class VolumeManager(manager.SchedulerDependentManager):
         try:
             attachments = volume.volume_attachment
             if not attachments:
+                # Pre- and post-copy driver-specific actions
+                self.driver.before_volume_copy(ctxt, volume, new_volume,
+                                               remote='dest')
                 self._copy_volume_data(ctxt, volume, new_volume, remote='dest')
+                self.driver.after_volume_copy(ctxt, volume, new_volume,
+                                              remote='dest')
+
                 # The above call is synchronous so we complete the migration
                 self.migrate_volume_completion(ctxt, volume.id,
                                                new_volume.id,