Federico Simoncelli has uploaded a new change for review. Change subject: oop: improve safety for truncateFile ......................................................................
oop: improve safety for truncateFile In order to make truncateFile safer and to avoid any confusion on its behavior: * a new comment has been added mentioning O_TRUNC and "w" to avoid any future mistake in this area * a new test has been added to check the expected outcomes * the "w" mode has been removed from truncateFile (used in os.fdopen) to prevent any future reconversion to open(path, "w") * the risk of a file descriptor leak (for a failing os.fdopen call) has been removed using the relevant posix calls Change-Id: Ib71b53498c7bc4ea7a1ab725feb18bc5929f8c85 Signed-off-by: Federico Simoncelli <fsimo...@redhat.com> --- M tests/remoteFileHandlerTests.py M vdsm/storage/remoteFileHandler.py 2 files changed, 39 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/46/20046/1 diff --git a/tests/remoteFileHandlerTests.py b/tests/remoteFileHandlerTests.py index 544ec28..a2ca574 100644 --- a/tests/remoteFileHandlerTests.py +++ b/tests/remoteFileHandlerTests.py @@ -18,6 +18,8 @@ # Refer to the README and COPYING files for full details of the license # import os +import string +import tempfile from vdsm import utils from testrunner import VdsmTestCase as TestCaseBase @@ -69,3 +71,29 @@ test = lambda: self.assertFalse(os.path.exists(procPath)) utils.retry(test, AssertionError, timeout=4, sleep=0.1) + + +class RemoteFileHandlerFunctionTests(TestCaseBase): + def testTruncateFile(self): + fd, path = tempfile.mkstemp() + try: + os.write(fd, string.ascii_uppercase) + os.close(fd) + + # Verifying content + data = string.ascii_uppercase + self.assertEquals(data, file(path).read()) + + # Testing truncate to a larger size + data = string.ascii_uppercase + chr(0) * 16 + + rhandler.truncateFile(path, len(data)) + self.assertEquals(data, file(path).read()) + + # Testing truncate to a smaller size + data = string.ascii_uppercase + + rhandler.truncateFile(path, len(data)) + self.assertEquals(data, file(path).read()) + finally: + os.unlink(path) diff --git a/vdsm/storage/remoteFileHandler.py b/vdsm/storage/remoteFileHandler.py index f01461f..5b24053 100644 --- a/vdsm/storage/remoteFileHandler.py +++ b/vdsm/storage/remoteFileHandler.py @@ -348,15 +348,23 @@ def truncateFile(path, size, mode=None, creatExcl=False): + # NOTE: Under no circumstance you should add the O_TRUNC + # flag here. We rely on the fact that the file content is + # not deleted when truncating to a larger size. + # Please also note that the "w" option used in open/file + # contains O_TRUNC and therefore should not be used here. flags = os.O_CREAT | os.O_WRONLY + if creatExcl: flags |= os.O_EXCL fd = os.open(path, flags) - with os.fdopen(fd, 'w') as f: + try: if mode is not None: - os.chmod(path, mode) - f.truncate(size) + os.fchmod(fd, mode) + os.ftruncate(fd, size) + finally: + os.close(fd) def readLines(path): -- To view, visit http://gerrit.ovirt.org/20046 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ib71b53498c7bc4ea7a1ab725feb18bc5929f8c85 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <fsimo...@redhat.com> _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches