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

[Jeff Layton]

On Mon, 2019-05-13 at 09:18 -0400, Jeff Layton wrote:
> Add a new --cache= command-line option that sets 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 option is only implemented when built with
> HAVE_STATX.
> 
> * NEWS: mention the enhancements
> * src/stat.c: add new option to control synchronization with server
> ---
>  NEWS       |  7 ++++++
>  src/stat.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 80 insertions(+), 1 deletion(-)
> 
> diff --git a/NEWS b/NEWS
> index 12c864dcc9ad..bac18edfc052 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -2,6 +2,13 @@ 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 a new --cached=
> +   option to control synchronization with the server on network file sytems.
> +
>  ** Bug fixes
>  
>    df now correctly parses the /proc/self/mountinfo file for unusual entries
> diff --git a/src/stat.c b/src/stat.c
> index 4b4dc52e5ca6..66446532ad4d 100644
> --- a/src/stat.c
> +++ b/src/stat.c
> @@ -194,6 +194,23 @@ enum
>    PRINTF_OPTION = CHAR_MAX + 1
>  };
>  
> +enum cached_mode
> +{
> +  cached_default,
> +  cached_never,
> +  cached_always
> +};
> +
> +static char const *const cached_args[] =
> +{
> +  "default", "never", "always", NULL
> +};
> +
> +static enum cached_mode const cached_modes[] =
> +{
> +  cached_default, cached_never, cached_always
> +};
> +
>  static struct option const long_options[] =
>  {
>    {"dereference", no_argument, NULL, 'L'},
> @@ -201,6 +218,9 @@ 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
> +  {"cached", required_argument, NULL, 0},
> +#endif
>    {GETOPT_HELP_OPTION_DECL},
>    {GETOPT_VERSION_OPTION_DECL},
>    {NULL, 0, NULL, 0}
> @@ -1200,6 +1220,15 @@ do_statfs (char const *filename, char const *format)
>  }
>  
>  #if HAVE_STATX && defined STATX_INO
> +/* Allowed command-line options */
> +static const char *optstring = "c:fLt0:";

We don't need to conditionally compile in a different optstring since
we've dropped the -C short argument. I've got this fixed in my tree now,
but I'll wait to send it out in case other changes are required:

    
https://github.com/jtlayton/coreutils/commit/fdf4749c724af1ccd89609b098871cd49e8827d9

> +
> +/* 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 +1501,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)
>      {
> @@ -1492,6 +1526,9 @@ error (0, errno, _("cannot statx %s"), quoteaf 
> (filename));
>    return ! fail;
>  }
>  #else /* HAVE_STATX && defined STATX_INO */
> +/* Allowed command-line options */
> +static const char *optstring = "c:fLt";
> +
>  static struct timespec
>  get_birthtime (int fd, char const *filename, struct stat const *st)
>  {
> @@ -1800,6 +1837,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 (_("\
> +      --cached=MODE     specify whether and how to use cached values;\n\
> +                          (typically useful for network file systems)\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\
> @@ -1811,6 +1854,16 @@ Display file or file system status.\n\
>        fputs (HELP_OPTION_DESCRIPTION, stdout);
>        fputs (VERSION_OPTION_DESCRIPTION, stdout);
>  
> +#if HAVE_STATX && defined STATX_INO
> +      fputs (_("\n\
> +The valid MODE arguments to --cached are:\n\
> +\n\
> +  always:  always use cached values\n\
> +  never:   never use cached values\n\
> +  default: leave it up to the underlying file system\n\
> +"), stdout);
> +#endif
> +
>        fputs (_("\n\
>  The valid format sequences for files (without --file-system):\n\
>  \n\
> @@ -1916,7 +1969,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, optstring, long_options, NULL)) != -1)
>      {
>        switch (c)
>          {
> @@ -1944,6 +1997,25 @@ main (int argc, char *argv[])
>            terse = true;
>            break;
>  
> +#if HAVE_STATX && defined STATX_INO
> +        case 0:
> +          switch (XARGMATCH ("--cached", optarg, cached_args, cached_modes))
> +            {
> +              case cached_never:
> +                force_sync = true;
> +                dont_sync = false;
> +                break;
> +              case cached_always:
> +                force_sync = false;
> +                dont_sync = true;
> +                break;
> +              case cached_default:
> +                force_sync = false;
> +                dont_sync = false;
> +            }
> +          break;
> +#endif
> +
>          case_GETOPT_HELP_CHAR;
>  
>          case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);

-- 
Jeff Layton <address@hidden>