bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified

[Giuseppe Scrivano]

Pádraig Brady <address@hidden> writes:

> thanks!
> Pádraig.

Thanks for the review, I've amended the changes you suggested:

>From b2babc9838b52892e2cdc46bc4590fa852daa0eb Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <address@hidden>
Date: Sun, 25 Jan 2015 01:33:45 +0100
Subject: [PATCH] sync: add support for fsync(2), fdatasync(2) and syncfs(2)

* AUTHORS: Add myself to sync's authors.
* NEWS: Mention the new feature.
* m4/jm-macros.m4 (coreutils_MACROS): Check for syncfs.
* man/sync.x: Add references to syncfs, fsync and fdatasync.
* doc/coreutils.texi (sync invocation): Document the new feature.
* src/sync.c: Include "quote.h".
(AUTHORS): Include myself.
(MODE_FILE, MODE_DATA, MODE_FILE_SYSTEM, MODE_SYNC): New enum values.
(long_options): Define.
(sync_arg): New function.
(usage): Describe that arguments are now accepted.
(main): Add arguments parsing and add support for fsync(2),
fdatasync(2) and syncfs(2).
---
 AUTHORS            |   2 +-
 NEWS               |   3 +
 doc/coreutils.texi |  20 ++++++-
 m4/jm-macros.m4    |   1 +
 man/sync.x         |   2 +-
 src/sync.c         | 157 +++++++++++++++++++++++++++++++++++++++++++++++++----
 6 files changed, 170 insertions(+), 15 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index 0296830..64c11d7 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -83,7 +83,7 @@ stat: Michael Meskes
 stdbuf: Pádraig Brady
 stty: David MacKenzie
 sum: Kayvan Aghaiepour, David MacKenzie
-sync: Jim Meyering
+sync: Jim Meyering, Giuseppe Scrivano
 tac: Jay Lepreau, David MacKenzie
 tail: Paul Rubin, David MacKenzie, Ian Lance Taylor, Jim Meyering
 tee: Mike Parker, Richard M. Stallman, David MacKenzie
diff --git a/NEWS b/NEWS
index e0a2893..3d4190b 100644
--- a/NEWS
+++ b/NEWS
@@ -48,6 +48,9 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   split accepts a new --separator option to select a record separator character
   other than the default newline character.
 
+  sync no longer ignores arguments and it uses fsync(2), fdatasync(2)
+  and syncfs(2) synchronization in addition to sync(2).
+
 ** Changes in behavior
 
   df no longer suppresses separate exports of the same remote device, as
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 5a3c31a..a1e664b 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -12053,8 +12053,24 @@ crashes, data may be lost or the file system corrupted 
as a
 result.  The @command{sync} command ensures everything in memory
 is written to disk.
 
-Any arguments are ignored, except for a lone @option{--help} or
address@hidden (@pxref{Common options}).
+If any argument is specified then only the specified files will be
+synchronized.  It uses internally the syscall fsync(2) on each of them.
+
+If at least one file is specified, it is possible to change the
+synchronization policy with the following options.  Also see
address@hidden options}.
+
address@hidden @samp
address@hidden --data
address@hidden --data
+Do not synchronize the file metadata unless it is required to maintain
+data integrity.  It uses the syscall fdatasync(2).
+
address@hidden --file-system
address@hidden --file-system
+Synchronize all the I/O waiting for the file systems that contain the file.
+It uses the syscall syncfs(2).
address@hidden table
 
 @exitstatus
 
diff --git a/m4/jm-macros.m4 b/m4/jm-macros.m4
index 58fdd97..79f124b 100644
--- a/m4/jm-macros.m4
+++ b/m4/jm-macros.m4
@@ -89,6 +89,7 @@ AC_DEFUN([coreutils_MACROS],
     sethostname
     siginterrupt
     sync
+    syncfs
     sysctl
     sysinfo
     tcgetpgrp
diff --git a/man/sync.x b/man/sync.x
index 7947bb7..6ced07e 100644
--- a/man/sync.x
+++ b/man/sync.x
@@ -3,4 +3,4 @@ sync \- flush file system buffers
 [DESCRIPTION]
 .\" Add any additional description here
 [SEE ALSO]
-sync(2)
+fdatasync(2), fsync(2), sync(2), syncfs(2)
diff --git a/src/sync.c b/src/sync.c
index e9f4d7e..6c4d571 100644
--- a/src/sync.c
+++ b/src/sync.c
@@ -23,12 +23,31 @@
 
 #include "system.h"
 #include "error.h"
-#include "long-options.h"
+#include "quote.h"
 
 /* The official name of this program (e.g., no 'g' prefix).  */
 #define PROGRAM_NAME "sync"
 
-#define AUTHORS proper_name ("Jim Meyering")
+#define AUTHORS                                 \
+  proper_name ("Jim Meyering"),                 \
+  proper_name ("Giuseppe Scrivano")
+
+enum
+{
+  MODE_FILE,
+  MODE_DATA,
+  MODE_FILE_SYSTEM,
+  MODE_SYNC
+};
+
+static struct option const long_options[] =
+{
+  {"data", no_argument, NULL, MODE_DATA},
+  {"file-system", no_argument, NULL, MODE_FILE_SYSTEM},
+  {GETOPT_HELP_OPTION_DECL},
+  {GETOPT_VERSION_OPTION_DECL},
+  {NULL, 0, NULL, 0}
+};
 
 void
 usage (int status)
@@ -37,11 +56,21 @@ usage (int status)
     emit_try_help ();
   else
     {
-      printf (_("Usage: %s [OPTION]\n"), program_name);
+      printf (_("Usage: %s [OPTION] [PATH]...\n"), program_name);
       fputs (_("\
 Force changed blocks to disk, update the super block.\n\
 \n\
+If one or more file paths are specified, sync only them,\n\
+use --data and --file-system to change the default behavior\n\
+\n\
 "), stdout);
+
+      fputs (_("\
+  --file-system              sync the file systems that contain the path\n\
+  --data                     flush the metadata only if needed later for\n\
+                             data retrieval to be correctly handled\n\
+"), stdout);
+
       fputs (HELP_OPTION_DESCRIPTION, stdout);
       fputs (VERSION_OPTION_DESCRIPTION, stdout);
       emit_ancillary_info (PROGRAM_NAME);
@@ -49,9 +78,61 @@ Force changed blocks to disk, update the super block.\n\
   exit (status);
 }
 
+static bool
+sync_arg (int mode, char *file)
+{
+  int ret = -1;
+  int fd = open (file, O_RDONLY);
+  if (fd < 0)
+    {
+      error (0, errno, _("cannot open %s for reading"), quote (file));
+      return false;
+    }
+
+  switch (mode)
+    {
+    case MODE_FILE:
+      ret = fsync (fd);
+      break;
+
+    case MODE_DATA:
+      ret = fdatasync (fd);
+      break;
+
+#if HAVE_SYNCFS
+    case MODE_FILE_SYSTEM:
+      ret = syncfs (fd);
+      break;
+#endif
+
+    default:
+      error (EXIT_FAILURE, errno, _("invalid mode specified"));
+      break;
+    }
+
+  if (ret < 0)
+    {
+      error (0, errno, _("syncing error"));
+      return false;
+    }
+
+  if (close (fd) < 0)
+    {
+      error (0, errno, _("failed to close %s"), quote (file));
+      return false;
+    }
+
+  return true;
+}
+
 int
 main (int argc, char **argv)
 {
+  bool args_specified;
+  int c, ret;
+  bool arg_data = false, arg_file_system = false;
+  int mode;
+
   initialize_main (&argc, &argv);
   set_program_name (argv[0]);
   setlocale (LC_ALL, "");
@@ -60,14 +141,68 @@ main (int argc, char **argv)
 
   atexit (close_stdout);
 
-  parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE, Version,
-                      usage, AUTHORS, (char const *) NULL);
-  if (getopt_long (argc, argv, "", NULL, NULL) != -1)
-    usage (EXIT_FAILURE);
+  while ((c = getopt_long (argc, argv, "", long_options, NULL))
+         != -1)
+    {
+      switch (c)
+        {
+        case MODE_DATA:
+          arg_data = true;
+          break;
+
+        case MODE_FILE_SYSTEM:
+          arg_file_system = true;
+          break;
+
+        case_GETOPT_HELP_CHAR;
+
+        case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);
+
+        default:
+          usage (EXIT_FAILURE);
+        }
+    }
+
+  args_specified = optind < argc;
+
+  if (arg_data && arg_file_system)
+    {
+      error (EXIT_FAILURE, errno,
+             _("cannot specify both --data and --file-system"));
+    }
+
+  if (!args_specified && arg_data)
+    error (EXIT_FAILURE, errno, _("--data needs at least one argument"));
+
+  if (arg_data)
+    mode = MODE_DATA;
+  else if (! arg_file_system)
+    {
+      mode = args_specified ? MODE_FILE : MODE_SYNC;
+    }
+  else
+    {
+      /* On systems where syncfs is not available, or if no args are specified,
+         fallback to sync.  */
+      if (HAVE_SYNCFS && args_specified)
+          mode = MODE_FILE_SYSTEM;
+      else
+        {
+          mode = MODE_SYNC;
+        }
+    }
 
-  if (optind < argc)
-    error (0, 0, _("ignoring all arguments"));
+  if (mode == MODE_SYNC)
+    {
+      sync ();
+      return EXIT_SUCCESS;
+    }
 
-  sync ();
-  return EXIT_SUCCESS;
+  ret = EXIT_SUCCESS;
+  for (; optind < argc; optind++)
+    {
+      if (! sync_arg (mode, argv[optind]))
+        ret = EXIT_FAILURE;
+    }
+  return ret;
 }
-- 
2.1.0