[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified
From: |
Giuseppe Scrivano |
Subject: |
bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified |
Date: |
Mon, 26 Jan 2015 22:27:31 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.4 (gnu/linux) |
Pádraig Brady <address@hidden> writes:
> On 26/01/15 08:36, Giuseppe Scrivano wrote:
>> Pádraig Brady <address@hidden> 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 <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.
* 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
address@hidden (@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
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 system that contains the path.
+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/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
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, (continued)
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Giuseppe Scrivano, 2015/01/25
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Pádraig Brady, 2015/01/25
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Bernhard Voelker, 2015/01/25
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Pádraig Brady, 2015/01/25
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Giuseppe Scrivano, 2015/01/26
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Pádraig Brady, 2015/01/26
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified,
Giuseppe Scrivano <=
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Pádraig Brady, 2015/01/26
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Giuseppe Scrivano, 2015/01/27
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Pádraig Brady, 2015/01/27
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Bernhard Voelker, 2015/01/28
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Pádraig Brady, 2015/01/28
bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Eric Blake, 2015/01/26