[Libguestfs] [PATCH] fuse: provide a stub "flush" implementation (RHBZ#660687).

[Richard W.M. Jones]

On Thu, Dec 12, 2013 at 04:28:31PM +0100, Pino Toscano wrote:
> It seems that FUSE can invoke flush to make sure the pending changes
> (e.g. to the attributes) of a file are set. Since a missing flush
> implementation is handled as if it were returning ENOSYS, this can cause
> issues later.
> 
> To overcome this, just provide a stub implementation which does nothing,
> since we have nothing to do and don't want to have FUSE error out.
> 
> Furthermore, uncomment the timestamp checks in test-fuse.sh, since now
> they should be working fine.
> ---
>  fuse/test-fuse.sh | 23 +++++++++++------------
>  src/fuse.c        | 15 +++++++++++++++
>  2 files changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/fuse/test-fuse.sh b/fuse/test-fuse.sh
> index f1e03d0..30b3c31 100755
> --- a/fuse/test-fuse.sh
> +++ b/fuse/test-fuse.sh
> @@ -220,18 +220,17 @@ if truncate --help >/dev/null 2>&1; then
>      rm -f truncated
>  fi
>  
> -# Disabled because of RHBZ#660687 on Debian.
> -# stage Checking utimens and timestamps
> -# for ts in 12345 1234567 987654321; do
> -#     # NB: It's not possible to set the ctime with touch.
> -#     touch -a -d @$ts timestamp
> -#     [ "$(stat -c %X timestamp)" -eq $ts ]
> -#     touch -m -d @$ts timestamp
> -#     [ "$(stat -c %Y timestamp)" -eq $ts ]
> -#     touch    -d @$ts timestamp
> -#     [ "$(stat -c %X timestamp)" -eq $ts ]
> -#     [ "$(stat -c %Y timestamp)" -eq $ts ]
> -# done
> +stage Checking utimens and timestamps
> +for ts in 12345 1234567 987654321; do
> +    # NB: It's not possible to set the ctime with touch.
> +    touch -a -d @$ts timestamp
> +    [ "$(stat -c %X timestamp)" -eq $ts ]
> +    touch -m -d @$ts timestamp
> +    [ "$(stat -c %Y timestamp)" -eq $ts ]
> +    touch    -d @$ts timestamp
> +    [ "$(stat -c %X timestamp)" -eq $ts ]
> +    [ "$(stat -c %Y timestamp)" -eq $ts ]
> +done
>  
>  stage Checking writes
>  cp hello.txt copy.txt
> diff --git a/src/fuse.c b/src/fuse.c
> index 967a744..748b933 100644
> --- a/src/fuse.c
> +++ b/src/fuse.c
> @@ -876,6 +876,20 @@ mount_local_removexattr(const char *path, const char *name)
>    return 0;
>  }
>  
> +static int
> +mount_local_flush(const char *path, struct fuse_file_info *fi)
> +{
> +  DECL_G ();
> +  DEBUG_CALL ("%s", path);
> +
> +  /* Just a stub. This method is called whenever FUSE wants to flush the
> +   * pending changes (f.ex. to attributes) to a file.  Since we don't have
> +   * anything to do and don't want FUSE to think something went badly,
> +   * just return 0.
> +   */
> +  return 0;
> +}
> +
>  static struct fuse_operations mount_local_operations = {
>    .getattr	= mount_local_getattr,
>    .access	= mount_local_access,
> @@ -902,6 +916,7 @@ static struct fuse_operations mount_local_operations = {
>    .getxattr	= mount_local_getxattr,
>    .listxattr	= mount_local_listxattr,
>    .removexattr	= mount_local_removexattr,
> +  .flush        = mount_local_flush,
>  };
>  
>  int
> -- 
> 1.8.3.1

I had to check what file_operations.flush is used for (to check that
it's not fsync), but as you say it's just used to indicate that the
file descriptor has been closed.

So, ACK.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org