Pádraig Brady <[email protected]> writes: > On 26/01/15 08:36, Giuseppe Scrivano wrote: >> Pádraig Brady <[email protected]> writes: >> >>> On 25/01/15 18:05, Bernhard Voelker wrote: >>>> On 01/25/2015 06:41 PM, Pádraig Brady wrote: >>>>> So we have: fdatasync < fsync < syncfs < sync >>>>> referring to:: file data, file data + metadata, file system, all file >>>>> systems >>>> >>>>> [...] >>>> >>>>> I'd be incline to go with the _what_ interface above. >>>> >>>> Either way, I think it's important to document sync is falling back >>>> to the bigger hammer if the smaller failed. >>>> ... or shouldn't do sync this? >>> >>> It should fall back where possible. >>> >>> Now there is a difference between the file and file system(s) interfaces >>> in that the former can return EIO error for example, while the latter >>> are specified to always return success. You wouldn't fall back to >>> a syncfs() if an fsync() gave an EIO for example. Also gnulib >>> guarantees that fsync() and fdatasync() are available, so I wouldn't >>> fallback from file -> file system interfaces, nor between file interfaces. >> >> one risk here is when multiple arguments are specified and the fsync >> will return EIO more than once, we will fallback to syncfs multiple >> times. Couldn't in this case a single sync be a better choice? > > I was saying we shouldn't fall back from fsync() to syncfs(). > Just process each argument. Diagnose any errors and EXIT_FAILURE > if there was any error?
sorry for misunderstanding that. I've worked out a new version that includes these suggestions, also since now the user can explicitly ask for the sync mechanism to use, I agree with you and we should raise an error if something goes wrong. Since sync is completely different now, I took the freedom to add myself to the AUTHORS, feel free to drop this part if you disagree. Regards, Giuseppe >From 0dbc5ce9c78bc97ec5a678803270767ad9980618 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano <[email protected]> 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. * doc/coreutils.texi (sync invocation): Document the new feature. * src/sync.c: Include "quote.h". (AUTHORS): Include myself. (MODE_FILE, MODE_FILE_DATA, MODE_FILE_SYSTEM): New enum values. (long_options): Define. (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 + src/sync.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++++---- 5 files changed, 131 insertions(+), 11 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..c99b8ed 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 -@option{--version} (@pxref{Common options}). +If any argument is specified then only the specified paths will be +synchronized. It uses internally the syscall fsync(2) on each of them. + +If at least one path is specified, it is possible to change the +synchronization policy with the following options. Also see +@ref{Common options}. + +@table @samp +@item --data +@opindex --data +Do not synchronize the file metadata unless it is required to maintain +data integrity. It uses the syscall fdatasync(2). + +@item --file-system +@opindex --file-system +Synchronize all the I/O waiting for the file system that contains the path. +It uses the syscall syncfs(2). +@end 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/src/sync.c b/src/sync.c index e9f4d7e..4a15d75 100644 --- a/src/sync.c +++ b/src/sync.c @@ -23,12 +23,30 @@ #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_FILE_DATA, + MODE_FILE_SYSTEM +}; + +static struct option const long_options[] = +{ + {"data", no_argument, NULL, MODE_FILE_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 +55,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); @@ -52,6 +80,11 @@ Force changed blocks to disk, update the super block.\n\ int main (int argc, char **argv) { + bool args_specified; + int c; + int mode = MODE_FILE; + int (*sync_func)(int) = NULL; + initialize_main (&argc, &argv); set_program_name (argv[0]); setlocale (LC_ALL, ""); @@ -60,12 +93,79 @@ 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_FILE_DATA: + mode = MODE_FILE_DATA; + break; + + case MODE_FILE_SYSTEM: + mode = MODE_FILE_SYSTEM; + break; + + case_GETOPT_HELP_CHAR; + + case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS); + + default: + usage (EXIT_FAILURE); + } + } + + args_specified = optind < argc; + + if (! args_specified) + goto sync; + + if (!args_specified && mode != MODE_FILE) + error (EXIT_FAILURE, errno, _("mode specified without any argument")); + + switch (mode) + { + case MODE_FILE_DATA: + sync_func = fdatasync; + break; + + case MODE_FILE: + sync_func = fsync; + break; +#if HAVE_SYNCFS + /* On systems where syncfs is not available, fallback to sync. */ + case MODE_FILE_SYSTEM: + sync_func = syncfs; + break; +#endif + default: + goto sync; + } + + while (optind < argc) + { + int fd = open (argv[optind], O_RDONLY); + if (fd < 0) + { + error (EXIT_FAILURE, errno, _("cannot open %s for reading"), + quote (argv[optind])); + } + + if (sync_func (fd) < 0) + error (EXIT_FAILURE, errno, _("syncing error")); + + if (close (fd) < 0) + { + error (EXIT_FAILURE, errno, _("failed to close %s"), + quote (argv[optind])); + } + + optind++; + } + return EXIT_SUCCESS; - if (optind < argc) +sync: + if (args_specified) error (0, 0, _("ignoring all arguments")); sync (); -- 2.1.0
