[issue11877] Change os.fsync() to support physical backing store syncs

[Steffen Daode Nurpmeso]

Steffen Daode Nurpmeso <sdaoden at googlemail.com> added the comment:

I've dropped wet-her!
I hope now you're satisfied!!!!!
So the buffer cache is all which remains hot.
How deserted!

> And you could also add a test (I guess that just calling fsync
> with full_sync=True on a valid FD would be enough.

I was able to add two tests as an extension to what is yet tested
about os.fsync(), but that uses an invalid fd.
(At least it enters the conditional and fails as expected.)

> I'm not sure static is necessary, I'd rather make it const.

Yes..

> This code is correct as it is, see other extension modules in
> the stdlib for other examples of this pattern

..but i've used copy+paste here.

> And you could also add a test (I guess that just calling fsync
> with full_sync=True on a valid FD would be enough.

> The alternative would be that full_sync

Ok, i've renamed full_fsync to full_sync.

----------
Added file: http://bugs.python.org/file22016/11877.9.diff

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue11877>
_______________________________________
-------------- next part --------------
diff --git a/Doc/library/os.rst b/Doc/library/os.rst
--- a/Doc/library/os.rst
+++ b/Doc/library/os.rst
@@ -798,7 +798,7 @@
    Availability: Unix.
 
 
-.. function:: fsync(fd)
+.. function:: fsync(fd, full_sync=False)
 
    Force write of file with filedescriptor *fd* to disk.  On Unix, this calls the
    native :c:func:`fsync` function; on Windows, the MS :c:func:`_commit` function.
@@ -807,6 +807,15 @@
    ``f.flush()``, and then do ``os.fsync(f.fileno())``, to ensure that all internal
    buffers associated with *f* are written to disk.
 
+   The POSIX standart requires that :c:func:`fsync` must transfer the buffered
+   data to the storage device, not that the data is actually written by the
+   device itself.  It explicitely leaves it up to operating system implementors
+   whether users are given stronger guarantees on data integrity or not.  Some
+   systems also offer special functions which overtake the part of making such
+   stronger guarantees, i.e., Mac OS X and NetBSD.  The optional *full_sync*
+   argument can be used to enforce usage of these special functions if that is
+   appropriate for the *fd* in question.
+
    Availability: Unix, and Windows.
 
 
diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py
--- a/Lib/test/test_os.py
+++ b/Lib/test/test_os.py
@@ -837,10 +837,10 @@
     singles = ["fchdir", "dup", "fdopen", "fdatasync", "fstat",
                "fstatvfs", "fsync", "tcgetpgrp", "ttyname"]
     #singles.append("close")
-    #We omit close because it doesn'r raise an exception on some platforms
+    # We omit close because it doesn't raise an exception on some platforms
     def get_single(f):
         def helper(self):
-            if  hasattr(os, f):
+            if hasattr(os, f):
                 self.check(getattr(os, f))
         return helper
     for f in singles:
@@ -855,6 +855,11 @@
             self.fail("%r didn't raise a OSError with a bad file descriptor"
                       % f)
 
+    def test_fsync_arg(self):
+        if hasattr(os, "fsync"):
+            self.check(os.fsync, True)
+            self.check(os.fsync, False)
+
     def test_isatty(self):
         if hasattr(os, "isatty"):
             self.assertEqual(os.isatty(support.make_bad_fd()), False)
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -2121,13 +2121,50 @@
 
 #ifdef HAVE_FSYNC
 PyDoc_STRVAR(posix_fsync__doc__,
-"fsync(fildes)\n\n\
-force write of file with filedescriptor to disk.");
-
-static PyObject *
-posix_fsync(PyObject *self, PyObject *fdobj)
-{
-    return posix_fildes(fdobj, fsync);
+"fsync(fildes, full_sync=False)\n\n"
+"force write of file buffers with fildes to disk;\n"
+"full_sync forces flush of disk caches in case fsync() alone is not enough.");
+
+static PyObject *
+posix_fsync(PyObject *self, PyObject *args, PyObject *kwargs)
+{
+    PyObject *fdobj;
+    int full_sync = 0;
+    static char *keywords[] = {"fd", "full_sync", NULL };
+
+    if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O|i", keywords,
+                                     &fdobj, &full_sync))
+        return NULL;
+
+    /* See issue 11877 discussion */
+# if ((defined __APPLE__ && defined F_FULLFSYNC) || \
+      (defined __NetBSD__ && defined FDISKSYNC))
+    if (full_sync != 0) {
+        int res, fd = PyObject_AsFileDescriptor(fdobj);
+        if (fd < 0)
+            return NULL;
+        if (!_PyVerify_fd(fd))
+            return posix_error();
+
+        Py_BEGIN_ALLOW_THREADS
+#  if defined __APPLE__
+        /* F_FULLFSYNC is not supported for all types of FDs/FSYSs;
+         * be on the safe side and test for inappropriate ioctl errors */
+        res = fcntl(fd, F_FULLFSYNC);
+        if (res < 0 && errno == ENOTTY)
+            res = fsync(fd);
+#  elif defined __NetBSD__
+        res = fsync_range(fd, FFILESYNC|FDISKSYNC, 0, 0);
+#  endif
+        Py_END_ALLOW_THREADS
+
+        if (res < 0)
+            return posix_error();
+        Py_INCREF(Py_None);
+        return Py_None;
+    } else
+# endif
+        return posix_fildes(fdobj, fsync);
 }
 #endif /* HAVE_FSYNC */
 
@@ -9472,7 +9509,8 @@
     {"fchdir",          posix_fchdir, METH_O, posix_fchdir__doc__},
 #endif
 #ifdef HAVE_FSYNC
-    {"fsync",       posix_fsync, METH_O, posix_fsync__doc__},
+    {"fsync",           (PyCFunction)posix_fsync, METH_VARARGS|METH_KEYWORDS,
+                        posix_fsync__doc__},
 #endif
 #ifdef HAVE_SYNC
     {"sync",        posix_sync, METH_NOARGS, posix_sync__doc__},