openstack/swift
Code Issues Proposed changes

Merge "Change how O_TMPFILE support is detected"

Browse Source
This commit is contained in:
Zuul 2019-02-01 00:58:39 +00:00 committed by Gerrit Code Review
commit 32fee1dc13
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 functools
import platform import platform
import email.parser import email.parser
from distutils.version import LooseVersion
from hashlib import md5, sha1 from hashlib import md5, sha1
from random import random, shuffle from random import random, shuffle
from contextlib import contextmanager, closing from contextlib import contextmanager, closing
@ -5206,22 +5205,6 @@ def o_tmpfile_in_tmpdir_supported():
return o_tmpfile_in_path_supported(gettempdir()) 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): def safe_json_loads(value):
if value: if value:
try: 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, \ fsync_dir, drop_buffer_cache, lock_path, write_pickle, \
config_true_value, listdir, split_path, remove_file, \ config_true_value, listdir, split_path, remove_file, \
get_md5_socket, F_SETPIPE_SZ, decode_timestamps, encode_timestamps, \ 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 O_TMPFILE, makedirs_count, replace_partition_in_path, remove_directory
from swift.common.splice import splice, tee from swift.common.splice import splice, tee
from swift.common.exceptions import DiskFileQuarantined, DiskFileNotExist, \ from swift.common.exceptions import DiskFileQuarantined, DiskFileNotExist, \
@ -705,7 +705,7 @@ class BaseDiskFileManager(object):
with open('/proc/sys/fs/pipe-max-size') as f: with open('/proc/sys/fs/pipe-max-size') as f:
max_pipe_size = int(f.read()) max_pipe_size = int(f.read())
self.pipe_size = min(max_pipe_size, self.disk_chunk_size) self.pipe_size = min(max_pipe_size, self.disk_chunk_size)
self.use_linkat = o_tmpfile_supported() self.use_linkat = True
@classmethod @classmethod
def check_policy(cls, policy): def check_policy(cls, policy):
@ -1672,7 +1672,6 @@ class BaseDiskFileWriter(object):
return self.manager.logger return self.manager.logger
def _get_tempfile(self): def _get_tempfile(self):
fallback_to_mkstemp = False
tmppath = None tmppath = None
if self.manager.use_linkat: if self.manager.use_linkat:
self._dirs_created = makedirs_count(self._datadir) self._dirs_created = makedirs_count(self._datadir)
@ -1684,10 +1683,10 @@ class BaseDiskFileWriter(object):
Falling back to using mkstemp()' \ Falling back to using mkstemp()' \
% (self._datadir, os.strerror(err.errno)) % (self._datadir, os.strerror(err.errno))
self.logger.warning(msg) self.logger.warning(msg)
fallback_to_mkstemp = True self.manager.use_linkat = False
else: else:
raise raise
if not self.manager.use_linkat or fallback_to_mkstemp: if not self.manager.use_linkat:
tmpdir = join(self._diskfile._device_path, tmpdir = join(self._diskfile._device_path,
get_tmp_dir(self._diskfile.policy)) get_tmp_dir(self._diskfile.policy))
if not exists(tmpdir): if not exists(tmpdir):
@ -2244,8 +2243,7 @@ class BaseDiskFile(object):
def __init__(self, mgr, device_path, partition, def __init__(self, mgr, device_path, partition,
account=None, container=None, obj=None, _datadir=None, account=None, container=None, obj=None, _datadir=None,
policy=None, use_splice=False, pipe_size=None, policy=None, use_splice=False, pipe_size=None,
open_expired=False, next_part_power=None, open_expired=False, next_part_power=None, **kwargs):
**kwargs):
self._manager = mgr self._manager = mgr
self._device_path = device_path self._device_path = device_path
self._logger = mgr.logger 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 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): class StubResponse(object):
def __init__(self, status, body='', headers=None, frag_index=None): 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.header_key_dict import HeaderKeyDict
from swift.common.storage_policy import POLICIES, reload_storage_policies from swift.common.storage_policy import POLICIES, reload_storage_policies
from swift.common.swob import Request, Response from swift.common.swob import Request, Response
from test.unit import FakeLogger, requires_o_tmpfile_support, \ from test.unit import FakeLogger, requires_o_tmpfile_support_in_tmp, \
requires_o_tmpfile_support_in_tmp, quiet_eventlet_exceptions quiet_eventlet_exceptions
threading = eventlet.patcher.original('threading') threading = eventlet.patcher.original('threading')
@ -3930,7 +3930,6 @@ cluster_dfw1 = http://dfw1.host/v1/
os.close(fd) os.close(fd)
shutil.rmtree(tempdir) shutil.rmtree(tempdir)
@requires_o_tmpfile_support
def test_link_fd_to_path_errno_not_EEXIST_or_ENOENT(self): def test_link_fd_to_path_errno_not_EEXIST_or_ENOENT(self):
_m_linkat = mock.Mock( _m_linkat = mock.Mock(
side_effect=IOError(errno.EACCES, os.strerror(errno.EACCES))) 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, from test.unit import (mock as unit_mock, temptree, mock_check_drive,
patch_policies, debug_logger, EMPTY_ETAG, patch_policies, debug_logger, EMPTY_ETAG,
make_timestamp_iter, DEFAULT_TEST_EC_TYPE, make_timestamp_iter, DEFAULT_TEST_EC_TYPE,
requires_o_tmpfile_support, encode_frag_archive_bodies, requires_o_tmpfile_support_in_tmp,
skip_if_no_xattrs) encode_frag_archive_bodies, skip_if_no_xattrs)
from swift.obj import diskfile from swift.obj import diskfile
from swift.common import utils from swift.common import utils
from swift.common.utils import hash_path, mkdirs, Timestamp, \ from swift.common.utils import hash_path, mkdirs, Timestamp, \
@ -3224,6 +3224,10 @@ class DiskFileMixin(BaseDiskFileTestMixin):
timestamp = time() timestamp = time()
timestamp = Timestamp(timestamp) 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: if df.policy.policy_type == EC_POLICY:
data = encode_frag_archive_bodies(df.policy, data)[df._frag_index] data = encode_frag_archive_bodies(df.policy, data)[df._frag_index]
@ -5104,7 +5108,7 @@ class DiskFileMixin(BaseDiskFileTestMixin):
for line in error_lines: for line in error_lines:
self.assertTrue(line.startswith("Error removing tempfile:")) 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): def test_get_tempfile_use_linkat_os_open_called(self):
df = self._simple_get_diskfile() df = self._simple_get_diskfile()
self.assertTrue(df.manager.use_linkat) self.assertTrue(df.manager.use_linkat)
@ -5123,12 +5127,13 @@ class DiskFileMixin(BaseDiskFileTestMixin):
self.assertEqual(fd, 12345) self.assertEqual(fd, 12345)
self.assertFalse(_m_mkstemp.called) self.assertFalse(_m_mkstemp.called)
@requires_o_tmpfile_support @requires_o_tmpfile_support_in_tmp
def test_get_tempfile_fallback_to_mkstemp(self): def test_get_tempfile_fallback_to_mkstemp(self):
df = self._simple_get_diskfile() df = self._simple_get_diskfile()
df._logger = debug_logger() df._logger = debug_logger()
self.assertTrue(df.manager.use_linkat) self.assertTrue(df.manager.use_linkat)
for err in (errno.EOPNOTSUPP, errno.EISDIR, errno.EINVAL): 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_open = mock.Mock(side_effect=OSError(err, os.strerror(err)))
_m_mkstemp = mock.MagicMock(return_value=(0, "blah")) _m_mkstemp = mock.MagicMock(return_value=(0, "blah"))
_m_mkc = mock.Mock() _m_mkc = mock.Mock()
@ -5142,13 +5147,14 @@ class DiskFileMixin(BaseDiskFileTestMixin):
# Fallback should succeed and mkstemp() should be called. # Fallback should succeed and mkstemp() should be called.
self.assertTrue(_m_mkstemp.called) self.assertTrue(_m_mkstemp.called)
self.assertEqual(tmppath, "blah") self.assertEqual(tmppath, "blah")
# Despite fs not supporting O_TMPFILE, use_linkat should not change # Once opening file with O_TMPFILE has failed,
self.assertTrue(df.manager.use_linkat) # failure is cached to not try again
self.assertFalse(df.manager.use_linkat)
log = df.manager.logger.get_lines_for_level('warning') log = df.manager.logger.get_lines_for_level('warning')
self.assertGreater(len(log), 0) self.assertGreater(len(log), 0)
self.assertTrue('O_TMPFILE' in log[-1]) 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): def test_get_tmpfile_os_open_other_exceptions_are_raised(self):
df = self._simple_get_diskfile() df = self._simple_get_diskfile()
_m_open = mock.Mock(side_effect=OSError(errno.ENOSPC, _m_open = mock.Mock(side_effect=OSError(errno.ENOSPC,
@ -5169,7 +5175,7 @@ class DiskFileMixin(BaseDiskFileTestMixin):
# mkstemp() should not be invoked. # mkstemp() should not be invoked.
self.assertFalse(_m_mkstemp.called) self.assertFalse(_m_mkstemp.called)
@requires_o_tmpfile_support @requires_o_tmpfile_support_in_tmp
def test_create_use_linkat_renamer_not_called(self): def test_create_use_linkat_renamer_not_called(self):
df = self._simple_get_diskfile() df = self._simple_get_diskfile()
data = '0' * 100 data = '0' * 100
@ -6731,6 +6737,10 @@ class TestSuffixHashes(unittest.TestCase):
df = df_mgr.get_diskfile('sda1', '0', 'a', 'c', 'o', df = df_mgr.get_diskfile('sda1', '0', 'a', 'c', 'o',
policy=policy) policy=policy)
suffix = os.path.basename(os.path.dirname(df._datadir)) 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: if existing:
df.delete(self.ts()) df.delete(self.ts())
hashes = df_mgr.get_hashes('sda1', '0', [], policy) hashes = df_mgr.get_hashes('sda1', '0', [], policy)
@ -6892,6 +6902,10 @@ class TestSuffixHashes(unittest.TestCase):
# create something to hash # create something to hash
df = df_mgr.get_diskfile('sda1', '0', 'a', 'c', 'o', df = df_mgr.get_diskfile('sda1', '0', 'a', 'c', 'o',
policy=policy) 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()) df.delete(self.ts())
suffix_dir = os.path.dirname(df._datadir) suffix_dir = os.path.dirname(df._datadir)
suffix = os.path.basename(suffix_dir) suffix = os.path.basename(suffix_dir)