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 uuid 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 @@ from swift.common.utils import mkdirs, Timestamp, \ 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 @@ class BaseDiskFileManager(object): 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 @@ class BaseDiskFileWriter(object): 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 @@ class BaseDiskFileWriter(object): 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 requires_o_tmpfile_support_in_tmp(func): 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.container_sync_realms import ContainerSyncRealms 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 @@ cluster_dfw1 = http://dfw1.host/v1/ 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 swift.obj.diskfile import MD5_OF_EMPTY_STRING, update_auditor_status 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 @@ class DiskFileMixin(BaseDiskFileTestMixin): 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 @@ class DiskFileMixin(BaseDiskFileTestMixin): 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 @@ class DiskFileMixin(BaseDiskFileTestMixin): 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 @@ class DiskFileMixin(BaseDiskFileTestMixin): # 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 @@ class DiskFileMixin(BaseDiskFileTestMixin): # 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 @@ class TestSuffixHashes(unittest.TestCase): 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 @@ class TestSuffixHashes(unittest.TestCase): # 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)