diff --git a/ironic/common/image_service.py b/ironic/common/image_service.py index cf1eefcb5c..15f0f2cd43 100644 --- a/ironic/common/image_service.py +++ b/ironic/common/image_service.py @@ -34,6 +34,10 @@ from ironic.common.i18n import _ from ironic.common import utils IMAGE_CHUNK_SIZE = 1024 * 1024 # 1mb +# NOTE(kaifeng) Image will be truncated to 2GiB by sendfile, +# we use a large chunk size here for a better performance +# while keep the chunk size less than the size limit. +SENDFILE_CHUNK_SIZE = 1024 * 1024 * 1024 # 1Gb LOG = log.getLogger(__name__) @@ -211,9 +215,17 @@ class FileImageService(BaseImageService): os.link(source_image_path, dest_image_path) else: filesize = os.path.getsize(source_image_path) + offset = 0 with open(source_image_path, 'rb') as input_img: - sendfile.sendfile(image_file.fileno(), input_img.fileno(), - 0, filesize) + while offset < filesize: + # TODO(kaifeng) Use os.sendfile() and remove sendfile + # dependency when python2 support is dropped. + count = min(SENDFILE_CHUNK_SIZE, filesize - offset) + nbytes_out = sendfile.sendfile(image_file.fileno(), + input_img.fileno(), + offset, + count) + offset += nbytes_out except Exception as e: raise exception.ImageDownloadFailed(image_href=image_href, reason=six.text_type(e)) diff --git a/ironic/tests/unit/common/test_image_service.py b/ironic/tests/unit/common/test_image_service.py index 91d46159df..2f15da595b 100644 --- a/ironic/tests/unit/common/test_image_service.py +++ b/ironic/tests/unit/common/test_image_service.py @@ -198,7 +198,7 @@ class FileImageServiceTestCase(base.TestCase): remove_mock.assert_called_once_with('file') link_mock.assert_called_once_with(self.href_path, 'file') - @mock.patch.object(sendfile, 'sendfile', autospec=True) + @mock.patch.object(sendfile, 'sendfile', return_value=42, autospec=True) @mock.patch.object(os.path, 'getsize', return_value=42, autospec=True) @mock.patch.object(__builtin__, 'open', autospec=True) @mock.patch.object(os, 'access', return_value=False, autospec=True) @@ -220,6 +220,38 @@ class FileImageServiceTestCase(base.TestCase): copy_mock.assert_called_once_with(file_mock.fileno(), input_mock.__enter__().fileno(), 0, 42) + + @mock.patch.object(sendfile, 'sendfile', autospec=True) + @mock.patch.object(os.path, 'getsize', return_value=42, autospec=True) + @mock.patch.object(__builtin__, 'open', autospec=True) + @mock.patch.object(os, 'access', return_value=False, autospec=True) + @mock.patch.object(os, 'stat', autospec=True) + @mock.patch.object(image_service.FileImageService, 'validate_href', + autospec=True) + def test_download_copy_segmented(self, _validate_mock, stat_mock, + access_mock, open_mock, size_mock, + copy_mock): + # Fake a 3G + 1k image + chunk_size = image_service.SENDFILE_CHUNK_SIZE + fake_image_size = chunk_size * 3 + 1024 + fake_chunk_seq = [chunk_size, chunk_size, chunk_size, 1024] + _validate_mock.return_value = self.href_path + stat_mock.return_value.st_dev = 'dev1' + file_mock = mock.MagicMock(spec=file) + file_mock.name = 'file' + input_mock = mock.MagicMock(spec=file) + open_mock.return_value = input_mock + size_mock.return_value = fake_image_size + copy_mock.side_effect = fake_chunk_seq + self.service.download(self.href, file_mock) + _validate_mock.assert_called_once_with(mock.ANY, self.href) + self.assertEqual(2, stat_mock.call_count) + access_mock.assert_called_once_with(self.href_path, os.R_OK | os.W_OK) + copy_calls = [mock.call(file_mock.fileno(), + input_mock.__enter__().fileno(), + chunk_size * i, + fake_chunk_seq[i]) for i in range(4)] + copy_mock.assert_has_calls(copy_calls) size_mock.assert_called_once_with(self.href_path) @mock.patch.object(os, 'remove', side_effect=OSError, autospec=True) diff --git a/releasenotes/notes/fix-sendfile-size-cap-d9966a96e2d7db51.yaml b/releasenotes/notes/fix-sendfile-size-cap-d9966a96e2d7db51.yaml new file mode 100644 index 0000000000..7ea999ded3 --- /dev/null +++ b/releasenotes/notes/fix-sendfile-size-cap-d9966a96e2d7db51.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes an issue when the image source is a local file, the image will be + truncated to 2G and fails deployment due to image corruption. +