diff --git a/cinder/image/image_utils.py b/cinder/image/image_utils.py index 0d9821c107d..4570be6dd08 100644 --- a/cinder/image/image_utils.py +++ b/cinder/image/image_utils.py @@ -207,8 +207,8 @@ def _get_qemu_convert_luks_cmd(src: str, prefix: Optional[tuple] = None, cipher_spec: Optional[dict] = None, passphrase_file: Optional[str] = None, - src_passphrase_file: Optional[str] = None) \ - -> list[str]: + src_passphrase_file: Optional[str] = None, + disable_sparse: bool = False) -> list[str]: cmd = ['qemu-img', 'convert'] if prefix: @@ -217,6 +217,9 @@ def _get_qemu_convert_luks_cmd(src: str, if cache_mode: cmd += ('-t', cache_mode) + if disable_sparse: + cmd += ('-S', '0') + obj1 = ['--object', 'secret,id=sec1,format=raw,file=%s' % src_passphrase_file] obj2 = ['--object', @@ -242,8 +245,8 @@ def _get_qemu_convert_cmd(src: str, cipher_spec: Optional[dict] = None, passphrase_file: Optional[str] = None, compress: bool = False, - src_passphrase_file: Optional[str] = None) \ - -> list[str]: + src_passphrase_file: Optional[str] = None, + disable_sparse: bool = False) -> list[str]: if src_passphrase_file is not None: if passphrase_file is None: message = _("Can't create unencrypted volume %(format)s " @@ -262,7 +265,8 @@ def _get_qemu_convert_cmd(src: str, prefix=None, cipher_spec=cipher_spec, passphrase_file=passphrase_file, - src_passphrase_file=src_passphrase_file) + src_passphrase_file=src_passphrase_file, + disable_sparse=disable_sparse) if out_format == 'vhd': # qemu-img still uses the legacy vpc name @@ -276,6 +280,9 @@ def _get_qemu_convert_cmd(src: str, if cache_mode: cmd += ('-t', cache_mode) + if disable_sparse: + cmd += ('-S', '0') + if CONF.image_compress_on_upload and compress: if out_format in COMPRESSIBLE_IMAGE_FORMATS: cmd += ('-c',) @@ -340,7 +347,8 @@ def _convert_image(prefix: tuple, cipher_spec: Optional[dict] = None, passphrase_file: Optional[str] = None, compress: bool = False, - src_passphrase_file: Optional[str] = None) -> None: + src_passphrase_file: Optional[str] = None, + disable_sparse: bool = False) -> None: """Convert image to other format. NOTE: If the qemu-img convert command fails and this function raises an @@ -389,7 +397,8 @@ def _convert_image(prefix: tuple, cipher_spec=cipher_spec, passphrase_file=passphrase_file, compress=compress, - src_passphrase_file=src_passphrase_file) + src_passphrase_file=src_passphrase_file, + disable_sparse=disable_sparse) start_time = timeutils.utcnow() @@ -450,7 +459,8 @@ def convert_image(source: str, compress: bool = False, src_passphrase_file: Optional[str] = None, image_id: Optional[str] = None, - data: Optional[imageutils.QemuImgInfo] = None) -> None: + data: Optional[imageutils.QemuImgInfo] = None, + disable_sparse: bool = False) -> None: """Convert image to other format. NOTE: If the qemu-img convert command fails and this function raises an @@ -489,7 +499,8 @@ def convert_image(source: str, cipher_spec=cipher_spec, passphrase_file=passphrase_file, compress=compress, - src_passphrase_file=src_passphrase_file) + src_passphrase_file=src_passphrase_file, + disable_sparse=disable_sparse) def resize_image(source: str, @@ -812,11 +823,13 @@ def fetch_to_vhd(context: context.RequestContext, volume_subformat: Optional[str] = None, user_id: Optional[str] = None, project_id: Optional[str] = None, - run_as_root: bool = True) -> None: + run_as_root: bool = True, + disable_sparse: bool = False) -> None: fetch_to_volume_format(context, image_service, image_id, dest, 'vpc', blocksize, volume_subformat=volume_subformat, user_id=user_id, project_id=project_id, - run_as_root=run_as_root) + run_as_root=run_as_root, + disable_sparse=disable_sparse) def fetch_to_raw(context: context.RequestContext, @@ -827,10 +840,12 @@ def fetch_to_raw(context: context.RequestContext, user_id: Optional[str] = None, project_id: Optional[str] = None, size: Optional[int] = None, - run_as_root: bool = True) -> None: + run_as_root: bool = True, + disable_sparse: bool = False) -> None: fetch_to_volume_format(context, image_service, image_id, dest, 'raw', blocksize, user_id=user_id, project_id=project_id, - size=size, run_as_root=run_as_root) + size=size, run_as_root=run_as_root, + disable_sparse=disable_sparse) def check_image_conversion_disable(disk_format, volume_format, image_id, @@ -865,7 +880,8 @@ def fetch_to_volume_format(context: context.RequestContext, user_id: Optional[str] = None, project_id: Optional[str] = None, size: Optional[int] = None, - run_as_root: bool = True) -> None: + run_as_root: bool = True, + disable_sparse: bool = False) -> None: qemu_img = True image_meta = image_service.show(context, image_id) @@ -980,7 +996,8 @@ def fetch_to_volume_format(context: context.RequestContext, src_format=disk_format, run_as_root=run_as_root, image_id=image_id, - data=data) + data=data, + disable_sparse=disable_sparse) def upload_volume(context: context.RequestContext, diff --git a/cinder/interface/volume_driver.py b/cinder/interface/volume_driver.py index e0642724d57..804698163bd 100644 --- a/cinder/interface/volume_driver.py +++ b/cinder/interface/volume_driver.py @@ -291,13 +291,15 @@ class VolumeDriverCore(base.CinderInterface): whether the clone occurred. """ - def copy_image_to_volume(self, context, volume, image_service, image_id): + 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. :param context: Security/policy info for the request. :param volume: The volume to create. :param image_service: The image service to use. :param image_id: The image identifier. + :param disable_sparse: Enable or disable sparse copy. Default=False. :returns: Model updates. """ diff --git a/cinder/tests/unit/test_image_utils.py b/cinder/tests/unit/test_image_utils.py index b5b8167a40c..164d58b4035 100644 --- a/cinder/tests/unit/test_image_utils.py +++ b/cinder/tests/unit/test_image_utils.py @@ -314,6 +314,24 @@ class TestConvertImage(test.TestCase): mock_exec.assert_called_once_with(*exec_args, run_as_root=True) + @mock.patch('cinder.image.image_utils.qemu_img_info') + @mock.patch('cinder.utils.execute') + @mock.patch('cinder.utils.is_blk_device', return_value=False) + def test_convert_disable_sparse(self, mock_isblk, + mock_exec, mock_info): + source = mock.sentinel.source + dest = mock.sentinel.dest + out_format = mock.sentinel.out_format + mock_info.return_value.virtual_size = 1048576 + + output = image_utils.convert_image(source, dest, out_format, + disable_sparse=True) + + self.assertIsNone(output) + mock_exec.assert_called_once_with('qemu-img', 'convert', + '-O', out_format, '-S', '0', source, + dest, run_as_root=True) + @mock.patch('cinder.volume.volume_utils.check_for_odirect_support', return_value=True) @mock.patch('cinder.image.image_utils.qemu_img_info') @@ -1013,7 +1031,8 @@ class TestFetchToVhd(test.TestCase): volume_subformat=out_subformat, user_id=None, project_id=None, - run_as_root=True) + run_as_root=True, + disable_sparse=False) @mock.patch('cinder.image.image_utils.check_available_space') @mock.patch('cinder.image.image_utils.fetch_to_volume_format') @@ -1039,7 +1058,8 @@ class TestFetchToVhd(test.TestCase): volume_subformat=out_subformat, user_id=user_id, project_id=project_id, - run_as_root=run_as_root) + run_as_root=run_as_root, + disable_sparse=False) class TestFetchToRaw(test.TestCase): @@ -1057,7 +1077,8 @@ class TestFetchToRaw(test.TestCase): mock_fetch_to.assert_called_once_with(ctxt, image_service, image_id, dest, 'raw', blocksize, user_id=None, project_id=None, - size=None, run_as_root=True) + size=None, run_as_root=True, + disable_sparse=False) @mock.patch('cinder.image.image_utils.check_available_space') @mock.patch('cinder.image.image_utils.fetch_to_volume_format') @@ -1081,7 +1102,8 @@ class TestFetchToRaw(test.TestCase): dest, 'raw', blocksize, user_id=user_id, size=size, project_id=project_id, - run_as_root=run_as_root) + run_as_root=run_as_root, + disable_sparse=False) class FakeImageService(object): @@ -1147,7 +1169,8 @@ class TestFetchToVolumeFormat(test.TestCase): run_as_root=True, src_format=disk_format, image_id=image_id, - data=data) + data=data, + disable_sparse=False) @mock.patch('cinder.image.image_utils.check_virtual_size') @mock.patch('cinder.image.image_utils.check_available_space') @@ -1203,7 +1226,8 @@ class TestFetchToVolumeFormat(test.TestCase): run_as_root=run_as_root, src_format=qemu_img_format, image_id=image_id, - data=data) + data=data, + disable_sparse=False) mock_check_size.assert_called_once_with(data.virtual_size, size, image_id) @@ -1289,7 +1313,8 @@ class TestFetchToVolumeFormat(test.TestCase): run_as_root=run_as_root, src_format=expect_format, image_id=image_id, - data=data) + data=data, + disable_sparse=False) @mock.patch('cinder.image.image_utils.check_virtual_size') @mock.patch('cinder.image.image_utils.check_available_space') @@ -1344,7 +1369,8 @@ class TestFetchToVolumeFormat(test.TestCase): run_as_root=run_as_root, src_format=expect_format, image_id=image_id, - data=data) + data=data, + disable_sparse=False) @mock.patch('cinder.image.image_utils.check_available_space', new=mock.Mock()) @@ -1385,7 +1411,8 @@ class TestFetchToVolumeFormat(test.TestCase): output = image_utils.fetch_to_volume_format(ctxt, image_service, image_id, dest, volume_format, - blocksize) + blocksize, + disable_sparse=False) self.assertIsNone(output) self.assertEqual(2, mock_temp.call_count) @@ -1402,7 +1429,8 @@ class TestFetchToVolumeFormat(test.TestCase): run_as_root=True, src_format=qemu_img_format, image_id=image_id, - data=data) + data=data, + disable_sparse=False) @mock.patch('cinder.image.image_utils.convert_image') @mock.patch('cinder.image.image_utils.volume_utils.copy_volume') @@ -1702,7 +1730,8 @@ class TestFetchToVolumeFormat(test.TestCase): run_as_root=run_as_root, src_format=qemu_img_format, image_id=image_id, - data=data) + data=data, + disable_sparse=False) @mock.patch('cinder.image.image_utils.fetch') @mock.patch('cinder.image.image_utils.qemu_img_info', @@ -1858,7 +1887,8 @@ class TestFetchToVolumeFormat(test.TestCase): run_as_root=True, src_format=qemu_img_format, image_id=image_id, - data=data) + data=data, + disable_sparse=False) mock_engine.decompress_img.assert_called() diff --git a/cinder/tests/unit/test_volume_utils.py b/cinder/tests/unit/test_volume_utils.py index c7bd295fd4b..a78cfee732a 100644 --- a/cinder/tests/unit/test_volume_utils.py +++ b/cinder/tests/unit/test_volume_utils.py @@ -1213,10 +1213,12 @@ class VolumeUtilsTestCase(test.TestCase): fake_image_service) if is_encrypted: fake_driver.copy_image_to_encrypted_volume.assert_called_once_with( - ctxt, volume, fake_image_service, image_id) + ctxt, volume, fake_image_service, image_id, + disable_sparse=False) else: fake_driver.copy_image_to_volume.assert_called_once_with( - ctxt, volume, fake_image_service, image_id) + ctxt, volume, fake_image_service, image_id, + disable_sparse=False) @ddt.data({'cipher': 'aes-xts-plain64', 'provider': 'luks'}, diff --git a/cinder/tests/unit/volume/__init__.py b/cinder/tests/unit/volume/__init__.py index b65234ed38f..40d544fa3d3 100644 --- a/cinder/tests/unit/volume/__init__.py +++ b/cinder/tests/unit/volume/__init__.py @@ -117,7 +117,7 @@ class BaseVolumeTestCase(test.TestCase): pass def fake_fetch_to_raw(ctx, image_service, image_id, path, blocksize, - size=None, throttle=None): + size=None, throttle=None, disable_sparse=False): pass def fake_clone_image(ctx, volume_ref, diff --git a/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_mirror_fc.py b/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_mirror_fc.py index e65c2fde7c8..3b3249f8a05 100644 --- a/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_mirror_fc.py +++ b/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_mirror_fc.py @@ -1243,7 +1243,8 @@ class HBSDMIRRORFCDriverTest(test.TestCase): self.driver.copy_image_to_volume( self.ctxt, TEST_VOLUME[0], image_service, image_id) mock_copy_image.assert_called_with( - self.ctxt, TEST_VOLUME[0], image_service, image_id) + self.ctxt, TEST_VOLUME[0], image_service, image_id, + disable_sparse=False) self.assertEqual(2, request.call_count) @mock.patch.object(requests.Session, "request") diff --git a/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_rest_fc.py b/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_rest_fc.py index 63e48b5bdd0..f0f635c052f 100644 --- a/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_rest_fc.py +++ b/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_rest_fc.py @@ -1253,7 +1253,8 @@ class HBSDRESTFCDriverTest(test.TestCase): self.driver.copy_image_to_volume( self.ctxt, TEST_VOLUME[0], image_service, image_id) mock_copy_image.assert_called_with( - self.ctxt, TEST_VOLUME[0], image_service, image_id) + self.ctxt, TEST_VOLUME[0], image_service, image_id, + disable_sparse=False) self.assertEqual(1, request.call_count) @mock.patch.object(requests.Session, "request") diff --git a/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_rest_iscsi.py b/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_rest_iscsi.py index 9d79a1031e0..004ad5d3ef9 100644 --- a/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_rest_iscsi.py +++ b/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_rest_iscsi.py @@ -856,7 +856,8 @@ class HBSDRESTISCSIDriverTest(test.TestCase): self.driver.copy_image_to_volume( self.ctxt, TEST_VOLUME[0], image_service, image_id) mock_copy_image.assert_called_with( - self.ctxt, TEST_VOLUME[0], image_service, image_id) + self.ctxt, TEST_VOLUME[0], image_service, image_id, + disable_sparse=False) self.assertEqual(1, request.call_count) @mock.patch.object(requests.Session, "request") diff --git a/cinder/tests/unit/volume/drivers/hpe/xp/test_hpe_xp_rest_fc.py b/cinder/tests/unit/volume/drivers/hpe/xp/test_hpe_xp_rest_fc.py index 1499964c3aa..1ba2cae68fe 100644 --- a/cinder/tests/unit/volume/drivers/hpe/xp/test_hpe_xp_rest_fc.py +++ b/cinder/tests/unit/volume/drivers/hpe/xp/test_hpe_xp_rest_fc.py @@ -938,7 +938,8 @@ class HPEXPRESTFCDriverTest(test.TestCase): self.driver.copy_image_to_volume( self.ctxt, TEST_VOLUME[0], image_service, image_id) mock_copy_image.assert_called_with( - self.ctxt, TEST_VOLUME[0], image_service, image_id) + self.ctxt, TEST_VOLUME[0], image_service, image_id, + disable_sparse=False) self.assertEqual(1, request.call_count) @mock.patch.object(requests.Session, "request") diff --git a/cinder/tests/unit/volume/drivers/hpe/xp/test_hpe_xp_rest_iscsi.py b/cinder/tests/unit/volume/drivers/hpe/xp/test_hpe_xp_rest_iscsi.py index 18e933d8085..f6f4f84e755 100644 --- a/cinder/tests/unit/volume/drivers/hpe/xp/test_hpe_xp_rest_iscsi.py +++ b/cinder/tests/unit/volume/drivers/hpe/xp/test_hpe_xp_rest_iscsi.py @@ -751,7 +751,8 @@ class HPEXPRESTISCSIDriverTest(test.TestCase): self.driver.copy_image_to_volume( self.ctxt, TEST_VOLUME[0], image_service, image_id) mock_copy_image.assert_called_with( - self.ctxt, TEST_VOLUME[0], image_service, image_id) + self.ctxt, TEST_VOLUME[0], image_service, image_id, + disable_sparse=False) self.assertEqual(1, request.call_count) @mock.patch.object(requests.Session, "request") diff --git a/cinder/tests/unit/volume/drivers/nec/v/test_internal_nec_rest_fc.py b/cinder/tests/unit/volume/drivers/nec/v/test_internal_nec_rest_fc.py index 43b3bfb9250..23aef4da74d 100644 --- a/cinder/tests/unit/volume/drivers/nec/v/test_internal_nec_rest_fc.py +++ b/cinder/tests/unit/volume/drivers/nec/v/test_internal_nec_rest_fc.py @@ -932,7 +932,8 @@ class VStorageRESTFCDriverTest(test.TestCase): self.driver.copy_image_to_volume( self.ctxt, TEST_VOLUME[0], image_service, image_id) mock_copy_image.assert_called_with( - self.ctxt, TEST_VOLUME[0], image_service, image_id) + self.ctxt, TEST_VOLUME[0], image_service, image_id, + disable_sparse=False) self.assertEqual(1, request.call_count) @mock.patch.object(requests.Session, "request") diff --git a/cinder/tests/unit/volume/drivers/nec/v/test_internal_nec_rest_iscsi.py b/cinder/tests/unit/volume/drivers/nec/v/test_internal_nec_rest_iscsi.py index b5baa149a6d..2c76e2a5172 100644 --- a/cinder/tests/unit/volume/drivers/nec/v/test_internal_nec_rest_iscsi.py +++ b/cinder/tests/unit/volume/drivers/nec/v/test_internal_nec_rest_iscsi.py @@ -790,7 +790,8 @@ class VStorageRESTISCSIDriverTest(test.TestCase): self.driver.copy_image_to_volume( self.ctxt, TEST_VOLUME[0], image_service, image_id) mock_copy_image.assert_called_with( - self.ctxt, TEST_VOLUME[0], image_service, image_id) + self.ctxt, TEST_VOLUME[0], image_service, image_id, + disable_sparse=False) self.assertEqual(1, request.call_count) @mock.patch.object(requests.Session, "request") 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 9c6e686cd91..ec8e2dd8712 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 @@ -572,7 +572,7 @@ class NetAppNfsDriverTestCase(test.TestCase): mock_copy_image.assert_called_once_with( 'fake_context', fake.NFS_VOLUME, 'fake_img_service', - fake.IMAGE_FILE_ID) + fake.IMAGE_FILE_ID, disable_sparse=False) self.assertEqual(1, mock_log.info.call_count) mock_register_image.assert_called_once_with( fake.NFS_VOLUME, fake.IMAGE_FILE_ID) diff --git a/cinder/tests/unit/volume/drivers/test_nfs.py b/cinder/tests/unit/volume/drivers/test_nfs.py index b1fa90cd124..ba0a6ea81f3 100644 --- a/cinder/tests/unit/volume/drivers/test_nfs.py +++ b/cinder/tests/unit/volume/drivers/test_nfs.py @@ -537,7 +537,7 @@ class NfsDriverTestCase(test.TestCase): mock_fetch.assert_called_once_with( None, None, None, test_img_source, mock.ANY, run_as_root=True, - size=self.TEST_SIZE_IN_GB) + size=self.TEST_SIZE_IN_GB, disable_sparse=False) mock_resize.assert_called_once_with(test_img_source, self.TEST_SIZE_IN_GB, run_as_root=True) diff --git a/cinder/tests/unit/volume/flows/test_create_volume_flow.py b/cinder/tests/unit/volume/flows/test_create_volume_flow.py index c9a3b81f027..b318c50e74a 100644 --- a/cinder/tests/unit/volume/flows/test_create_volume_flow.py +++ b/cinder/tests/unit/volume/flows/test_create_volume_flow.py @@ -1170,7 +1170,8 @@ class CreateVolumeFlowManagerTestCase(test.TestCase): fake_driver.create_volume.assert_called_once_with(volume) fake_driver.copy_image_to_encrypted_volume.assert_called_once_with( - self.ctxt, volume, fake_image_service, image_id) + self.ctxt, volume, fake_image_service, image_id, + disable_sparse=False) mock_prepare_image_cache.assert_not_called() mock_handle_bootable.assert_called_once_with(self.ctxt, volume, image_id=image_id, @@ -1220,7 +1221,8 @@ class CreateVolumeFlowManagerTestCase(test.TestCase): fake_driver.create_volume.assert_called_once_with(volume) fake_driver.copy_image_to_encrypted_volume.assert_not_called() fake_driver.copy_image_to_volume.assert_called_once_with( - self.ctxt, volume, fake_image_service, image_id) + self.ctxt, volume, fake_image_service, image_id, + disable_sparse=False) mock_handle_bootable.assert_called_once_with(self.ctxt, volume, image_id=image_id, image_meta=image_meta) diff --git a/cinder/tests/unit/volume/test_driver.py b/cinder/tests/unit/volume/test_driver.py index 388073b19e3..0285bdf31e0 100644 --- a/cinder/tests/unit/volume/test_driver.py +++ b/cinder/tests/unit/volume/test_driver.py @@ -465,7 +465,7 @@ class GenericVolumeDriverTestCase(BaseDriverTestCase): self.context, attach_info, encryption) mock_fetch_to_raw.assert_called_once_with( self.context, image_service, fake.IMAGE_ID, - local_path, '1M', size=2) + local_path, '1M', size=2, disable_sparse=False) mock_detach_encryptor.assert_called_once_with( attach_info, encryption) mock_detach_volume.assert_called_once_with( @@ -567,7 +567,8 @@ class GenericVolumeDriverTestCase(BaseDriverTestCase): self.context, attach_info, encryption) mock_fetch_to_raw.assert_called_once_with( self.context, image_service, fake.IMAGE_ID, - local_path, '1M', size=2) + local_path, '1M', size=2, + disable_sparse=False) mock_detach_encryptor.assert_called_once_with( attach_info, encryption) mock_detach_volume.assert_called_once_with( diff --git a/cinder/tests/unit/volume/test_image.py b/cinder/tests/unit/volume/test_image.py index 2b1b9c3923f..979a0e3be2c 100644 --- a/cinder/tests/unit/volume/test_image.py +++ b/cinder/tests/unit/volume/test_image.py @@ -644,7 +644,7 @@ class ImageVolumeTestCases(base.BaseVolumeTestCase): mock_qemu_info.return_value = image_info def fake_copy_image_to_volume(context, volume, image_service, - image_id): + image_id, disable_sparse=False): raise exception.ImageTooBig(image_id=image_id, reason='') self.mock_object(self.volume.driver, 'copy_image_to_volume', diff --git a/cinder/tests/unit/volume/test_volume_reimage.py b/cinder/tests/unit/volume/test_volume_reimage.py index e03c59bd612..8b99fa8fba6 100644 --- a/cinder/tests/unit/volume/test_volume_reimage.py +++ b/cinder/tests/unit/volume/test_volume_reimage.py @@ -46,7 +46,8 @@ class VolumeReimageTestCase(base.BaseVolumeTestCase): self.volume.reimage(self.context, volume, self.image_meta) mock_cp_img.assert_called_once_with(self.context, volume, fake_image.FakeImageService(), - self.image_meta['id']) + self.image_meta['id'], + disable_sparse=True) self.assertEqual(volume.status, 'available') def test_volume_reimage_raise_exception(self): diff --git a/cinder/tests/unit/windows/test_iscsi.py b/cinder/tests/unit/windows/test_iscsi.py index 166def263d6..8f4e764902e 100644 --- a/cinder/tests/unit/windows/test_iscsi.py +++ b/cinder/tests/unit/windows/test_iscsi.py @@ -339,7 +339,8 @@ class TestWindowsISCSIDriver(test.TestCase): image_utils.fetch_to_vhd.assert_called_once_with( mock.sentinel.context, mock.sentinel.image_service, mock.sentinel.image_id, mock.sentinel.tmp_vhd_path, - self._driver.configuration.volume_dd_blocksize) + self._driver.configuration.volume_dd_blocksize, + disable_sparse=False) mock_unlink.assert_called_once_with(mock.sentinel.vol_vhd_path) self._driver._vhdutils.convert_vhd.assert_called_once_with( diff --git a/cinder/tests/unit/windows/test_smbfs.py b/cinder/tests/unit/windows/test_smbfs.py index 5a76b7ecaac..ead798c9a4e 100644 --- a/cinder/tests/unit/windows/test_smbfs.py +++ b/cinder/tests/unit/windows/test_smbfs.py @@ -816,7 +816,8 @@ class WindowsSmbFsTestCase(test.TestCase): mock.sentinel.image_id, self._FAKE_VOLUME_PATH, mock.sentinel.volume_format, mock.sentinel.block_size, - mock_get_vhd_type.return_value) + mock_get_vhd_type.return_value, + disable_sparse=False) drv._vhdutils.resize_vhd.assert_called_once_with( self._FAKE_VOLUME_PATH, self.volume.size * units.Gi, diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index f623a39a0e0..84404c028f0 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -867,25 +867,30 @@ class BaseVD(object, metaclass=abc.ABCMeta): data["pools"].append(single_pool) self._stats = data - def copy_image_to_volume(self, context, volume, image_service, image_id): + def copy_image_to_volume(self, context, volume, image_service, image_id, + disable_sparse=False): """Fetch image from image_service and write to unencrypted volume. This does not attach an encryptor layer when connecting to the volume. """ self._copy_image_data_to_volume( - context, volume, image_service, image_id, encrypted=False) + context, volume, image_service, image_id, encrypted=False, + disable_sparse=disable_sparse) def copy_image_to_encrypted_volume( - self, context, volume, image_service, image_id): + self, context, volume, image_service, image_id, + disable_sparse=False): """Fetch image from image_service and write to encrypted volume. This attaches the encryptor layer when connecting to the volume. """ self._copy_image_data_to_volume( - context, volume, image_service, image_id, encrypted=True) + context, volume, image_service, image_id, encrypted=True, + disable_sparse=disable_sparse) def _copy_image_data_to_volume(self, context, volume, image_service, - image_id, encrypted=False): + image_id, encrypted=False, + disable_sparse=False): """Fetch the image from image_service and write it to the volume.""" LOG.debug('copy_image_to_volume %s.', volume['name']) @@ -909,7 +914,7 @@ class BaseVD(object, metaclass=abc.ABCMeta): image_id, attach_info['device']['path'], self.configuration.volume_dd_blocksize, - size=volume['size']) + size=volume['size'], disable_sparse=disable_sparse) except exception.ImageTooBig: with excutils.save_and_reraise_exception(): LOG.exception("Copying image %(image_id)s " diff --git a/cinder/volume/drivers/dell_emc/powerflex/driver.py b/cinder/volume/drivers/dell_emc/powerflex/driver.py index 206752f625b..dfde157fc19 100644 --- a/cinder/volume/drivers/dell_emc/powerflex/driver.py +++ b/cinder/volume/drivers/dell_emc/powerflex/driver.py @@ -1236,7 +1236,8 @@ class PowerFlexDriver(driver.VolumeDriver): self.connector.disconnect_volume(connection_properties, volume) - def copy_image_to_volume(self, context, volume, image_service, image_id): + def copy_image_to_volume(self, context, volume, image_service, image_id, + disable_sparse=False): """Fetch image from image service and write it to volume.""" LOG.info("Copy image %(image_id)s from image service %(service)s " @@ -1252,7 +1253,8 @@ class PowerFlexDriver(driver.VolumeDriver): image_id, self._sio_attach_volume(volume), BLOCK_SIZE, - size=volume.size) + size=volume.size, + disable_sparse=disable_sparse) finally: self._sio_detach_volume(volume) diff --git a/cinder/volume/drivers/fungible/driver.py b/cinder/volume/drivers/fungible/driver.py index 0843bafa6b6..d41837ff7e4 100644 --- a/cinder/volume/drivers/fungible/driver.py +++ b/cinder/volume/drivers/fungible/driver.py @@ -1016,7 +1016,8 @@ class FungibleDriver(driver.BaseVD): ignore_errors=True ) - def copy_image_to_volume(self, context, volume, image_service, image_id): + 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.""" LOG.info( "Copy image %s from image service %s " @@ -1047,6 +1048,7 @@ class FungibleDriver(driver.BaseVD): attach_info["device"]["path"], self.configuration.volume_dd_blocksize, size=volume["size"], + disable_sparse=disable_sparse, ) LOG.debug( "Copy image %s to volume %s complete", diff --git a/cinder/volume/drivers/hitachi/hbsd_fc.py b/cinder/volume/drivers/hitachi/hbsd_fc.py index 4ebee7ae2bc..e4233ea1353 100644 --- a/cinder/volume/drivers/hitachi/hbsd_fc.py +++ b/cinder/volume/drivers/hitachi/hbsd_fc.py @@ -195,10 +195,12 @@ class HBSDFCDriver(driver.FibreChannelDriver): ctxt, volume, new_volume, original_volume_status) @volume_utils.trace - def copy_image_to_volume(self, context, volume, image_service, image_id): + 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.""" super(HBSDFCDriver, self).copy_image_to_volume( - context, volume, image_service, image_id) + context, volume, image_service, image_id, + disable_sparse=disable_sparse) self.common.discard_zero_page(volume) @volume_utils.trace diff --git a/cinder/volume/drivers/hitachi/hbsd_iscsi.py b/cinder/volume/drivers/hitachi/hbsd_iscsi.py index d06f665cd4c..95ac706c0f7 100644 --- a/cinder/volume/drivers/hitachi/hbsd_iscsi.py +++ b/cinder/volume/drivers/hitachi/hbsd_iscsi.py @@ -191,10 +191,12 @@ class HBSDISCSIDriver(driver.ISCSIDriver): ctxt, volume, new_volume, original_volume_status) @volume_utils.trace - def copy_image_to_volume(self, context, volume, image_service, image_id): + 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.""" super(HBSDISCSIDriver, self).copy_image_to_volume( - context, volume, image_service, image_id) + context, volume, image_service, image_id, + disable_sparse=disable_sparse) self.common.discard_zero_page(volume) @volume_utils.trace diff --git a/cinder/volume/drivers/ibm/gpfs.py b/cinder/volume/drivers/ibm/gpfs.py index 892e7ce384b..f8fd1cf4333 100644 --- a/cinder/volume/drivers/ibm/gpfs.py +++ b/cinder/volume/drivers/ibm/gpfs.py @@ -950,7 +950,8 @@ class GPFSDriver(driver.CloneableImageVD, return {'provider_location': None}, True - def copy_image_to_volume(self, context, volume, image_service, image_id): + 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. Note that cinder.volume.flows.create_volume will attempt to use @@ -967,7 +968,8 @@ class GPFSDriver(driver.CloneableImageVD, image_utils.fetch_to_raw(context, image_service, image_id, self.local_path(volume), self.configuration.volume_dd_blocksize, - size=volume['size']) + size=volume['size'], + disable_sparse=disable_sparse) self._resize_volume_file(volume, volume['size']) def _resize_volume_file(self, volume, new_size): diff --git a/cinder/volume/drivers/linstordrv.py b/cinder/volume/drivers/linstordrv.py index c621e275456..fa0c17d2c4d 100644 --- a/cinder/volume/drivers/linstordrv.py +++ b/cinder/volume/drivers/linstordrv.py @@ -951,7 +951,8 @@ class LinstorBaseDriver(driver.VolumeDriver): self.delete_snapshot(snapshot) - def copy_image_to_volume(self, context, volume, image_service, image_id): + def copy_image_to_volume(self, context, volume, image_service, image_id, + disable_sparse=False): # self.create_volume(volume) already called by Cinder, and works full_rsc_name = self._drbd_resource_name_from_cinder_volume(volume) @@ -961,7 +962,8 @@ class LinstorBaseDriver(driver.VolumeDriver): image_id, str(self._get_rsc_path(full_rsc_name)), self.default_blocksize, - size=volume['size']) + size=volume['size'], + disable_sparse=disable_sparse) return {} def copy_volume_to_image(self, context, volume, image_service, image_meta): diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index ffa55a1d96f..0b7ecad45b9 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -526,14 +526,16 @@ class LVMVolumeDriver(driver.VolumeDriver): escaped_name = self._escape_snapshot(volume['name']).replace('-', '--') return "/dev/mapper/%s-%s" % (escaped_group, escaped_name) - def copy_image_to_volume(self, context, volume, image_service, image_id): + 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.""" image_utils.fetch_to_raw(context, image_service, image_id, self.local_path(volume), self.configuration.volume_dd_blocksize, - size=volume['size']) + size=volume['size'], + disable_sparse=disable_sparse) def copy_volume_to_image(self, context, volume, image_service, image_meta): """Copy the volume to the specified image.""" diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_base.py b/cinder/volume/drivers/netapp/dataontap/nfs_base.py index d5fec6f0ac3..0c5960aba4d 100644 --- a/cinder/volume/drivers/netapp/dataontap/nfs_base.py +++ b/cinder/volume/drivers/netapp/dataontap/nfs_base.py @@ -496,11 +496,13 @@ class NetAppNfsDriver(driver.ManageableVD, """Get the default goodness_function string.""" return self.DEFAULT_GOODNESS_FUNCTION - def copy_image_to_volume(self, context, volume, image_service, image_id): + 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) + context, volume, image_service, image_id, + disable_sparse=disable_sparse) LOG.info('Copied image to volume %s using regular download.', volume['id']) diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py index e710fd356ec..1f4dac8d9fd 100644 --- a/cinder/volume/drivers/rbd.py +++ b/cinder/volume/drivers/rbd.py @@ -1902,16 +1902,20 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, context: context.RequestContext, volume: Volume, image_service, - image_id: str) -> None: + image_id: str, + disable_sparse=False) -> None: self._copy_image_to_volume(context, volume, image_service, image_id, - encrypted=True) + encrypted=True, + disable_sparse=disable_sparse) def copy_image_to_volume(self, context: context.RequestContext, volume: Volume, image_service, - image_id: str) -> None: - self._copy_image_to_volume(context, volume, image_service, image_id) + image_id: str, + disable_sparse: bool = False) -> None: + self._copy_image_to_volume(context, volume, image_service, image_id, + disable_sparse=disable_sparse) def _encrypt_image(self, context: context.RequestContext, @@ -1956,7 +1960,8 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, volume: Volume, image_service: Any, image_id: str, - encrypted: bool = False) -> None: + encrypted: bool = False, + disable_sparse: bool = False) -> None: tmp_dir = volume_utils.image_conversion_dir() @@ -1964,7 +1969,8 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, image_utils.fetch_to_raw(context, image_service, image_id, tmp.name, self.configuration.volume_dd_blocksize, - size=volume.size) + size=volume.size, + disable_sparse=disable_sparse) if encrypted: self._encrypt_image(context, volume, tmp_dir, tmp.name) diff --git a/cinder/volume/drivers/remotefs.py b/cinder/volume/drivers/remotefs.py index d026d40b6dc..cb0333d9dc7 100644 --- a/cinder/volume/drivers/remotefs.py +++ b/cinder/volume/drivers/remotefs.py @@ -524,7 +524,8 @@ class RemoteFSDriver(driver.BaseVD): context: context.RequestContext, volume: objects.Volume, image_service, - image_id: str) -> None: + image_id: str, + disable_sparse: bool = False) -> None: """Fetch the image from image_service and write it to the volume.""" image_utils.fetch_to_raw(context, @@ -533,7 +534,8 @@ class RemoteFSDriver(driver.BaseVD): self.local_path(volume), self.configuration.volume_dd_blocksize, size=volume.size, - run_as_root=self._execute_as_root) + run_as_root=self._execute_as_root, + disable_sparse=disable_sparse) # NOTE (leseb): Set the virtual size of the image # the raw conversion overwrote the destination file diff --git a/cinder/volume/drivers/spdk.py b/cinder/volume/drivers/spdk.py index 34c7b1fe676..255363b8fba 100644 --- a/cinder/volume/drivers/spdk.py +++ b/cinder/volume/drivers/spdk.py @@ -326,7 +326,8 @@ class SPDKDriver(driver.VolumeDriver): if volume.size > src_volume.size: self.extend_volume(volume, volume.size) - def copy_image_to_volume(self, context, volume, image_service, image_id): + 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.""" volume['provider_location'] = ( @@ -350,7 +351,8 @@ class SPDKDriver(driver.VolumeDriver): image_id, device_info['path'], self.configuration.volume_dd_blocksize, - size=volume['size']) + size=volume['size'], + disable_sparse=disable_sparse) finally: target_connector.disconnect_volume(connection_data, volume) diff --git a/cinder/volume/drivers/storpool.py b/cinder/volume/drivers/storpool.py index 47685cb3f51..0fec1d490d4 100644 --- a/cinder/volume/drivers/storpool.py +++ b/cinder/volume/drivers/storpool.py @@ -409,7 +409,8 @@ class StorPoolDriver(driver.VolumeDriver): '%(vol)s: %(err)s', {'name': name, 'vol': volname, 'err': e}) - def copy_image_to_volume(self, context, volume, image_service, image_id): + def copy_image_to_volume(self, context, volume, image_service, image_id, + disable_sparse=False): req_id = context.request_id name = self._attach.volumeName(volume['id']) self._attach.add(req_id, { @@ -420,7 +421,8 @@ class StorPoolDriver(driver.VolumeDriver): }) try: return super(StorPoolDriver, self).copy_image_to_volume( - context, volume, image_service, image_id) + context, volume, image_service, image_id, + disable_sparse=disable_sparse) finally: self._attach.remove(req_id) diff --git a/cinder/volume/drivers/vmware/fcd.py b/cinder/volume/drivers/vmware/fcd.py index ca8a06f482c..bc08d9c5034 100644 --- a/cinder/volume/drivers/vmware/fcd.py +++ b/cinder/volume/drivers/vmware/fcd.py @@ -181,13 +181,16 @@ class VMwareVStorageObjectDriver(vmdk.VMwareVcVmdkDriver): LOG.error(msg) raise exception.ImageUnacceptable(image_id=image_id, reason=msg) - def copy_image_to_volume(self, context, volume, image_service, image_id): + 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. :param context: Security/policy info for the request. :param volume: The volume to create. :param image_service: The image service to use. :param image_id: The image identifier. + :param disable_sparse: Enable or disable sparse copy. Default=False. + This parameter is ignored by VMware driver. :returns: Model updates. """ metadata = image_service.show(context, image_id) diff --git a/cinder/volume/drivers/vmware/vmdk.py b/cinder/volume/drivers/vmware/vmdk.py index 0da51163f29..154410e7e10 100644 --- a/cinder/volume/drivers/vmware/vmdk.py +++ b/cinder/volume/drivers/vmware/vmdk.py @@ -1441,7 +1441,8 @@ class VMwareVcVmdkDriver(driver.VolumeDriver): context, volume, image_service, image_meta['id']) return (ret, True) - def copy_image_to_volume(self, context, volume, image_service, image_id): + def copy_image_to_volume(self, context, volume, image_service, image_id, + disable_sparse=False): """Creates volume from image. This method only supports Glance image of VMDK disk format. @@ -1453,6 +1454,8 @@ class VMwareVcVmdkDriver(driver.VolumeDriver): :param volume: Volume object :param image_service: Glance image service :param image_id: Glance image id + :param disable_sparse: Enable or disable sparse copy. Default=False. + This parameter is ignored by VMDK driver. """ LOG.debug("Copy glance image: %s to create new volume.", image_id) diff --git a/cinder/volume/drivers/vzstorage.py b/cinder/volume/drivers/vzstorage.py index dad9b110025..3c18ea9dd95 100644 --- a/cinder/volume/drivers/vzstorage.py +++ b/cinder/volume/drivers/vzstorage.py @@ -473,7 +473,8 @@ class VZStorageDriver(remotefs_drv.RemoteFSSnapDriver): self._execute('ploop', 'restore-descriptor', image_dir, image_file) - def copy_image_to_volume(self, context, volume, image_service, image_id): + 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.""" volume_format = self.get_volume_format(volume) qemu_volume_format = image_utils.fixup_disk_format(volume_format) @@ -484,7 +485,8 @@ class VZStorageDriver(remotefs_drv.RemoteFSSnapDriver): image_utils.fetch_to_volume_format( context, image_service, image_id, image_path, qemu_volume_format, - self.configuration.volume_dd_blocksize) + self.configuration.volume_dd_blocksize, + disable_sparse=disable_sparse) if volume_format == DISK_FORMAT_PLOOP: self._recreate_ploop_desc(self.local_path(volume), image_path) diff --git a/cinder/volume/drivers/windows/iscsi.py b/cinder/volume/drivers/windows/iscsi.py index 9ca2b360889..bba763746e5 100644 --- a/cinder/volume/drivers/windows/iscsi.py +++ b/cinder/volume/drivers/windows/iscsi.py @@ -253,14 +253,16 @@ class WindowsISCSIDriver(driver.ISCSIDriver): target_name = self._get_target_name(volume) self._tgt_utils.delete_iscsi_target(target_name) - def copy_image_to_volume(self, context, volume, image_service, image_id): + def copy_image_to_volume(self, context, volume, image_service, image_id, + disable_sparse=False): """Fetch the image from image_service and create a volume using it.""" # Convert to VHD and file back to VHD vhd_type = self._tgt_utils.get_supported_vhd_type() with image_utils.temporary_file(suffix='.vhd') as tmp: volume_path = self.local_path(volume) image_utils.fetch_to_vhd(context, image_service, image_id, tmp, - self.configuration.volume_dd_blocksize) + self.configuration.volume_dd_blocksize, + disable_sparse=disable_sparse) # The vhd must be disabled and deleted before being replaced with # the desired image. self._tgt_utils.change_wt_disk_status(volume.name, diff --git a/cinder/volume/drivers/windows/smbfs.py b/cinder/volume/drivers/windows/smbfs.py index 3ee552813b2..0846964bd0a 100644 --- a/cinder/volume/drivers/windows/smbfs.py +++ b/cinder/volume/drivers/windows/smbfs.py @@ -582,7 +582,8 @@ class WindowsSmbfsDriver(remotefs_drv.RevertToSnapshotMixin, if temp_path: self._delete(temp_path) - def copy_image_to_volume(self, context, volume, image_service, image_id): + 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.""" volume_path = self.local_path(volume) volume_format = self.get_volume_format(volume, qemu_format=True) @@ -593,7 +594,8 @@ class WindowsSmbfsDriver(remotefs_drv.RevertToSnapshotMixin, context, image_service, image_id, volume_path, volume_format, self.configuration.volume_dd_blocksize, - volume_subformat) + volume_subformat, + disable_sparse=disable_sparse) volume_path = self.local_path(volume) self._vhdutils.set_vhd_guid(volume_path, volume.id) diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 104291563d0..0a9e20877c6 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -5350,7 +5350,8 @@ class VolumeManager(manager.CleanableManager, volume_utils.copy_image_to_volume(self.driver, context, volume, image_meta, image_location, - image_service) + image_service, + disable_sparse=True) self._refresh_volume_glance_meta(context, volume, image_meta) volume.status = volume.previous_status diff --git a/cinder/volume/volume_utils.py b/cinder/volume/volume_utils.py index 831b137b9a1..ba5b5a4210a 100644 --- a/cinder/volume/volume_utils.py +++ b/cinder/volume/volume_utils.py @@ -1184,7 +1184,8 @@ def copy_image_to_volume(driver, volume: 'objects.Volume', image_meta: dict, image_location: Union[str, tuple[Optional[str], Any]], - image_service) -> None: + image_service, + disable_sparse: bool = False) -> None: """Downloads Glance image to the specified volume.""" image_id = image_meta['id'] LOG.debug("Attempting download of %(image_id)s (%(image_location)s)" @@ -1199,15 +1200,18 @@ def copy_image_to_volume(driver, # already cloned it to the volume's key in # _get_encryption_key_id, so we can do a direct copy. driver.copy_image_to_volume( - context, volume, image_service, image_id) + context, volume, image_service, image_id, + disable_sparse=disable_sparse) elif volume.encryption_key_id: # Creating an encrypted volume from a normal, unencrypted, # image. driver.copy_image_to_encrypted_volume( - context, volume, image_service, image_id) + context, volume, image_service, image_id, + disable_sparse=disable_sparse) else: driver.copy_image_to_volume( - context, volume, image_service, image_id) + context, volume, image_service, image_id, + disable_sparse=disable_sparse) except processutils.ProcessExecutionError as ex: LOG.exception("Failed to copy image %(image_id)s to volume: " "%(volume_id)s", diff --git a/releasenotes/notes/fix-reimage-sparse-copy-d346e8f55afa6280.yaml b/releasenotes/notes/fix-reimage-sparse-copy-d346e8f55afa6280.yaml new file mode 100644 index 00000000000..49a18d93a36 --- /dev/null +++ b/releasenotes/notes/fix-reimage-sparse-copy-d346e8f55afa6280.yaml @@ -0,0 +1,12 @@ +--- +fixes: + - | + `Bug #2045431 `_: Fixed + a data leak scenario where we preserve sparseness when reimaging the + volume. + + We currently do a sparse copy when writing an image on the volume. This + could be a potential data leak scenario where the zero blocks of the new + image are not written on the existing volume and the data from the old + image still exists on the volume. We fix the scenario by not doing sparse + copy when reimaging the volume.