openstack
/
ironic-inspector
Code Issues Proposed changes

Fix Py2/Py3 differences in write locking code 

In Py2, a file object write method returns NoneType.

In Py3, a file object write method returns a count of
bytes written, which can be used to interpret success.

As this code, at least for now, still needs to work
on Py2 without raising what appears to be errors, we
need to remove the assumption that data is returned
upon write.

Should a write fail, IOError is still raised in Py2,
which means the existing exception handling should be
sufficent.

Also, added an explicit flush for before we release the file
lock, just to ensure that the data is actually written out of
python's buffers before the lock is released.

Change-Id: I1cae8f1cd2f7da39600d72a84fe041ff0a97e580
Closes-Bug: #1741035
This commit is contained in:
Julia Kreger 2018-01-09 17:11:44 -08:00 committed by Ruby Loo
parent 6cfe607eaa
commit b88823a771
2 changed files with 10 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
ironic_inspector/pxe_filter/dnsmasq.py
View File

@ -164,7 +164,12 @@ def _exclusive_write_or_pass(path, buf):
while attempts:
try:
fcntl.flock(f, fcntl.LOCK_EX | fcntl.LOCK_NB)
return bool(f.write(buf))
f.write(buf)
# Go ahead and flush the data now instead of waiting until
# after the automatic flush with the file close after the
# file lock is released.
f.flush()
return True
except IOError as e:
if e.errno == os.errno.EWOULDBLOCK:
LOG.debug('%s locked; will try again (later)', path)
@ -177,7 +182,7 @@ def _exclusive_write_or_pass(path, buf):
LOG.debug('Failed to write the exclusively-locked path: %(path)s for '
'%(attempts)s times', {'attempts': _EXCLUSIVE_WRITE_ATTEMPTS,
'path': path})
return 0
return False
def _blacklist_mac(mac):

6
ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py
View File

@ -106,7 +106,7 @@ class TestExclusiveWriteOrPass(test_base.BaseTest):
def test_write(self):
wrote = dnsmasq._exclusive_write_or_pass(self.path, self.buf)
self.assertIs(bool(self.mock_fd.write.return_value), wrote)
self.assertEqual(True, wrote)
self.mock_open.assert_called_once_with(self.path, 'w', 1)
self.mock_fcntl.assert_has_calls(
[self.fcntl_lock_call, self.fcntl_unlock_call])
@ -124,7 +124,7 @@ class TestExclusiveWriteOrPass(test_base.BaseTest):
None, None]
wrote = dnsmasq._exclusive_write_or_pass(self.path, self.buf)
self.assertIs(bool(self.mock_fd.write.return_value), wrote)
self.assertEqual(True, wrote)
self.mock_open.assert_called_once_with(self.path, 'w', 1)
self.mock_fcntl.assert_has_calls(
[self.fcntl_lock_call, self.fcntl_unlock_call],
@ -144,7 +144,7 @@ class TestExclusiveWriteOrPass(test_base.BaseTest):
self.mock_fcntl.side_effect = [err, None]
wrote = dnsmasq._exclusive_write_or_pass(self.path, self.buf)
self.assertEqual(0, wrote)
self.assertEqual(False, wrote)
self.mock_open.assert_called_once_with(self.path, 'w', 1)
self.mock_fcntl.assert_has_calls(
[self.fcntl_lock_call, self.fcntl_unlock_call])