diff --git a/etc/ironic/rootwrap.d/ironic-lib.filters b/etc/ironic/rootwrap.d/ironic-lib.filters index 0b3f43ec..b03a9f8b 100644 --- a/etc/ironic/rootwrap.d/ironic-lib.filters +++ b/etc/ironic/rootwrap.d/ironic-lib.filters @@ -12,7 +12,6 @@ blkid: CommandFilter, blkid, root blockdev: CommandFilter, blockdev, root hexdump: CommandFilter, hexdump, root lsblk: CommandFilter, lsblk, root -qemu-img: CommandFilter, qemu-img, root wipefs: CommandFilter, wipefs, root sgdisk: CommandFilter, sgdisk, root partprobe: CommandFilter, partprobe, root @@ -26,3 +25,6 @@ mount: CommandFilter, mount, root # ironic_lib/disk_partitioner.py fuser: CommandFilter, fuser, root parted: CommandFilter, parted, root + +# ironic_lib/qemu_img.py +qemu-img: CommandFilter, qemu-img, root diff --git a/ironic_lib/disk_utils.py b/ironic_lib/disk_utils.py index ecc35ee4..7765cf68 100644 --- a/ironic_lib/disk_utils.py +++ b/ironic_lib/disk_utils.py @@ -23,13 +23,11 @@ import warnings from oslo_concurrency import processutils from oslo_config import cfg from oslo_utils import excutils -from oslo_utils import imageutils -from oslo_utils import units -import tenacity from ironic_lib.common.i18n import _ from ironic_lib import disk_partitioner from ironic_lib import exception +from ironic_lib import qemu_img from ironic_lib import utils @@ -56,13 +54,6 @@ opts = [ default=10, help='Maximum number of attempts to try to read the ' 'partition.'), - cfg.IntOpt('image_convert_memory_limit', - default=2048, - help='Memory limit for "qemu-img convert" in MiB. Implemented ' - 'via the address space resource limit.'), - cfg.IntOpt('image_convert_attempts', - default=3, - help='Number of attempts to convert an image.'), ] CONF = cfg.CONF @@ -83,17 +74,9 @@ GPT_SIZE_SECTORS = 33 # Maximum disk size supported by MBR is 2TB (2 * 1024 * 1024 MB) MAX_DISK_SIZE_MB_SUPPORTED_BY_MBR = 2097152 -# Limit the memory address space to 1 GiB when running qemu-img -QEMU_IMG_LIMITS = None - - -def _qemu_img_limits(): - global QEMU_IMG_LIMITS - if QEMU_IMG_LIMITS is None: - QEMU_IMG_LIMITS = processutils.ProcessLimits( - address_space=CONF.disk_utils.image_convert_memory_limit - * units.Mi) - return QEMU_IMG_LIMITS +# Backward compatibility, do not use +qemu_img_info = qemu_img.image_info +convert_image = qemu_img.convert_image def list_partitions(device): @@ -485,73 +468,12 @@ def dd(src, dst, conv_flags=None): *extra_args) -def qemu_img_info(path): - """Return an object containing the parsed output from qemu-img info.""" - if not os.path.exists(path): - raise FileNotFoundError(_("File %s does not exist") % path) - - out, err = utils.execute('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path, - '--output=json', - prlimit=_qemu_img_limits()) - return imageutils.QemuImgInfo(out, format='json') - - -def _retry_on_res_temp_unavailable(exc): - if (isinstance(exc, processutils.ProcessExecutionError) - and ('Resource temporarily unavailable' in exc.stderr - or 'Cannot allocate memory' in exc.stderr)): - return True - return False - - -@tenacity.retry( - retry=tenacity.retry_if_exception(_retry_on_res_temp_unavailable), - stop=tenacity.stop_after_attempt(CONF.disk_utils.image_convert_attempts), - reraise=True) -def convert_image(source, dest, out_format, run_as_root=False, cache=None, - out_of_order=False, sparse_size=None): - """Convert image to other format.""" - cmd = ['qemu-img', 'convert', '-O', out_format] - if cache is not None: - cmd += ['-t', cache] - if sparse_size is not None: - cmd += ['-S', sparse_size] - if out_of_order: - cmd.append('-W') - cmd += [source, dest] - # NOTE(TheJulia): Statically set the MALLOC_ARENA_MAX to prevent leaking - # and the creation of new malloc arenas which will consume the system - # memory. If limited to 1, qemu-img consumes ~250 MB of RAM, but when - # another thread tries to access a locked section of memory in use with - # another thread, then by default a new malloc arena is created, - # which essentially balloons the memory requirement of the machine. - # Default for qemu-img is 8 * nCPU * ~250MB (based on defaults + - # thread/code/process/library overhead. In other words, 64 GB. Limiting - # this to 3 keeps the memory utilization in happy cases below the overall - # threshold which is in place in case a malicious image is attempted to - # be passed through qemu-img. - env_vars = {'MALLOC_ARENA_MAX': '3'} - try: - utils.execute(*cmd, run_as_root=run_as_root, - prlimit=_qemu_img_limits(), - use_standard_locale=True, - env_variables=env_vars) - except processutils.ProcessExecutionError as e: - if ('Resource temporarily unavailable' in e.stderr - or 'Cannot allocate memory' in e.stderr): - LOG.debug('Failed to convert image, retrying. Error: %s', e) - # Sync disk caches before the next attempt - utils.execute('sync') - raise - - def populate_image(src, dst, conv_flags=None): - data = qemu_img_info(src) + data = qemu_img.image_info(src) if data.file_format == 'raw': dd(src, dst, conv_flags=conv_flags) else: - convert_image(src, dst, 'raw', True, sparse_size='0') + qemu_img.convert_image(src, dst, 'raw', True, sparse_size='0') def block_uuid(dev): @@ -575,7 +497,7 @@ def get_image_mb(image_path, virtual_size=True): if not virtual_size: image_byte = os.path.getsize(image_path) else: - data = qemu_img_info(image_path) + data = qemu_img.image_info(image_path) image_byte = data.virtual_size # round up size to MB diff --git a/ironic_lib/qemu_img.py b/ironic_lib/qemu_img.py new file mode 100644 index 00000000..06707ece --- /dev/null +++ b/ironic_lib/qemu_img.py @@ -0,0 +1,117 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import logging +import os + +from oslo_concurrency import processutils +from oslo_config import cfg +from oslo_utils import imageutils +from oslo_utils import units +import tenacity + +from ironic_lib.common.i18n import _ +from ironic_lib import utils + + +opts = [ + cfg.IntOpt('image_convert_memory_limit', + default=2048, + help='Memory limit for "qemu-img convert" in MiB. Implemented ' + 'via the address space resource limit.'), + cfg.IntOpt('image_convert_attempts', + default=3, + help='Number of attempts to convert an image.'), +] + +LOG = logging.getLogger(__name__) + +CONF = cfg.CONF +CONF.register_opts(opts, group='disk_utils') + +# Limit the memory address space to 1 GiB when running qemu-img +QEMU_IMG_LIMITS = None + + +def _qemu_img_limits(): + global QEMU_IMG_LIMITS + if QEMU_IMG_LIMITS is None: + QEMU_IMG_LIMITS = processutils.ProcessLimits( + address_space=CONF.disk_utils.image_convert_memory_limit + * units.Mi) + return QEMU_IMG_LIMITS + + +def _retry_on_res_temp_unavailable(exc): + if (isinstance(exc, processutils.ProcessExecutionError) + and ('Resource temporarily unavailable' in exc.stderr + or 'Cannot allocate memory' in exc.stderr)): + return True + return False + + +def image_info(path): + """Return an object containing the parsed output from qemu-img info.""" + if not os.path.exists(path): + raise FileNotFoundError(_("File %s does not exist") % path) + + out, err = utils.execute('env', 'LC_ALL=C', 'LANG=C', + 'qemu-img', 'info', path, + '--output=json', + prlimit=_qemu_img_limits()) + return imageutils.QemuImgInfo(out, format='json') + + +@tenacity.retry( + retry=tenacity.retry_if_exception(_retry_on_res_temp_unavailable), + stop=tenacity.stop_after_attempt(CONF.disk_utils.image_convert_attempts), + reraise=True) +def convert_image(source, dest, out_format, run_as_root=False, cache=None, + out_of_order=False, sparse_size=None): + """Convert image to other format.""" + cmd = ['qemu-img', 'convert', '-O', out_format] + if cache is not None: + cmd += ['-t', cache] + if sparse_size is not None: + cmd += ['-S', sparse_size] + if out_of_order: + cmd.append('-W') + cmd += [source, dest] + # NOTE(TheJulia): Statically set the MALLOC_ARENA_MAX to prevent leaking + # and the creation of new malloc arenas which will consume the system + # memory. If limited to 1, qemu-img consumes ~250 MB of RAM, but when + # another thread tries to access a locked section of memory in use with + # another thread, then by default a new malloc arena is created, + # which essentially balloons the memory requirement of the machine. + # Default for qemu-img is 8 * nCPU * ~250MB (based on defaults + + # thread/code/process/library overhead. In other words, 64 GB. Limiting + # this to 3 keeps the memory utilization in happy cases below the overall + # threshold which is in place in case a malicious image is attempted to + # be passed through qemu-img. + env_vars = {'MALLOC_ARENA_MAX': '3'} + try: + utils.execute(*cmd, run_as_root=run_as_root, + prlimit=_qemu_img_limits(), + use_standard_locale=True, + env_variables=env_vars) + except processutils.ProcessExecutionError as e: + if ('Resource temporarily unavailable' in e.stderr + or 'Cannot allocate memory' in e.stderr): + LOG.debug('Failed to convert image, retrying. Error: %s', e) + # Sync disk caches before the next attempt + utils.execute('sync') + raise + + +def list_opts(): + """Entry point for oslo-config-generator.""" + return [('disk_utils', opts)] diff --git a/ironic_lib/tests/test_disk_utils.py b/ironic_lib/tests/test_disk_utils.py index dd6c5cd6..d5ccba69 100644 --- a/ironic_lib/tests/test_disk_utils.py +++ b/ironic_lib/tests/test_disk_utils.py @@ -19,10 +19,10 @@ from unittest import mock from oslo_concurrency import processutils from oslo_config import cfg -from oslo_utils import imageutils from ironic_lib import disk_utils from ironic_lib import exception +from ironic_lib import qemu_img from ironic_lib.tests import base from ironic_lib import utils @@ -545,8 +545,8 @@ class GetDeviceBlockSizeTestCase(base.IronicLibTestCase): @mock.patch.object(disk_utils, 'dd', autospec=True) -@mock.patch.object(disk_utils, 'qemu_img_info', autospec=True) -@mock.patch.object(disk_utils, 'convert_image', autospec=True) +@mock.patch.object(qemu_img, 'image_info', autospec=True) +@mock.patch.object(qemu_img, 'convert_image', autospec=True) class PopulateImageTestCase(base.IronicLibTestCase): def test_populate_raw_image(self, mock_cg, mock_qinfo, mock_dd): @@ -603,147 +603,8 @@ class OtherFunctionTestCase(base.IronicLibTestCase): disk_utils.is_block_device, device) mock_os.assert_has_calls([mock.call(device)] * 2) - @mock.patch.object(os.path, 'exists', return_value=False, autospec=True) - def test_qemu_img_info_path_doesnt_exist(self, path_exists_mock): - self.assertRaises(FileNotFoundError, disk_utils.qemu_img_info, 'noimg') - path_exists_mock.assert_called_once_with('noimg') - - @mock.patch.object(utils, 'execute', return_value=('out', 'err'), - autospec=True) - @mock.patch.object(imageutils, 'QemuImgInfo', autospec=True) - @mock.patch.object(os.path, 'exists', return_value=True, autospec=True) - def test_qemu_img_info_path_exists(self, path_exists_mock, - qemu_img_info_mock, execute_mock): - disk_utils.qemu_img_info('img') - path_exists_mock.assert_called_once_with('img') - execute_mock.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', 'img', - '--output=json', - prlimit=mock.ANY) - qemu_img_info_mock.assert_called_once_with('out', format='json') - - @mock.patch.object(utils, 'execute', autospec=True) - def test_convert_image(self, execute_mock): - disk_utils.convert_image('source', 'dest', 'out_format') - execute_mock.assert_called_once_with( - 'qemu-img', 'convert', '-O', - 'out_format', 'source', 'dest', - run_as_root=False, - prlimit=mock.ANY, - use_standard_locale=True, - env_variables={'MALLOC_ARENA_MAX': '3'}) - - @mock.patch.object(utils, 'execute', autospec=True) - def test_convert_image_flags(self, execute_mock): - disk_utils.convert_image('source', 'dest', 'out_format', - cache='directsync', out_of_order=True, - sparse_size='0') - execute_mock.assert_called_once_with( - 'qemu-img', 'convert', '-O', - 'out_format', '-t', 'directsync', - '-S', '0', '-W', 'source', 'dest', - run_as_root=False, - prlimit=mock.ANY, - use_standard_locale=True, - env_variables={'MALLOC_ARENA_MAX': '3'}) - - @mock.patch.object(utils, 'execute', autospec=True) - def test_convert_image_retries(self, execute_mock): - ret_err = 'qemu: qemu_thread_create: Resource temporarily unavailable' - execute_mock.side_effect = [ - processutils.ProcessExecutionError(stderr=ret_err), ('', ''), - processutils.ProcessExecutionError(stderr=ret_err), ('', ''), - ('', ''), - ] - - disk_utils.convert_image('source', 'dest', 'out_format') - convert_call = mock.call('qemu-img', 'convert', '-O', - 'out_format', 'source', 'dest', - run_as_root=False, - prlimit=mock.ANY, - use_standard_locale=True, - env_variables={'MALLOC_ARENA_MAX': '3'}) - execute_mock.assert_has_calls([ - convert_call, - mock.call('sync'), - convert_call, - mock.call('sync'), - convert_call, - ]) - - @mock.patch.object(utils, 'execute', autospec=True) - def test_convert_image_retries_alternate_error(self, execute_mock): - ret_err = 'Failed to allocate memory: Cannot allocate memory\n' - execute_mock.side_effect = [ - processutils.ProcessExecutionError(stderr=ret_err), ('', ''), - processutils.ProcessExecutionError(stderr=ret_err), ('', ''), - ('', ''), - ] - - disk_utils.convert_image('source', 'dest', 'out_format') - convert_call = mock.call('qemu-img', 'convert', '-O', - 'out_format', 'source', 'dest', - run_as_root=False, - prlimit=mock.ANY, - use_standard_locale=True, - env_variables={'MALLOC_ARENA_MAX': '3'}) - execute_mock.assert_has_calls([ - convert_call, - mock.call('sync'), - convert_call, - mock.call('sync'), - convert_call, - ]) - - @mock.patch.object(utils, 'execute', autospec=True) - def test_convert_image_retries_and_fails(self, execute_mock): - ret_err = 'qemu: qemu_thread_create: Resource temporarily unavailable' - execute_mock.side_effect = [ - processutils.ProcessExecutionError(stderr=ret_err), ('', ''), - processutils.ProcessExecutionError(stderr=ret_err), ('', ''), - processutils.ProcessExecutionError(stderr=ret_err), ('', ''), - processutils.ProcessExecutionError(stderr=ret_err), - ] - - self.assertRaises(processutils.ProcessExecutionError, - disk_utils.convert_image, - 'source', 'dest', 'out_format') - convert_call = mock.call('qemu-img', 'convert', '-O', - 'out_format', 'source', 'dest', - run_as_root=False, - prlimit=mock.ANY, - use_standard_locale=True, - env_variables={'MALLOC_ARENA_MAX': '3'}) - execute_mock.assert_has_calls([ - convert_call, - mock.call('sync'), - convert_call, - mock.call('sync'), - convert_call, - ]) - - @mock.patch.object(utils, 'execute', autospec=True) - def test_convert_image_just_fails(self, execute_mock): - ret_err = 'Aliens' - execute_mock.side_effect = [ - processutils.ProcessExecutionError(stderr=ret_err), - ] - - self.assertRaises(processutils.ProcessExecutionError, - disk_utils.convert_image, - 'source', 'dest', 'out_format') - convert_call = mock.call('qemu-img', 'convert', '-O', - 'out_format', 'source', 'dest', - run_as_root=False, - prlimit=mock.ANY, - use_standard_locale=True, - env_variables={'MALLOC_ARENA_MAX': '3'}) - execute_mock.assert_has_calls([ - convert_call, - ]) - @mock.patch.object(os.path, 'getsize', autospec=True) - @mock.patch.object(disk_utils, 'qemu_img_info', autospec=True) + @mock.patch.object(qemu_img, 'image_info', autospec=True) def test_get_image_mb(self, mock_qinfo, mock_getsize): mb = 1024 * 1024 diff --git a/ironic_lib/tests/test_qemu_img.py b/ironic_lib/tests/test_qemu_img.py new file mode 100644 index 00000000..5be23c98 --- /dev/null +++ b/ironic_lib/tests/test_qemu_img.py @@ -0,0 +1,169 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import os +from unittest import mock + +from oslo_concurrency import processutils +from oslo_config import cfg +from oslo_utils import imageutils + +from ironic_lib import qemu_img +from ironic_lib.tests import base +from ironic_lib import utils + +CONF = cfg.CONF + + +class ImageInfoTestCase(base.IronicLibTestCase): + + @mock.patch.object(os.path, 'exists', return_value=False, autospec=True) + def test_image_info_path_doesnt_exist(self, path_exists_mock): + self.assertRaises(FileNotFoundError, qemu_img.image_info, 'noimg') + path_exists_mock.assert_called_once_with('noimg') + + @mock.patch.object(utils, 'execute', return_value=('out', 'err'), + autospec=True) + @mock.patch.object(imageutils, 'QemuImgInfo', autospec=True) + @mock.patch.object(os.path, 'exists', return_value=True, autospec=True) + def test_image_info_path_exists(self, path_exists_mock, + image_info_mock, execute_mock): + qemu_img.image_info('img') + path_exists_mock.assert_called_once_with('img') + execute_mock.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', + 'qemu-img', 'info', 'img', + '--output=json', + prlimit=mock.ANY) + image_info_mock.assert_called_once_with('out', format='json') + + +class ConvertImageTestCase(base.IronicLibTestCase): + + @mock.patch.object(utils, 'execute', autospec=True) + def test_convert_image(self, execute_mock): + qemu_img.convert_image('source', 'dest', 'out_format') + execute_mock.assert_called_once_with( + 'qemu-img', 'convert', '-O', + 'out_format', 'source', 'dest', + run_as_root=False, + prlimit=mock.ANY, + use_standard_locale=True, + env_variables={'MALLOC_ARENA_MAX': '3'}) + + @mock.patch.object(utils, 'execute', autospec=True) + def test_convert_image_flags(self, execute_mock): + qemu_img.convert_image('source', 'dest', 'out_format', + cache='directsync', out_of_order=True, + sparse_size='0') + execute_mock.assert_called_once_with( + 'qemu-img', 'convert', '-O', + 'out_format', '-t', 'directsync', + '-S', '0', '-W', 'source', 'dest', + run_as_root=False, + prlimit=mock.ANY, + use_standard_locale=True, + env_variables={'MALLOC_ARENA_MAX': '3'}) + + @mock.patch.object(utils, 'execute', autospec=True) + def test_convert_image_retries(self, execute_mock): + ret_err = 'qemu: qemu_thread_create: Resource temporarily unavailable' + execute_mock.side_effect = [ + processutils.ProcessExecutionError(stderr=ret_err), ('', ''), + processutils.ProcessExecutionError(stderr=ret_err), ('', ''), + ('', ''), + ] + + qemu_img.convert_image('source', 'dest', 'out_format') + convert_call = mock.call('qemu-img', 'convert', '-O', + 'out_format', 'source', 'dest', + run_as_root=False, + prlimit=mock.ANY, + use_standard_locale=True, + env_variables={'MALLOC_ARENA_MAX': '3'}) + execute_mock.assert_has_calls([ + convert_call, + mock.call('sync'), + convert_call, + mock.call('sync'), + convert_call, + ]) + + @mock.patch.object(utils, 'execute', autospec=True) + def test_convert_image_retries_alternate_error(self, execute_mock): + ret_err = 'Failed to allocate memory: Cannot allocate memory\n' + execute_mock.side_effect = [ + processutils.ProcessExecutionError(stderr=ret_err), ('', ''), + processutils.ProcessExecutionError(stderr=ret_err), ('', ''), + ('', ''), + ] + + qemu_img.convert_image('source', 'dest', 'out_format') + convert_call = mock.call('qemu-img', 'convert', '-O', + 'out_format', 'source', 'dest', + run_as_root=False, + prlimit=mock.ANY, + use_standard_locale=True, + env_variables={'MALLOC_ARENA_MAX': '3'}) + execute_mock.assert_has_calls([ + convert_call, + mock.call('sync'), + convert_call, + mock.call('sync'), + convert_call, + ]) + + @mock.patch.object(utils, 'execute', autospec=True) + def test_convert_image_retries_and_fails(self, execute_mock): + ret_err = 'qemu: qemu_thread_create: Resource temporarily unavailable' + execute_mock.side_effect = [ + processutils.ProcessExecutionError(stderr=ret_err), ('', ''), + processutils.ProcessExecutionError(stderr=ret_err), ('', ''), + processutils.ProcessExecutionError(stderr=ret_err), ('', ''), + processutils.ProcessExecutionError(stderr=ret_err), + ] + + self.assertRaises(processutils.ProcessExecutionError, + qemu_img.convert_image, + 'source', 'dest', 'out_format') + convert_call = mock.call('qemu-img', 'convert', '-O', + 'out_format', 'source', 'dest', + run_as_root=False, + prlimit=mock.ANY, + use_standard_locale=True, + env_variables={'MALLOC_ARENA_MAX': '3'}) + execute_mock.assert_has_calls([ + convert_call, + mock.call('sync'), + convert_call, + mock.call('sync'), + convert_call, + ]) + + @mock.patch.object(utils, 'execute', autospec=True) + def test_convert_image_just_fails(self, execute_mock): + ret_err = 'Aliens' + execute_mock.side_effect = [ + processutils.ProcessExecutionError(stderr=ret_err), + ] + + self.assertRaises(processutils.ProcessExecutionError, + qemu_img.convert_image, + 'source', 'dest', 'out_format') + convert_call = mock.call('qemu-img', 'convert', '-O', + 'out_format', 'source', 'dest', + run_as_root=False, + prlimit=mock.ANY, + use_standard_locale=True, + env_variables={'MALLOC_ARENA_MAX': '3'}) + execute_mock.assert_has_calls([ + convert_call, + ]) diff --git a/setup.cfg b/setup.cfg index 81fd5439..7d39a706 100644 --- a/setup.cfg +++ b/setup.cfg @@ -37,6 +37,7 @@ oslo.config.opts = ironic_lib.mdns = ironic_lib.mdns:list_opts ironic_lib.metrics = ironic_lib.metrics_utils:list_opts ironic_lib.metrics_statsd = ironic_lib.metrics_statsd:list_opts + ironic_lib.qemu_img = ironic_lib.qemu_img:list_opts ironic_lib.utils = ironic_lib.utils:list_opts [extra] @@ -52,4 +53,4 @@ json_rpc = quiet-level = 4 # Words to ignore: # crypted: Valid in some contexts, e.g. "crypted password" -ignore-words-list = crypted \ No newline at end of file +ignore-words-list = crypted