From 0668731839599180e581338320c5d216b8d8bb1f Mon Sep 17 00:00:00 2001 From: Thiago da Silva Date: Mon, 28 Jan 2019 21:30:57 -0500 Subject: [PATCH] Change how O_TMPFILE support is detected Previously o_tmpfile support was detected by checking the kernel version as it was officially introduced in XFS in 3.15. The problem is that RHEL has backported the support to at least RHEL 7.6 but the kernel version is not updated. This patch changes o_tmpfile is detected by actually attempting to open a file with the O_TMPFILE flag and keeps the information cached in DiskFileManager so that the check only happens once while process is running. Change-Id: I3599e2ab257bcd99467aee83b747939afac639d8 --- swift/common/utils.py | 17 ----------------- swift/obj/diskfile.py | 12 +++++------- test/unit/__init__.py | 9 --------- test/unit/common/test_utils.py | 5 ++--- test/unit/obj/test_diskfile.py | 30 ++++++++++++++++++++++-------- 5 files changed, 29 insertions(+), 44 deletions(-) diff --git a/swift/common/utils.py b/swift/common/utils.py index d765d122f7..430a07bf9b 100644 --- a/swift/common/utils.py +++ b/swift/common/utils.py @@ -40,7 +40,6 @@ import functools import platform import email.parser -from distutils.version import LooseVersion from hashlib import md5, sha1 from random import random, shuffle from contextlib import contextmanager, closing @@ -5206,22 +5205,6 @@ def o_tmpfile_in_tmpdir_supported(): return o_tmpfile_in_path_supported(gettempdir()) -def o_tmpfile_supported(): - """ - Returns True if O_TMPFILE flag is supported. - - O_TMPFILE was introduced in Linux 3.11 but it also requires support from - underlying filesystem being used. Some common filesystems and linux - versions in which those filesystems added support for O_TMPFILE: - xfs (3.15) - ext4 (3.11) - btrfs (3.16) - """ - return all([linkat.available, - platform.system() == 'Linux', - LooseVersion(platform.release()) >= LooseVersion('3.16')]) - - def safe_json_loads(value): if value: try: diff --git a/swift/obj/diskfile.py b/swift/obj/diskfile.py index c1316f5d05..d82110d6eb 100644 --- a/swift/obj/diskfile.py +++ b/swift/obj/diskfile.py @@ -65,7 +65,7 @@ fsync_dir, drop_buffer_cache, lock_path, write_pickle, \ config_true_value, listdir, split_path, remove_file, \ get_md5_socket, F_SETPIPE_SZ, decode_timestamps, encode_timestamps, \ - MD5_OF_EMPTY_STRING, link_fd_to_path, o_tmpfile_supported, \ + MD5_OF_EMPTY_STRING, link_fd_to_path, \ O_TMPFILE, makedirs_count, replace_partition_in_path, remove_directory from swift.common.splice import splice, tee from swift.common.exceptions import DiskFileQuarantined, DiskFileNotExist, \ @@ -705,7 +705,7 @@ def __init__(self, conf, logger): with open('/proc/sys/fs/pipe-max-size') as f: max_pipe_size = int(f.read()) self.pipe_size = min(max_pipe_size, self.disk_chunk_size) - self.use_linkat = o_tmpfile_supported() + self.use_linkat = True @classmethod def check_policy(cls, policy): @@ -1664,7 +1664,6 @@ def logger(self): return self.manager.logger def _get_tempfile(self): - fallback_to_mkstemp = False tmppath = None if self.manager.use_linkat: self._dirs_created = makedirs_count(self._datadir) @@ -1676,10 +1675,10 @@ def _get_tempfile(self): Falling back to using mkstemp()' \ % (self._datadir, os.strerror(err.errno)) self.logger.warning(msg) - fallback_to_mkstemp = True + self.manager.use_linkat = False else: raise - if not self.manager.use_linkat or fallback_to_mkstemp: + if not self.manager.use_linkat: tmpdir = join(self._diskfile._device_path, get_tmp_dir(self._diskfile.policy)) if not exists(tmpdir): @@ -2236,8 +2235,7 @@ class BaseDiskFile(object): def __init__(self, mgr, device_path, partition, account=None, container=None, obj=None, _datadir=None, policy=None, use_splice=False, pipe_size=None, - open_expired=False, next_part_power=None, - **kwargs): + open_expired=False, next_part_power=None, **kwargs): self._manager = mgr self._device_path = device_path self._logger = mgr.logger diff --git a/test/unit/__init__.py b/test/unit/__init__.py index 24c0eb08b2..cb2cb0b41d 100644 --- a/test/unit/__init__.py +++ b/test/unit/__init__.py @@ -1131,15 +1131,6 @@ def wrapper(*args, **kwargs): return wrapper -def requires_o_tmpfile_support(func): - @functools.wraps(func) - def wrapper(*args, **kwargs): - if not utils.o_tmpfile_supported(): - raise SkipTest('Requires O_TMPFILE support') - return func(*args, **kwargs) - return wrapper - - class StubResponse(object): def __init__(self, status, body='', headers=None, frag_index=None): diff --git a/test/unit/common/test_utils.py b/test/unit/common/test_utils.py index c163f3ab24..0879da1dae 100644 --- a/test/unit/common/test_utils.py +++ b/test/unit/common/test_utils.py @@ -77,8 +77,8 @@ from swift.common.header_key_dict import HeaderKeyDict from swift.common.storage_policy import POLICIES, reload_storage_policies from swift.common.swob import Request, Response -from test.unit import FakeLogger, requires_o_tmpfile_support, \ - requires_o_tmpfile_support_in_tmp, quiet_eventlet_exceptions +from test.unit import FakeLogger, requires_o_tmpfile_support_in_tmp, \ + quiet_eventlet_exceptions threading = eventlet.patcher.original('threading') @@ -3930,7 +3930,6 @@ def test_link_fd_to_path_target_exists(self): os.close(fd) shutil.rmtree(tempdir) - @requires_o_tmpfile_support def test_link_fd_to_path_errno_not_EEXIST_or_ENOENT(self): _m_linkat = mock.Mock( side_effect=IOError(errno.EACCES, os.strerror(errno.EACCES))) diff --git a/test/unit/obj/test_diskfile.py b/test/unit/obj/test_diskfile.py index 0ca1326e29..06eefcee2b 100644 --- a/test/unit/obj/test_diskfile.py +++ b/test/unit/obj/test_diskfile.py @@ -44,8 +44,8 @@ from test.unit import (mock as unit_mock, temptree, mock_check_drive, patch_policies, debug_logger, EMPTY_ETAG, make_timestamp_iter, DEFAULT_TEST_EC_TYPE, - requires_o_tmpfile_support, encode_frag_archive_bodies, - skip_if_no_xattrs) + requires_o_tmpfile_support_in_tmp, + encode_frag_archive_bodies, skip_if_no_xattrs) from swift.obj import diskfile from swift.common import utils from swift.common.utils import hash_path, mkdirs, Timestamp, \ @@ -3227,6 +3227,10 @@ def _create_test_file(self, data, timestamp=None, metadata=None, timestamp = time() timestamp = Timestamp(timestamp) + # avoid getting O_TMPFILE warning in logs + if not utils.o_tmpfile_in_tmpdir_supported(): + df.manager.use_linkat = False + if df.policy.policy_type == EC_POLICY: data = encode_frag_archive_bodies(df.policy, data)[df._frag_index] @@ -5127,7 +5131,7 @@ def test_create_unlink_cleanup_logging(self): for line in error_lines: self.assertTrue(line.startswith("Error removing tempfile:")) - @requires_o_tmpfile_support + @requires_o_tmpfile_support_in_tmp def test_get_tempfile_use_linkat_os_open_called(self): df = self._simple_get_diskfile() self.assertTrue(df.manager.use_linkat) @@ -5146,12 +5150,13 @@ def test_get_tempfile_use_linkat_os_open_called(self): self.assertEqual(fd, 12345) self.assertFalse(_m_mkstemp.called) - @requires_o_tmpfile_support + @requires_o_tmpfile_support_in_tmp def test_get_tempfile_fallback_to_mkstemp(self): df = self._simple_get_diskfile() df._logger = debug_logger() self.assertTrue(df.manager.use_linkat) for err in (errno.EOPNOTSUPP, errno.EISDIR, errno.EINVAL): + df.manager.use_linkat = True _m_open = mock.Mock(side_effect=OSError(err, os.strerror(err))) _m_mkstemp = mock.MagicMock(return_value=(0, "blah")) _m_mkc = mock.Mock() @@ -5165,13 +5170,14 @@ def test_get_tempfile_fallback_to_mkstemp(self): # Fallback should succeed and mkstemp() should be called. self.assertTrue(_m_mkstemp.called) self.assertEqual(tmppath, "blah") - # Despite fs not supporting O_TMPFILE, use_linkat should not change - self.assertTrue(df.manager.use_linkat) + # Once opening file with O_TMPFILE has failed, + # failure is cached to not try again + self.assertFalse(df.manager.use_linkat) log = df.manager.logger.get_lines_for_level('warning') self.assertGreater(len(log), 0) self.assertTrue('O_TMPFILE' in log[-1]) - @requires_o_tmpfile_support + @requires_o_tmpfile_support_in_tmp def test_get_tmpfile_os_open_other_exceptions_are_raised(self): df = self._simple_get_diskfile() _m_open = mock.Mock(side_effect=OSError(errno.ENOSPC, @@ -5192,7 +5198,7 @@ def test_get_tmpfile_os_open_other_exceptions_are_raised(self): # mkstemp() should not be invoked. self.assertFalse(_m_mkstemp.called) - @requires_o_tmpfile_support + @requires_o_tmpfile_support_in_tmp def test_create_use_linkat_renamer_not_called(self): df = self._simple_get_diskfile() data = '0' * 100 @@ -6754,6 +6760,10 @@ def _check_unpickle_error_and_get_hashes_failure(self, existing): df = df_mgr.get_diskfile('sda1', '0', 'a', 'c', 'o', policy=policy) suffix = os.path.basename(os.path.dirname(df._datadir)) + + # avoid getting O_TMPFILE warning in logs + if not utils.o_tmpfile_in_tmpdir_supported(): + df.manager.use_linkat = False if existing: df.delete(self.ts()) hashes = df_mgr.get_hashes('sda1', '0', [], policy) @@ -6915,6 +6925,10 @@ def test_consolidate_hashes_raises_exception(self): # create something to hash df = df_mgr.get_diskfile('sda1', '0', 'a', 'c', 'o', policy=policy) + + # avoid getting O_TMPFILE warning in logs + if not utils.o_tmpfile_in_tmpdir_supported(): + df.manager.use_linkat = False df.delete(self.ts()) suffix_dir = os.path.dirname(df._datadir) suffix = os.path.basename(suffix_dir)