Skip to content

Commit

Permalink
Change how O_TMPFILE support is detected
Browse files Browse the repository at this point in the history
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
  • Loading branch information
thiagodasilva authored and tipabu committed Jan 31, 2019
1 parent 700fcc7 commit 0668731
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 44 deletions.
17 changes: 0 additions & 17 deletions swift/common/utils.py
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
12 changes: 5 additions & 7 deletions swift/obj/diskfile.py
Expand Up @@ -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, \
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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)
Expand All @@ -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):
Expand Down Expand Up @@ -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
Expand Down
9 changes: 0 additions & 9 deletions test/unit/__init__.py
Expand Up @@ -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):
Expand Down
5 changes: 2 additions & 3 deletions test/unit/common/test_utils.py
Expand Up @@ -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')

Expand Down Expand Up @@ -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)))
Expand Down
30 changes: 22 additions & 8 deletions test/unit/obj/test_diskfile.py
Expand Up @@ -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, \
Expand Down Expand Up @@ -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]

Expand Down Expand Up @@ -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)
Expand All @@ -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()
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 0668731

Please sign in to comment.