openstack/swift
Code Issues Proposed changes

Change how O_TMPFILE support is detected

Browse Source 
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
This commit is contained in:
Thiago da Silva 2019-01-28 21:30:57 -05:00 committed by Tim Burke
parent 700fcc7353
commit 0668731839
5 changed files with 29 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
swift/common/utils.py
View File

@ -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:

12
swift/obj/diskfile.py
View File

@ -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

9
test/unit/__init__.py
View File

@ -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):

5
test/unit/common/test_utils.py
View File

@ -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)))

30
test/unit/obj/test_diskfile.py
View File

@ -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)