Pino Toscano
2013-Dec-12 15:28 UTC
[Libguestfs] [PATCH] fuse: provide a stub "flush" implementation (RHBZ#660687).
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
Richard W.M. Jones
2013-Dec-12 15:48 UTC
Re: [Libguestfs] [PATCH] fuse: provide a stub "flush" implementation (RHBZ#660687).
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.1I 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
Pino Toscano
2014-Jan-15 17:59 UTC
Re: [Libguestfs] [PATCH] fuse: provide a stub "flush" implementation (RHBZ#660687).
Hi, On Thursday 12 December 2013 16:28:31 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.Apparently this way not the right fix for rh#660687 (although the addition of the patch did not harm). I should have hopefully found the right cause of it, will follow-up with the fix. -- Pino Toscano
Pino Toscano
2014-Jan-15 18:02 UTC
[Libguestfs] [PATCH] fuse: clear stat structs (RHBZ#660687).
Not all the fields of struct stat are actually filled by us. This caused
rubbish to appear in the microseconds fields, which were then used as
base when changing atime/ctime (with e.g. touch), triggering EINVAL by
futimens/utimensat when those rubbish values were out of the range
allowed for microseconds.
---
src/fuse.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/fuse.c b/src/fuse.c
index 288c02a..dd4f139 100644
--- a/src/fuse.c
+++ b/src/fuse.c
@@ -175,6 +175,7 @@ mount_local_readdir (const char *path, void *buf,
fuse_fill_dir_t filler,
if (ss->val[i].ino >= 0) {
struct stat statbuf;
+ memset (&statbuf, 0, sizeof statbuf);
statbuf.st_dev = ss->val[i].dev;
statbuf.st_ino = ss->val[i].ino;
statbuf.st_mode = ss->val[i].mode;
@@ -255,6 +256,7 @@ mount_local_getattr (const char *path, struct stat *statbuf)
if (r == NULL)
RETURN_ERRNO;
+ memset (statbuf, 0, sizeof *statbuf);
statbuf->st_dev = r->dev;
statbuf->st_ino = r->ino;
statbuf->st_mode = r->mode;
--
1.8.3.1
Seemingly Similar Threads
- [PATCH] fuse: provide a stub "flush" implementation (RHBZ#660687).
- [PATCH v2] New APIs: mount-local and umount-local using FUSE
- [PATCH 0/3] Enable FUSE support in the API via 'mount-local' call.
- [PATCH v3] New APIs: mount-local, mount-local-run and umount-local using FUSE
- [PATCH] New APIs: Implement stat calls that return nanosecond timestamps (RHBZ#1144891).