From acc7ddfb6d8edc7e6df7204089b25ba2e996ebef Mon Sep 17 00:00:00 2001 From: Mike Rooney Date: Mon, 31 Aug 2015 11:29:53 -0400 Subject: [PATCH] Local img-cache files ignored for image transfers The cDOT NFS driver will only attempt to use the local img-cache-* files if the copy offload path is defined in the cinder.conf file. This should be attempted regardless of copy-offload's configuration. Change-Id: I29e19da7c1e083e9c76cbcea1f397deefe30b016 Closes-Bug: #1490641 --- cinder/tests/unit/test_netapp_nfs.py | 45 +++++++++++-------- .../drivers/netapp/dataontap/nfs_base.py | 2 +- .../drivers/netapp/dataontap/nfs_cmode.py | 35 +++++++-------- 3 files changed, 45 insertions(+), 37 deletions(-) diff --git a/cinder/tests/unit/test_netapp_nfs.py b/cinder/tests/unit/test_netapp_nfs.py index ac9567799..20b1a8678 100644 --- a/cinder/tests/unit/test_netapp_nfs.py +++ b/cinder/tests/unit/test_netapp_nfs.py @@ -43,11 +43,11 @@ from cinder.volume.drivers.netapp.dataontap import nfs_base from cinder.volume.drivers.netapp.dataontap import ssc_cmode from cinder.volume.drivers.netapp import utils - from oslo_config import cfg -CONF = cfg.CONF +CONF = cfg.CONF + CONNECTION_INFO = { 'hostname': 'fake_host', 'transport_type': 'https', @@ -392,10 +392,10 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): mox = self.mox drv._mounted_shares = ['testshare'] mox.StubOutWithMock(drv, '_get_mount_point_for_share') - mox.StubOutWithMock(os.path, 'exists') + mox.StubOutWithMock(os.path, 'isfile') drv._get_mount_point_for_share('testshare').AndReturn('/mnt') - os.path.exists('/mnt/img-cache-id').AndReturn(True) + os.path.isfile('/mnt/img-cache-id').AndReturn(True) mox.ReplayAll() result = drv._find_image_in_cache('id') (share, file_name) = result[0] @@ -1300,15 +1300,15 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase): image_service = object() image_id = 'image_id' drv.zapi_client.get_ontapi_version = mock.Mock(return_value=(1, 20)) - drv._try_copyoffload = mock.Mock() + drv._copy_from_img_service = mock.Mock() drv._get_provider_location = mock.Mock(return_value='share') drv._get_vol_for_share = mock.Mock(return_value='vol') drv._update_stale_vols = mock.Mock() drv.copy_image_to_volume(context, volume, image_service, image_id) - drv._try_copyoffload.assert_called_once_with(context, volume, - image_service, - image_id) + drv._copy_from_img_service.assert_called_once_with(context, volume, + image_service, + image_id) drv._update_stale_vols.assert_called_once_with('vol') def test_copy_img_to_vol_copyoffload_failure(self): @@ -1318,17 +1318,17 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase): image_service = object() image_id = 'image_id' drv.zapi_client.get_ontapi_version = mock.Mock(return_value=(1, 20)) - drv._try_copyoffload = mock.Mock(side_effect=Exception()) + drv._copy_from_img_service = mock.Mock(side_effect=Exception()) nfs_base.NetAppNfsDriver.copy_image_to_volume = mock.Mock() drv._get_provider_location = mock.Mock(return_value='share') drv._get_vol_for_share = mock.Mock(return_value='vol') drv._update_stale_vols = mock.Mock() drv.copy_image_to_volume(context, volume, image_service, image_id) - drv._try_copyoffload.assert_called_once_with(context, volume, - image_service, - image_id) - nfs_base.NetAppNfsDriver.copy_image_to_volume.\ + drv._copy_from_img_service.assert_called_once_with(context, volume, + image_service, + image_id) + nfs_base.NetAppNfsDriver.copy_image_to_volume. \ assert_called_once_with(context, volume, image_service, image_id) drv._update_stale_vols.assert_called_once_with('vol') @@ -1356,7 +1356,7 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase): drv._clone_file_dst_exists = mock.Mock(side_effect=OSError()) # Verify the original error is propagated - self.assertRaises(OSError, drv._try_copyoffload, + self.assertRaises(OSError, drv._copy_from_img_service, context, volume, image_service, image_id) def test_copyoffload_frm_cache_success(self): @@ -1365,10 +1365,15 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase): volume = {'id': 'vol_id', 'name': 'name'} image_service = object() image_id = 'image_id' + drv.zapi_client.get_ontapi_version = mock.Mock(return_value=(1, 20)) + nfs_base.NetAppNfsDriver.copy_image_to_volume = mock.Mock() + drv._get_provider_location = mock.Mock(return_value='share') + drv._get_vol_for_share = mock.Mock(return_value='vol') + drv._update_stale_vols = mock.Mock() drv._find_image_in_cache = mock.Mock(return_value=[('share', 'img')]) drv._copy_from_cache = mock.Mock(return_value=True) - drv._try_copyoffload(context, volume, image_service, image_id) + drv.copy_image_to_volume(context, volume, image_service, image_id) drv._copy_from_cache.assert_called_once_with(volume, image_id, [('share', 'img')]) @@ -1380,11 +1385,15 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase): image_service = object() image_id = 'image_id' drv._client = mock.Mock() - drv._client.get_api_version = mock.Mock(return_value=(1, 20)) - drv._find_image_in_cache = mock.Mock(return_value=[]) + drv.zapi_client.get_ontapi_version = mock.Mock(return_value=(1, 20)) + nfs_base.NetAppNfsDriver.copy_image_to_volume = mock.Mock() + drv._get_provider_location = mock.Mock(return_value='share') + drv._get_vol_for_share = mock.Mock(return_value='vol') + drv._update_stale_vols = mock.Mock() + drv._find_image_in_cache = mock.Mock(return_value=False) drv._copy_from_img_service = mock.Mock() - drv._try_copyoffload(context, volume, image_service, image_id) + drv.copy_image_to_volume(context, volume, image_service, image_id) drv._copy_from_img_service.assert_called_once_with(context, volume, image_service, diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_base.py b/cinder/volume/drivers/netapp/dataontap/nfs_base.py index 37623ed86..2cfc1feab 100644 --- a/cinder/volume/drivers/netapp/dataontap/nfs_base.py +++ b/cinder/volume/drivers/netapp/dataontap/nfs_base.py @@ -324,7 +324,7 @@ class NetAppNfsDriver(driver.ManageableVD, dir = self._get_mount_point_for_share(share) file_name = 'img-cache-%s' % image_id file_path = '%s/%s' % (dir, file_name) - if os.path.exists(file_path): + if os.path.isfile(file_path): LOG.debug('Found cache file for image %(image_id)s' ' on share %(share)s', {'image_id': image_id, 'share': share}) diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py b/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py index 0dcb6d9d9..0e06045c2 100644 --- a/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py +++ b/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py @@ -351,15 +351,24 @@ class NetAppCmodeNfsDriver(nfs_base.NetAppNfsDriver): try: major, minor = self.zapi_client.get_ontapi_version() col_path = self.configuration.netapp_copyoffload_tool_path - if major == 1 and minor >= 20 and col_path: - self._try_copyoffload(context, volume, image_service, image_id) - copy_success = True - LOG.info(_LI('Copied image %(img)s to volume %(vol)s using ' - 'copy offload workflow.'), + # Search the local image cache before attempting copy offload + cache_result = self._find_image_in_cache(image_id) + if cache_result: + copy_success = self._copy_from_cache(volume, image_id, + cache_result) + if copy_success: + LOG.info(_LI('Copied image %(img)s to volume %(vol)s ' + 'using local image cache.'), + {'img': image_id, 'vol': volume['id']}) + # Image cache was not present, attempt copy offload workflow + if not copy_success and col_path and major == 1 and minor >= 20: + LOG.debug('No result found in image cache') + self._copy_from_img_service(context, volume, image_service, + image_id) + LOG.info(_LI('Copied image %(img)s to volume %(vol)s using' + ' copy offload workflow.'), {'img': image_id, 'vol': volume['id']}) - else: - LOG.debug("Copy offload either not configured or" - " unsupported.") + copy_success = True except Exception as e: LOG.exception(_LE('Copy offload workflow unsuccessful. %s'), e) finally: @@ -370,16 +379,6 @@ class NetAppCmodeNfsDriver(nfs_base.NetAppNfsDriver): sh = self._get_provider_location(volume['id']) self._update_stale_vols(self._get_vol_for_share(sh)) - def _try_copyoffload(self, context, volume, image_service, image_id): - """Tries server side file copy offload.""" - copied = False - cache_result = self._find_image_in_cache(image_id) - if cache_result: - copied = self._copy_from_cache(volume, image_id, cache_result) - if not cache_result or not copied: - self._copy_from_img_service(context, volume, image_service, - image_id) - def _get_ip_verify_on_cluster(self, host): """Verifies if host on same cluster and returns ip.""" ip = na_utils.resolve_hostname(host) -- 2.45.2