fsync in glib/gio

[Alexander Larsson <alexl redhat com>]

With all the recent yahoo about ext4 data loss and fsync I felt I had to
look at glib and make sure we're doing this right.

Attached is a patch that makes sure we fsync before closing in the gio
file saving code and in g_file_set_contents().

It also adds G_FILE_CREATE_ASYNC_WRITE flag to disable the fsync in the
gio saving code. This is used in the file copy code as I think the fsync
may significantly affect performance when copying lots of files. This is
similar to cp, which also doesn't fsync, so I think this is a decent
compromise. At least we do the safe thing when apps save a document.

Opinions? Maybe we should try to get this in this release, as a lot of
distros are starting to switch to ext4 which makes this a bit more
urgent.

It might also be interesting to look over the rest of our platform for
similar places where fsync is missing.

For some background info on why this is needed, see e.g.:
http://www.flamingspork.com/talks/2007/06/eat_my_data.odp

Index: gio/glocalfileoutputstream.c
===================================================================
--- gio/glocalfileoutputstream.c	(revision 7974)
+++ gio/glocalfileoutputstream.c	(working copy)
@@ -69,6 +69,7 @@ struct _GLocalFileOutputStreamPrivate {
   char *original_filename;
   char *backup_filename;
   char *etag;
+  gboolean async_write;
   int fd;
 };
 
@@ -81,7 +82,7 @@ static gboolean   g_local_file_output_st
 							   GCancellable       *cancellable,
 							   GError            **error);
 static GFileInfo *g_local_file_output_stream_query_info   (GFileOutputStream  *stream,
-							   char               *attributes,
+							   const char         *attributes,
 							   GCancellable       *cancellable,
 							   GError            **error);
 static char *     g_local_file_output_stream_get_etag     (GFileOutputStream  *stream);
@@ -190,6 +191,34 @@ g_local_file_output_stream_close (GOutpu
 
   file = G_LOCAL_FILE_OUTPUT_STREAM (stream);
 
+#ifdef HAVE_FSYNC
+  if (!file->priv->async_write)
+    {
+      /* We're either writing over an old truncated file, or to a
+       * new file that will be renamed over an old file. We want
+       * to ensure that all the data reaches disk here to avoid loss
+       * of the old data if delayed writes causes the new data to not
+       * be written in a while and the machine crashes.
+       *
+       * However, even though it might be a good idea we can't do this
+       * for all writes, because fsync is a very heavy operation that
+       * can take a long while. For instance, on ext3 with the default
+       * data=ordered setting it will flush all outstanding buffers,
+       * not just the ones related to this file.
+       */
+      if (fsync (file->priv->fd) != 0)
+	{
+          int errsv = errno;
+	  
+	  g_set_error (error, G_IO_ERROR,
+		       g_io_error_from_errno (errno),
+		       _("Error writing to file: %s"),
+		       g_strerror (errsv));
+	  goto err_out;
+	}
+    }
+#endif
+ 
 #ifdef G_OS_WIN32
 
   /* Must close before renaming on Windows, so just do the close first
@@ -459,7 +488,7 @@ g_local_file_output_stream_truncate (GFi
 
 static GFileInfo *
 g_local_file_output_stream_query_info (GFileOutputStream  *stream,
-				       char               *attributes,
+				       const char         *attributes,
 				       GCancellable       *cancellable,
 				       GError            **error)
 {
@@ -517,6 +546,8 @@ _g_local_file_output_stream_create  (con
   
   stream = g_object_new (G_TYPE_LOCAL_FILE_OUTPUT_STREAM, NULL);
   stream->priv->fd = fd;
+  if (flags & G_FILE_CREATE_ASYNC_WRITE)
+    stream->priv->async_write = TRUE;
   return G_FILE_OUTPUT_STREAM (stream);
 }
 
@@ -562,6 +593,8 @@ _g_local_file_output_stream_append  (con
   
   stream = g_object_new (G_TYPE_LOCAL_FILE_OUTPUT_STREAM, NULL);
   stream->priv->fd = fd;
+  if (flags & G_FILE_CREATE_ASYNC_WRITE)
+    stream->priv->async_write = TRUE;
   
   return G_FILE_OUTPUT_STREAM (stream);
 }
@@ -1022,6 +1055,8 @@ _g_local_file_output_stream_replace (con
  
   stream = g_object_new (G_TYPE_LOCAL_FILE_OUTPUT_STREAM, NULL);
   stream->priv->fd = fd;
+  if (flags & G_FILE_CREATE_ASYNC_WRITE)
+    stream->priv->async_write = TRUE;
   stream->priv->tmp_filename = temp_file;
   if (create_backup)
     stream->priv->backup_filename = create_backup_filename (filename);
Index: gio/gioenums.h
===================================================================
--- gio/gioenums.h	(revision 7974)
+++ gio/gioenums.h	(working copy)
@@ -164,13 +164,18 @@ typedef enum {
  *    You can think of it as "unlink destination" before
  *    writing to it, although the implementation may not
  *    be exactly like that. Since 2.20
+ * @G_FILE_CREATE_ASYNC_WRITE: Don't sync the file to make
+ *    sure its on disk before returning. This can lead to
+ *    data loss of the file in case of a system crash, but
+ *    allows better performance.
  *
  * Flags used when an operation may create a file.
  */
 typedef enum {
   G_FILE_CREATE_NONE    = 0,
   G_FILE_CREATE_PRIVATE = (1 << 0),
-  G_FILE_CREATE_REPLACE_DESTINATION = (1 << 1)
+  G_FILE_CREATE_REPLACE_DESTINATION = (1 << 1),
+  G_FILE_CREATE_ASYNC_WRITE = (1 << 2),
 } GFileCreateFlags;
 
 
Index: gio/gfile.c
===================================================================
--- gio/gfile.c	(revision 7974)
+++ gio/gfile.c	(working copy)
@@ -2360,7 +2360,8 @@ file_copy_fallback (GFile               
       out = (GOutputStream *)g_file_replace (destination,
 					     NULL,
 					     flags & G_FILE_COPY_BACKUP,
-                                             G_FILE_CREATE_REPLACE_DESTINATION,
+                                             G_FILE_CREATE_REPLACE_DESTINATION |
+                                             G_FILE_CREATE_ASYNC_WRITE,
 					     cancellable, error);
     }
   else
Index: configure.in
===================================================================
--- configure.in	(revision 7974)
+++ configure.in	(working copy)
@@ -563,6 +563,7 @@ AC_CHECK_FUNCS(mmap)
 AC_CHECK_FUNCS(posix_memalign)
 AC_CHECK_FUNCS(memalign)
 AC_CHECK_FUNCS(valloc)
+AC_CHECK_FUNCS(fsync)
 
 AC_CHECK_FUNCS(atexit on_exit)
 
Index: glib/gfileutils.c
===================================================================
--- glib/gfileutils.c	(revision 7974)
+++ glib/gfileutils.c	(working copy)
@@ -942,11 +942,47 @@ write_to_temp_file (const gchar  *conten
 	  goto out;
 	}
     }
-   
+
+  errno = 0;
+  if (fflush (file) != 0)
+    { 
+      save_errno = errno;
+      
+      g_set_error (err,
+		   G_FILE_ERROR,
+		   g_file_error_from_errno (save_errno),
+		   _("Failed to write file '%s': fflush() failed: %s"),
+		   display_name, 
+		   g_strerror (save_errno));
+
+      g_unlink (tmp_name);
+      
+      goto out;
+    }
+  
+#ifdef HAVE_FSYNC
+  errno = 0;
+  if (fsync (fileno (file)) != 0)
+    { 
+      save_errno = errno;
+      
+      g_set_error (err,
+		   G_FILE_ERROR,
+		   g_file_error_from_errno (save_errno),
+		   _("Failed to write file '%s': fsync() failed: %s"),
+		   display_name, 
+		   g_strerror (save_errno));
+
+      g_unlink (tmp_name);
+      
+      goto out;
+    }
+#endif
+  
   errno = 0;
   if (fclose (file) == EOF)
     { 
-      save_errno = 0;
+      save_errno = errno;
       
       g_set_error (err,
 		   G_FILE_ERROR,