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

[Steffen Daode Nurpmeso]

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

> So I think you should stick with the previous version (well, if the
> full sync fails on other FDs, then it's another story, but in that
> case it should just be dropped altogether if it's not reliable...).

Strong stuff.
*This* is the version which should have been implemented from the
beginning, but Apple states

 F_FULLFSYNC Does the same thing as fsync(2) then asks the drive to
             flush all buffered data to the permanent storage
             device (arg is ignored).  This is currently
             implemented on HFS, MS-DOS (FAT), and Universal Disk
             Format (UDF) file systems.

and i thought
- fsync (maybe move buffers to Queue; do reorder Queue as approbiate)
- do call fsys impl. to .. whatever
That's why i had a version of the patch which did 'fsync();fcntl();'
because it would have been an additional syscall but the fsync()
part would possibly have been essentially "skipped over" ..unless..
Linux RC scripts had 'sync && sync && sync' but it does not seem
to be necessary any more (was it ever - i don't know).

But who knows if that fcntl will fail on some non-noted fsys?
I think it's better to be on the safe side.
Quoting you, Charles-François
> People requiring write durability will probably manage to find
> this full_sync parameter
and if they do they thus really strive for data integrity, so call
fsync() as a fallback for the security which Apple provides.
Also: we cannot let os.fsync() fail with ENOTTY!?

> By the way, it's "appropriate", not "approbiate". You made the same
> typo in your patch.

8~I  That was not a typo.  Thanks.
I'll changed that.

----------
Added file: http://bugs.python.org/file21986/11877.8.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_fsync=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
+   wether 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_fsync*
+   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/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_fsync=False)\n\n"
+"force write of file buffers with fildes to disk;\n"
+"full_fsync 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_fsync = 0;
+    static char *keywords[] = {"fd", "full_fsync", NULL };
+
+    if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O|i", keywords,
+                                     &fdobj, &full_fsync))
+        return NULL;
+
+    /* See issue 11877 discussion */
+# if ((defined __APPLE__ && defined F_FULLFSYNC) || \
+      (defined __NetBSD__ && defined FDISKSYNC))
+    if (full_fsync != 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__},