From a6276f0d2d3a86091440f2f851bfbff37bbaac6d Mon Sep 17 00:00:00 2001 From: Dmitry Vasilets Date: Fri, 28 Aug 2026 14:02:02 +0200 Subject: [PATCH 1/3] image cache size --- .../drivers/netapp/dataontap/test_nfs_base.py | 151 ++++++++++++++++-- .../drivers/netapp/dataontap/nfs_base.py | 27 +++- 2 files changed, 164 insertions(+), 14 deletions(-) diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py index 9ea8dab189..ffbb8ef0dc 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py @@ -27,6 +27,7 @@ from cinder import context from cinder import exception from cinder.objects import fields +from cinder.image import image_utils from cinder.tests.unit import fake_snapshot from cinder.tests.unit import fake_volume from cinder.tests.unit import test @@ -550,11 +551,125 @@ def test__update_volume_stats(self): self.assertRaises(NotImplementedError, self.driver._update_volume_stats) + @mock.patch.object(image_utils, 'resize_image') + @mock.patch.object(image_utils, 'qemu_img_info') + @mock.patch.object(image_utils, 'fetch_to_raw') + def test_copy_image_to_volume_cache_uses_image_size( + self, + mock_fetch_to_raw, + mock_qemu_img_info, + mock_resize_image): + + volume = fake_volume.fake_volume_obj( + self.ctxt, + id=fake.VOLUME_ID, + size=10, + host='fake-host') + + image_id = 'image-1' + image_size = 10 * units.Gi + + mock_qemu_img_info.return_value.virtual_size = image_size + + self.mock_object( + self.driver, + '_is_flexgroup', + return_value=False) + + mock_register = self.mock_object( + self.driver, + '_register_image_in_cache') + + self.mock_object( + self.driver, + 'local_path', + return_value=f'/tmp/{volume.id}') + + self.driver.copy_image_to_volume( + mock.sentinel.context, + volume, + mock.sentinel.image_service, + image_id) + + mock_fetch_to_raw.assert_called_once_with( + mock.sentinel.context, + mock.sentinel.image_service, + image_id, + f'/tmp/{volume.id}', + self.driver.configuration.volume_dd_blocksize, + size=volume.size, + run_as_root=self.driver._execute_as_root, + disable_sparse=False) + + mock_qemu_img_info.assert_called_once_with( + f'/tmp/{volume.id}', + run_as_root=self.driver._execute_as_root) + + mock_resize_image.assert_called_once_with( + f'/tmp/{volume.id}', + image_size, + run_as_root=self.driver._execute_as_root) + + mock_register.assert_called_once_with( + volume, image_id) + + @mock.patch.object(image_utils, 'resize_image') + @mock.patch.object(image_utils, 'qemu_img_info') + @mock.patch.object(image_utils, 'fetch_to_raw') + def test_copy_image_to_volume_image_size_mismatch( + self, + mock_fetch_to_raw, + mock_qemu_img_info, + mock_resize_image): + + volume = fake_volume.fake_volume_obj( + self.ctxt, + id=fake.VOLUME_ID, + size=30, + host='fake-host') + + image_id = 'image-1' + + self.mock_object( + self.driver, + '_is_flexgroup', + return_value=False) + + mock_register = self.mock_object( + self.driver, + '_register_image_in_cache') + + self.mock_object( + self.driver, + 'local_path', + return_value=f'/tmp/{volume.id}') + + mock_qemu_img_info.return_value.virtual_size = 10 * units.Gi + + self.assertRaises( + exception.ImageUnacceptable, + self.driver.copy_image_to_volume, + mock.sentinel.context, + volume, + mock.sentinel.image_service, + image_id) + + mock_fetch_to_raw.assert_called_once() + mock_qemu_img_info.assert_called_once_with( + f'/tmp/{volume.id}', + run_as_root=self.driver._execute_as_root) + mock_resize_image.assert_called_once_with( + f'/tmp/{volume.id}', + 10 * units.Gi, + run_as_root=self.driver._execute_as_root) + + mock_register.assert_not_called() + def test_copy_image_to_volume_base_exception(self): mock_info_log = self.mock_object(nfs_base.LOG, 'info') self.mock_object(self.driver, '_ensure_flexgroup_not_in_cg') - self.mock_object(remotefs.RemoteFSDriver, 'copy_image_to_volume', - side_effect=exception.NfsException) + self.mock_object(self.driver, 'local_path', + mock.Mock(side_effect=exception.NfsException)) self.assertRaises(exception.NfsException, self.driver.copy_image_to_volume, @@ -562,29 +677,43 @@ def test_copy_image_to_volume_base_exception(self): 'fake_img_service', fake.IMAGE_FILE_ID) mock_info_log.assert_not_called() - def test_copy_image_to_volume(self): + @mock.patch.object(image_utils, 'resize_image') + @mock.patch.object(image_utils, 'qemu_img_info') + @mock.patch.object(image_utils, 'fetch_to_raw') + def test_copy_image_to_volume(self, + mock_fetch_to_raw, + mock_qemu_img_info, + mock_resize_image): + volume = fake_volume.fake_volume_obj(self.ctxt, **fake.NFS_VOLUME) + mock_qemu_img_info.return_value.virtual_size = ( + volume.size * units.Gi) + mock_log = self.mock_object(nfs_base, 'LOG') self.mock_object(self.driver, '_is_flexgroup', return_value=False) self.mock_object(self.driver, '_is_flexgroup_clone_file_supported', return_value=True) self.mock_object(self.driver, '_ensure_flexgroup_not_in_cg') - mock_copy_image = self.mock_object( - remotefs.RemoteFSDriver, 'copy_image_to_volume') + mock_local_path = self.mock_object(self.driver, 'local_path') mock_register_image = self.mock_object( self.driver, '_register_image_in_cache') + image_service = mock.Mock() + image_service.show.return_value = { + 'disk_format': 'raw', + 'container_format': 'bare', + } self.driver.copy_image_to_volume('fake_context', - fake.NFS_VOLUME, - 'fake_img_service', + volume, + image_service, fake.IMAGE_FILE_ID) - mock_copy_image.assert_called_once_with( - 'fake_context', fake.NFS_VOLUME, 'fake_img_service', - fake.IMAGE_FILE_ID, disable_sparse=False) + self.assertEqual(1, mock_fetch_to_raw.call_count) + self.assertEqual(1, mock_qemu_img_info.call_count) + self.assertEqual(1, mock_resize_image.call_count) self.assertEqual(1, mock_log.info.call_count) mock_register_image.assert_called_once_with( - fake.NFS_VOLUME, fake.IMAGE_FILE_ID) + volume, fake.IMAGE_FILE_ID) @ddt.data(None, Exception) def test__register_image_in_cache(self, exc): diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_base.py b/cinder/volume/drivers/netapp/dataontap/nfs_base.py index e25e42e631..7f21431748 100644 --- a/cinder/volume/drivers/netapp/dataontap/nfs_base.py +++ b/cinder/volume/drivers/netapp/dataontap/nfs_base.py @@ -511,9 +511,30 @@ def copy_image_to_volume(self, context, volume, image_service, image_id, disable_sparse=False): """Fetch the image from image_service and write it to the volume.""" self._ensure_flexgroup_not_in_cg(volume) - super(NetAppNfsDriver, self).copy_image_to_volume( - context, volume, image_service, image_id, - disable_sparse=disable_sparse) + # modified behaviour for cinder/volume/drivers/remotefs.py +530 + image_utils.fetch_to_raw(context, + image_service, + image_id, + self.local_path(volume), + self.configuration.volume_dd_blocksize, + size=volume.size, + run_as_root=self._execute_as_root, + disable_sparse=disable_sparse) + + data = image_utils.qemu_img_info(self.local_path(volume), + run_as_root=self._execute_as_root) + virt_size = int(data.virtual_size // units.Gi) + + image_utils.resize_image(self.local_path(volume), data.virtual_size, + run_as_root=self._execute_as_root) + + if virt_size != volume.size: + raise exception.ImageUnacceptable( + image_id=image_id, + reason=(_("Expected volume size was %d") % volume.size) + + (_(" but size is now %d") % virt_size)) + # end of cinder/volume/drivers/remotefs.py +530 + LOG.info('Copied image to volume %s using regular download.', volume['id']) From b34d4823ae01366ad640e9899cf7a4febe8bff6f Mon Sep 17 00:00:00 2001 From: Dmitry Vasilets Date: Wed, 16 Sep 2026 15:53:46 +0200 Subject: [PATCH 2/3] register in cache before resize --- .../drivers/netapp/dataontap/test_nfs_base.py | 8 +++---- .../drivers/netapp/dataontap/nfs_base.py | 24 ++++++++----------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py index ffbb8ef0dc..00f947f459 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py @@ -567,9 +567,9 @@ def test_copy_image_to_volume_cache_uses_image_size( host='fake-host') image_id = 'image-1' - image_size = 10 * units.Gi + image_size = 10 - mock_qemu_img_info.return_value.virtual_size = image_size + mock_qemu_img_info.return_value.virtual_size = image_size * units.Gi self.mock_object( self.driver, @@ -660,10 +660,10 @@ def test_copy_image_to_volume_image_size_mismatch( run_as_root=self.driver._execute_as_root) mock_resize_image.assert_called_once_with( f'/tmp/{volume.id}', - 10 * units.Gi, + 30, run_as_root=self.driver._execute_as_root) - mock_register.assert_not_called() + mock_register.assert_called_once() def test_copy_image_to_volume_base_exception(self): mock_info_log = self.mock_object(nfs_base.LOG, 'info') diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_base.py b/cinder/volume/drivers/netapp/dataontap/nfs_base.py index 7f21431748..76ee84b4e9 100644 --- a/cinder/volume/drivers/netapp/dataontap/nfs_base.py +++ b/cinder/volume/drivers/netapp/dataontap/nfs_base.py @@ -520,30 +520,26 @@ def copy_image_to_volume(self, context, volume, image_service, image_id, size=volume.size, run_as_root=self._execute_as_root, disable_sparse=disable_sparse) + LOG.info('Copied image to volume %s using regular download.', + volume['id']) + if (not self._is_flexgroup(host=volume['host']) or + self._is_flexgroup_clone_file_supported()): + # NOTE(felipe_rodrigues): NetApp image cache relies on the + # FlexClone file, which is only available for the earliest + # versions of FlexGroup. + self._register_image_in_cache(volume, image_id) + image_utils.resize_image(self.local_path(volume), volume.size, + run_as_root=self._execute_as_root) data = image_utils.qemu_img_info(self.local_path(volume), run_as_root=self._execute_as_root) virt_size = int(data.virtual_size // units.Gi) - - image_utils.resize_image(self.local_path(volume), data.virtual_size, - run_as_root=self._execute_as_root) - if virt_size != volume.size: raise exception.ImageUnacceptable( image_id=image_id, reason=(_("Expected volume size was %d") % volume.size) + (_(" but size is now %d") % virt_size)) - # end of cinder/volume/drivers/remotefs.py +530 - LOG.info('Copied image to volume %s using regular download.', - volume['id']) - - if (not self._is_flexgroup(host=volume['host']) or - self._is_flexgroup_clone_file_supported()): - # NOTE(felipe_rodrigues): NetApp image cache relies on the - # FlexClone file, which is only available for the earliest - # versions of FlexGroup. - self._register_image_in_cache(volume, image_id) def _register_image_in_cache(self, volume, image_id): """Stores image in the cache.""" From cde62fbf133bbecc485275b3bbeb0fa69a1b2fb6 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilets Date: Wed, 16 Sep 2026 17:31:05 +0200 Subject: [PATCH 3/3] pep8 env rerun and fixed --- .../volume/drivers/netapp/dataontap/test_nfs_base.py | 10 +++++----- cinder/volume/drivers/netapp/dataontap/nfs_base.py | 1 - 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py index 00f947f459..e224dba27f 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py @@ -26,8 +26,8 @@ from cinder import context from cinder import exception -from cinder.objects import fields from cinder.image import image_utils +from cinder.objects import fields from cinder.tests.unit import fake_snapshot from cinder.tests.unit import fake_volume from cinder.tests.unit import test @@ -567,7 +567,7 @@ def test_copy_image_to_volume_cache_uses_image_size( host='fake-host') image_id = 'image-1' - image_size = 10 + image_size = 10 mock_qemu_img_info.return_value.virtual_size = image_size * units.Gi @@ -669,7 +669,7 @@ def test_copy_image_to_volume_base_exception(self): mock_info_log = self.mock_object(nfs_base.LOG, 'info') self.mock_object(self.driver, '_ensure_flexgroup_not_in_cg') self.mock_object(self.driver, 'local_path', - mock.Mock(side_effect=exception.NfsException)) + mock.Mock(side_effect=exception.NfsException)) self.assertRaises(exception.NfsException, self.driver.copy_image_to_volume, @@ -687,14 +687,14 @@ def test_copy_image_to_volume(self, volume = fake_volume.fake_volume_obj(self.ctxt, **fake.NFS_VOLUME) mock_qemu_img_info.return_value.virtual_size = ( volume.size * units.Gi) - + mock_log = self.mock_object(nfs_base, 'LOG') self.mock_object(self.driver, '_is_flexgroup', return_value=False) self.mock_object(self.driver, '_is_flexgroup_clone_file_supported', return_value=True) self.mock_object(self.driver, '_ensure_flexgroup_not_in_cg') - mock_local_path = self.mock_object(self.driver, 'local_path') + self.mock_object(self.driver, 'local_path') mock_register_image = self.mock_object( self.driver, '_register_image_in_cache') diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_base.py b/cinder/volume/drivers/netapp/dataontap/nfs_base.py index 76ee84b4e9..063fcf20ba 100644 --- a/cinder/volume/drivers/netapp/dataontap/nfs_base.py +++ b/cinder/volume/drivers/netapp/dataontap/nfs_base.py @@ -540,7 +540,6 @@ def copy_image_to_volume(self, context, volume, image_service, image_id, reason=(_("Expected volume size was %d") % volume.size) + (_(" but size is now %d") % virt_size)) - def _register_image_in_cache(self, volume, image_id): """Stores image in the cache.""" file_name = 'img-cache-%s' % image_id