From 8d7e292bcd9ae610ac6341de2d00b8990252c24c Mon Sep 17 00:00:00 2001 From: whoami-rajat Date: Mon, 25 Sep 2023 16:17:49 +0000 Subject: [PATCH] Skip sparse copy during volume reimage When rebuilding a volume backed instance, while copying the new image to the existing volume, we preserve sparseness. This could be problematic since we don't write the zero blocks of the new image and the data in the old image can still persist leading to a data leak scenario. To prevent this, we are using `-S 0`[1][2] option with the `qemu-img convert` command to write all the zero bytes into the volume. In the testing done, this doesn't seem to be a problem with known 'raw' images but good to handle the case anyway. Following is the testing performed with 3 images: 1. CIRROS QCOW2 to RAW ====================== Volume size: 1 GiB Image size (raw): 112 MiB CREATE VOLUME FROM IMAGE (without -S 0) LVS (10.94% allocated) volume-91ea43ef-684c-402f-896e-63e45e5f4fff stack-volumes-lvmdriver-1 Vwi-a-tz-- 1.00g stack-volumes-lvmdriver-1-pool 10.94 REBUILD (with -S 0) LVS (10.94% allocated) volume-91ea43ef-684c-402f-896e-63e45e5f4fff stack-volumes-lvmdriver-1 Vwi-aotz-- 1.00g stack-volumes-lvmdriver-1-pool 10.94 Conclusion: Same space is consumed on the disk with and without preserving sparseness. 2. DEBIAN QCOW2 to RAW ====================== Volume size: 3 GiB Image size (raw): 2 GiB CREATE VOLUME FROM IMAGE (without -S 0) LVS (66.67% allocated) volume-edc42b6a-df5d-420e-85d3-b3e52bcb735e stack-volumes-lvmdriver-1 Vwi-a-tz-- 3.00g stack-volumes-lvmdriver-1-pool 66.67 REBUILD (with -S 0) LVS (66.67% allocated) volume-edc42b6a-df5d-420e-85d3-b3e52bcb735e stack-volumes-lvmdriver-1 Vwi-aotz-- 3.00g stack-volumes-lvmdriver-1-pool 66.67 Conclusion: Same space is consumed on the disk with and without preserving sparseness. 3. FEDORA QCOW2 TO RAW ====================== CREATE VOLUME FROM IMAGE (without -S 0) Volume size: 6 GiB Image size (raw): 5 GiB LVS (83.33% allocated) volume-efa1a227-a30d-4385-867a-db22a3e80ad7 stack-volumes-lvmdriver-1 Vwi-a-tz-- 6.00g stack-volumes-lvmdriver-1-pool 83.33 REBUILD (with -S 0) LVS (83.33% allocated) volume-efa1a227-a30d-4385-867a-db22a3e80ad7 stack-volumes-lvmdriver-1 Vwi-aotz-- 6.00g stack-volumes-lvmdriver-1-pool 83.33 Conclusion: Same space is consumed on the disk with and without preserving sparseness. Another testing was done to check if the `-S 0` option actually works in OpenStack setup. Note that we are converting qcow2 to qcow2 image which won't happen in a real world deployment and only for test purposes. DEBIAN QCOW2 TO QCOW2 ===================== CREATE VOLUME FROM IMAGE (without -S 0) LVS (52.61% allocated) volume-de581f84-e722-4f4a-94fb-10f767069f50 stack-volumes-lvmdriver-1 Vwi-a-tz-- 3.00g stack-volumes-lvmdriver-1-pool 52.61 REBUILD (with -S 0) LVS (66.68% allocated) volume-de581f84-e722-4f4a-94fb-10f767069f50 stack-volumes-lvmdriver-1 Vwi-aotz-- 3.00g stack-volumes-lvmdriver-1-pool 66.68 Conclusion: We can see that the space allocation increased hence we are not preserving sparseness when using the -S 0 option. [1] https://qemu-project.gitlab.io/qemu/tools/qemu-img.html#cmdoption-qemu-img-common-opts-S [2] https://github.com/qemu/qemu/blob/abf635ddfe3242df907f58967f3c1e6763bbca2d/qemu-img.c#L182-L186 Closes-Bug: #2045431 Change-Id: I5be7eaba68a5b8e1c43f0d95486b5c79c14e1b95 (cherry picked from commit 1a8ea0eac4f449c09c4da70302be1bacc29a9b79) (cherry picked from commit 85857a19ab7e3f6a331aa12615a874a4b4759c28) --- cinder/image/image_utils.py | 47 ++++++++++------ cinder/interface/volume_driver.py | 4 +- cinder/tests/unit/test_image_utils.py | 54 ++++++++++++++----- cinder/tests/unit/test_volume_utils.py | 6 ++- cinder/tests/unit/volume/__init__.py | 2 +- .../hitachi/test_hitachi_hbsd_mirror_fc.py | 3 +- .../hitachi/test_hitachi_hbsd_rest_fc.py | 3 +- .../hitachi/test_hitachi_hbsd_rest_iscsi.py | 3 +- .../drivers/hpe/xp/test_hpe_xp_rest_fc.py | 3 +- .../drivers/hpe/xp/test_hpe_xp_rest_iscsi.py | 3 +- .../nec/v/test_internal_nec_rest_fc.py | 3 +- .../nec/v/test_internal_nec_rest_iscsi.py | 3 +- .../drivers/netapp/dataontap/test_nfs_base.py | 2 +- cinder/tests/unit/volume/drivers/test_nfs.py | 2 +- .../volume/flows/test_create_volume_flow.py | 6 ++- cinder/tests/unit/volume/test_driver.py | 5 +- cinder/tests/unit/volume/test_image.py | 2 +- .../tests/unit/volume/test_volume_reimage.py | 3 +- cinder/tests/unit/windows/test_iscsi.py | 3 +- cinder/tests/unit/windows/test_smbfs.py | 3 +- cinder/volume/driver.py | 17 +++--- .../drivers/dell_emc/powerflex/driver.py | 6 ++- cinder/volume/drivers/fungible/driver.py | 4 +- cinder/volume/drivers/hitachi/hbsd_fc.py | 6 ++- cinder/volume/drivers/hitachi/hbsd_iscsi.py | 6 ++- cinder/volume/drivers/ibm/gpfs.py | 6 ++- cinder/volume/drivers/linstordrv.py | 6 ++- cinder/volume/drivers/lvm.py | 6 ++- .../drivers/netapp/dataontap/nfs_base.py | 6 ++- cinder/volume/drivers/rbd.py | 18 ++++--- cinder/volume/drivers/remotefs.py | 6 ++- cinder/volume/drivers/spdk.py | 6 ++- cinder/volume/drivers/storpool.py | 6 ++- cinder/volume/drivers/vmware/fcd.py | 5 +- cinder/volume/drivers/vmware/vmdk.py | 5 +- cinder/volume/drivers/vzstorage.py | 6 ++- cinder/volume/drivers/windows/iscsi.py | 6 ++- cinder/volume/drivers/windows/smbfs.py | 6 ++- cinder/volume/manager.py | 3 +- cinder/volume/volume_utils.py | 12 +++-- ...-reimage-sparse-copy-d346e8f55afa6280.yaml | 12 +++++ 41 files changed, 220 insertions(+), 94 deletions(-) create mode 100644 releasenotes/notes/fix-reimage-sparse-copy-d346e8f55afa6280.yaml 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.