Change in vdsm[master]: oop: improve safety for truncateFile
fsimonce at redhat.com
fsimonce at redhat.com
Wed Oct 9 19:14:59 UTC 2013
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 <fsimonce at 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 <fsimonce at redhat.com>
More information about the vdsm-patches
mailing list