openstack/cinder
Code Issues Proposed changes

Skip sparse copy during volume reimage

Browse Source 
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] abf635ddfe/qemu-img.c (L182-L186)

Closes-Bug: #2045431

Change-Id: I5be7eaba68a5b8e1c43f0d95486b5c79c14e1b95
(cherry picked from commit 1a8ea0eac4)
(cherry picked from commit 85857a19ab)
This commit is contained in:
whoami-rajat 2023-09-25 16:17:49 +00:00 committed by Rajat Dhasmana
parent e848c7cfc7
commit 8d7e292bcd
41 changed files with 220 additions and 94 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
cinder/image/image_utils.py
View File

@ -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,

4
cinder/interface/volume_driver.py
View File

@ -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.
"""

54
cinder/tests/unit/test_image_utils.py
View File

@ -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()

6
cinder/tests/unit/test_volume_utils.py
View File

@ -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'},

2
cinder/tests/unit/volume/__init__.py
View File

@ -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,

3
cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_mirror_fc.py
View File

@ -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")

3
cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_rest_fc.py
View File

@ -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")

3
cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_rest_iscsi.py
View File

@ -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")

3
cinder/tests/unit/volume/drivers/hpe/xp/test_hpe_xp_rest_fc.py
View File

@ -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")

3
cinder/tests/unit/volume/drivers/hpe/xp/test_hpe_xp_rest_iscsi.py
View File

@ -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")

3
cinder/tests/unit/volume/drivers/nec/v/test_internal_nec_rest_fc.py
View File

@ -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")

3
cinder/tests/unit/volume/drivers/nec/v/test_internal_nec_rest_iscsi.py
View File

@ -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")

2
cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py
View File

@ -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)

2
cinder/tests/unit/volume/drivers/test_nfs.py
View File

@ -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)

6
cinder/tests/unit/volume/flows/test_create_volume_flow.py
View File

@ -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)

5
cinder/tests/unit/volume/test_driver.py
View File

@ -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(

2
cinder/tests/unit/volume/test_image.py
View File

@ -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',

3
cinder/tests/unit/volume/test_volume_reimage.py
View File

@ -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):

3
cinder/tests/unit/windows/test_iscsi.py
View File

@ -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(

3
cinder/tests/unit/windows/test_smbfs.py
View File

@ -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,

17
cinder/volume/driver.py
View File

@ -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 "

6
cinder/volume/drivers/dell_emc/powerflex/driver.py
View File

@ -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)

4
cinder/volume/drivers/fungible/driver.py
View File

@ -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",

6
cinder/volume/drivers/hitachi/hbsd_fc.py
View File

@ -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

6
cinder/volume/drivers/hitachi/hbsd_iscsi.py
View File

@ -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

6
cinder/volume/drivers/ibm/gpfs.py
View File

@ -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):

6
cinder/volume/drivers/linstordrv.py
View File

@ -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):

6
cinder/volume/drivers/lvm.py
View File

@ -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."""

6
cinder/volume/drivers/netapp/dataontap/nfs_base.py
View File

@ -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'])

18
cinder/volume/drivers/rbd.py
View File

@ -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)

6
cinder/volume/drivers/remotefs.py
View File

@ -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

6
cinder/volume/drivers/spdk.py
View File

@ -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)

6
cinder/volume/drivers/storpool.py
View File

@ -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)

5
cinder/volume/drivers/vmware/fcd.py
View File

@ -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)

5
cinder/volume/drivers/vmware/vmdk.py
View File

@ -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)

6
cinder/volume/drivers/vzstorage.py
View File

@ -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)

6
cinder/volume/drivers/windows/iscsi.py
View File

@ -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,

6
cinder/volume/drivers/windows/smbfs.py
View File

@ -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)

3
cinder/volume/manager.py
View File

@ -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

12
cinder/volume/volume_utils.py
View File

@ -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",

12
releasenotes/notes/fix-reimage-sparse-copy-d346e8f55afa6280.yaml Normal file
View File

@ -0,0 +1,12 @@
---
fixes:
- |
`Bug #2045431 <https://bugs.launchpad.net/cinder/+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.