Change in vdsm[master]: oop: improve safety for truncateFile

Wednesday, 9 October 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(a)redhat.com&gt;
---
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(a)redhat.com&gt;

    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011