Re: [PATCH v3 4/4] stat: support statx DONT_SYNC and FORCE_SYNC flags

[Jeff Layton]

On Tue, Apr 16, 2019 at 5:23 PM Jeff Layton <address@hidden> wrote:
>
> Add two new command-line options that set the appropriate hint flags in
> the statx call. These are primarily used with network filesystems to
> indicate what level of cache coherency the application can tolerate.
> The new options are only implemented when built with HAVE_STATX.
>
> * NEWS: mention the enhancements
> * src/stat.c: add new options to control synchronization with server
> ---
>  NEWS       |  8 ++++++++
>  src/stat.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/NEWS b/NEWS
> index 6844228bef9f..f8de29a397ab 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -2,6 +2,14 @@ GNU coreutils NEWS                                    -*- 
> outline -*-
>
>  * Noteworthy changes in release ?.? (????-??-??) [?]
>
> +** New Features
> +
> +   stat now uses the statx() system call to do its bidding when it is
> +   available. It will craft a statx mask with only the needed attributes.
> +   When built with statx support, it also supports new command line
> +   options to control synchronization with the server on network
> +   filesytems.
> +
>  ** Bug fixes
>
>    factor again outputs immediately when stdout is a tty but stdin is not.
> diff --git a/src/stat.c b/src/stat.c
> index 2bbc75792b5a..754c4eecb184 100644
> --- a/src/stat.c
> +++ b/src/stat.c
> @@ -201,6 +201,10 @@ static struct option const long_options[] =
>    {"format", required_argument, NULL, 'c'},
>    {"printf", required_argument, NULL, PRINTF_OPTION},
>    {"terse", no_argument, NULL, 't'},
> +#if HAVE_STATX && defined STATX_INO
> +  {"dont-sync", no_argument, NULL, 'D'},
> +  {"force-sync", no_argument, NULL, 'F'},
> +#endif
>    {GETOPT_HELP_OPTION_DECL},
>    {GETOPT_VERSION_OPTION_DECL},
>    {NULL, 0, NULL, 0}
> @@ -1200,6 +1204,12 @@ do_statfs (char const *filename, char const *format)
>  }
>
>  #if HAVE_STATX && defined STATX_INO
> +/* Ask statx to avoid syncing? */
> +static bool dont_sync;
> +
> +/* Ask statx to force sync? */
> +static bool force_sync;
> +
>  struct printarg {
>    struct statx *stx;
>    struct stat *st;
> @@ -1472,6 +1482,11 @@ do_stat (char const *filename, char const *format, 
> char const *format2)
>        flags = AT_SYMLINK_NOFOLLOW;
>      }
>
> +  if (dont_sync)
> +    flags |= AT_STATX_DONT_SYNC;
> +  else if (force_sync)
> +    flags |= AT_STATX_FORCE_SYNC;
> +
>    fd = statx(fd, pathname, flags, format_to_mask(format), &stx);
>    if (fd < 0)
>      {
> @@ -1800,6 +1815,12 @@ Display file or file system status.\n\
>    -L, --dereference     follow links\n\
>    -f, --file-system     display file system status instead of file status\n\
>  "), stdout);
> +#if HAVE_STATX && defined STATX_INO
> +      fputs (_("\
> +  -D, --dont-sync       don't synchronize with server (for network fs')\n\
> +  -F, --force-sync      force synchronization with server (for network 
> fs')\n\
> +"), stdout);
> +#endif
>        fputs (_("\
>    -c  --format=FORMAT   use the specified FORMAT instead of the default;\n\
>                            output a newline after each use of FORMAT\n\
> @@ -1916,7 +1937,7 @@ main (int argc, char *argv[])
>
>    atexit (close_stdout);
>
> -  while ((c = getopt_long (argc, argv, "c:fLt", long_options, NULL)) != -1)
> +  while ((c = getopt_long (argc, argv, "c:DfFLt", long_options, NULL)) != -1)

Hmm...we'll probably want to use a different string if HAVE_STATX is
not defined. Fixed in my tree, but I'll refrain from resending until I
get some other feedback on this.

BTW, ping on this? I'd like to know whether this patchset is
reasonable, as I was planning to use this functionality to write some
tests for cephfs.

Thanks,

>      {
>        switch (c)
>          {
> @@ -1932,6 +1953,26 @@ main (int argc, char *argv[])
>            trailing_delim = "\n";
>            break;
>
> +#if HAVE_STATX && defined STATX_INO
> +       case 'D':
> +         if (force_sync)
> +            {
> +              error (0, 0, _("--dont-sync and --force-sync are mutually 
> exclusive"));
> +              usage (EXIT_FAILURE);
> +            }
> +         dont_sync = true;
> +         break;
> +
> +       case 'F':
> +         if (dont_sync)
> +            {
> +              error (0, 0, _("--dont-sync and --force-sync are mutually 
> exclusive"));
> +              usage (EXIT_FAILURE);
> +            }
> +         force_sync = true;
> +         break;
> +#endif
> +
>          case 'L':
>            follow_links = true;
>            break;
> --
> 2.20.1
>
>


-- 
Jeff Layton <address@hidden>