openstack/swift
Code Issues Proposed changes

Better handle missing files in _construct_from_data_file

Browse Source 
There's a window between when we list the files on disk and when we actually
try to open the .data file where another process could delete it. That should
raise a DiskFileNotExist error, not an IOError.

Change-Id: I1b43fef35949cb6f71997874e4e6b7646eeec8fe
Closes-Bug: 1712662
This commit is contained in:
Tim Burke 2017-11-02 11:43:16 -07:00
parent 5731fab90b
commit 396380f340
2 changed files with 24 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
swift/obj/diskfile.py
View File

@ -2468,7 +2468,12 @@ class BaseDiskFile(object):
:raises DiskFileError: various exceptions from
:func:`swift.obj.diskfile.DiskFile._verify_data_file`
"""
fp = open(data_file, 'rb')
try:
fp = open(data_file, 'rb')
except IOError as e:
if e.errno == errno.ENOENT:
raise DiskFileNotExist()
raise
self._datafile_metadata = self._failsafe_read_metadata(
fp, data_file,
add_missing_checksum=modernize)

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

@ -2235,11 +2235,11 @@ class TestECDiskFileManager(DiskFileManagerMixin, unittest.TestCase):
rmtree(df._datadir, ignore_errors=True)
# sanity
files = [
good_files = [
'0000000006.00000.meta',
'0000000006.00000#1#d.data'
]
with create_files(class_under_test, files):
with create_files(class_under_test, good_files):
class_under_test.open()
scenarios = [['0000000007.00000.meta'],
@ -2263,6 +2263,22 @@ class TestECDiskFileManager(DiskFileManagerMixin, unittest.TestCase):
self.fail('expected DiskFileNotExist opening %s with %r' % (
class_under_test.__class__.__name__, files))
# Simulate another process deleting the data after we list contents
# but before we actually open them
orig_listdir = os.listdir
def deleting_listdir(d):
result = orig_listdir(d)
for f in result:
os.unlink(os.path.join(d, f))
return result
with create_files(class_under_test, good_files), \
mock.patch('swift.obj.diskfile.os.listdir',
side_effect=deleting_listdir), \
self.assertRaises(DiskFileNotExist):
class_under_test.open()
def test_verify_ondisk_files(self):
# _verify_ondisk_files should only return False if get_ondisk_files
# has produced a bad set of files due to a bug, so to test it we need