Change in vdsm[master]: directio: Require full blocks

nsoffer at redhat.com nsoffer at redhat.com
Mon Mar 14 11:33:38 UTC 2016


Nir Soffer has uploaded a new change for review.

Change subject: directio: Require full blocks
......................................................................

directio: Require full blocks

Previously write accepted a partial block, silently padding the data
with zeros, possibly overwriting data.

For example, if file contents was:

    b"a" * 512 + b"b" * 512

Writing "c" * 768 would change to file to:

    b"c" * 768 + b"\x00" * 256

The correct way to perform such write is to read 2 blocks, modify the
data, and write back 2 blocks. But this is inefficient, so it must be
implemented in the caller of DirectFile. It also does not make sense in
the context of directio.

Now this write will fail with ValueError:

    ValueError: You can only write in 512 multiplies

Change-Id: I6c13e322926c424b83ae09d9d427e71598f99f3b
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
M lib/vdsm/storage/directio.py
M tests/storage_directio_test.py
2 files changed, 9 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/15/54715/1

diff --git a/lib/vdsm/storage/directio.py b/lib/vdsm/storage/directio.py
index a67bdb1..5092708 100644
--- a/lib/vdsm/storage/directio.py
+++ b/lib/vdsm/storage/directio.py
@@ -152,10 +152,8 @@
 
     def write(self, data):
         length = len(data)
-        padding = 512 - (length % 512)
-        if padding == 512:
-            padding = 0
-        length = length + padding
+        if length % 512:
+            raise ValueError("You can only write in 512 multiplies")
         pdata = ctypes.c_char_p(data)
         with self._createAlignedBuffer(length) as pbuff:
             ctypes.memmove(pbuff, pdata, len(data))
diff --git a/tests/storage_directio_test.py b/tests/storage_directio_test.py
index 6744b9d..9098c15 100644
--- a/tests/storage_directio_test.py
+++ b/tests/storage_directio_test.py
@@ -62,6 +62,13 @@
             with io.open(srcPath, "rb") as f:
                 self.assertEquals(f.read(), self.DATA)
 
+    def test_write_unaligned(self):
+        with temporaryPath(data=self.DATA) as srcPath, \
+                directio.DirectFile(srcPath, "r+") as f:
+            self.assertRaises(ValueError, f.write, "x" * 511)
+            with io.open(srcPath, "rb") as f:
+                self.assertEqual(f.read(), self.DATA)
+
     def test_update_and_read(self):
         with temporaryPath() as srcPath, \
                 directio.DirectFile(srcPath, "w") as f:


-- 
To view, visit https://gerrit.ovirt.org/54715
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6c13e322926c424b83ae09d9d427e71598f99f3b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>


More information about the vdsm-patches mailing list