GNOME Bugzilla – Bug 573837
gvfs streams lack truncate support
Last modified: 2013-12-06 15:31:47 UTC
Please describe the problem: There have been many problems in the past with OOo's ability to use gvfs/gio natively to save to network fileshares. One of the issues is that it cannot save to sftp properly for some reason. So I tried saving directly to the gvfs-fuse location eg ~/.gvfs/some/path/to/file/ and noticed even saving that way, which bypasses all of OOo's internal gvfs/gio code still does not work correctly. It gives this error message: " Error saving the document (document name): Object not accessible. The object cannot be accessed due to insufficient user rights. " I tested on 1.1.7 on Ubuntu 9.04 (Jaunty) with OOo 3.0.1. The following bug looks related as I have successfully managed to save to a system whose uid/gid matched up properly: http://bugzilla.gnome.org/show_bug.cgi?id=531598 Steps to reproduce: 1. Start OOo 2. Create a document 3. Attempt to save to the gvfs fuse location (where your uid/gid does not match your systems) to bypass any internal gnome-vfs/gvfs/gio support in OOo. 4. Notice it fails with an error message. Actual results: It fails with an error message. Expected results: It should work. Does this happen every time? It seems to happen anytime uid/gid does not match, but I have not done extensive testing. Other information: I frequent freenode/oftc as 'calc' and can get on gnome irc for interactive testing if needed.
Interesting enough I get the same error trying to write to a smb gvfs-fuse mount as well.
Ok, the first time it tries to save it fails and leaves a file -rwx------ 1 ccheney ccheney 70 2009-03-02 17:33 .~lock.test-gvfs-fuse-smb.odt# Then if you try to save a second time it works and two files show up: -rwx------ 1 ccheney ccheney 70 2009-03-02 17:33 .~lock.test-gvfs-fuse-smb.odt# -rwx------ 1 ccheney ccheney 7901 2009-03-02 17:34 test-gvfs-fuse-smb.odt I am attaching a strace log of the initial failure.
Created attachment 129900 [details] strace of initial failure to save
I'm not certain if I am reading this correctly but it looks like it might be failing due to lack of ftruncate support? I thought that was one of the things fixed in 1.1.7? -- Snippet from above strace log: 24804 stat("/home/ccheney/.gvfs/personal on 10.0.0.1/.~lock.test-gvfs-fuse-smb.odt#", 0x7fff08496dd0) = -1 ENOENT (No such file or directory) 24804 open("/home/ccheney/.gvfs/personal on 10.0.0.1/.~lock.test-gvfs-fuse-smb.odt#", O_RDWR|O_CREAT|O_EXCL, 0666) = 46 24804 lseek(45, 0, SEEK_SET) = 0 24804 read(45, ",ccheney,x200,02.03.2009 17:36,fi"..., 32768) = 70 24804 close(45) = 0 24804 write(46, ",ccheney,x200,02.03.2009 17:36,fi"..., 70) = 70 24804 ftruncate(46, 70) = -1 EOPNOTSUPP (Operation not supported) 24804 fstat(46, {st_mode=S_IFREG|0700, st_size=70, ...}) = 0 24804 close(46) = 0 24804 access("/home/ccheney/.gvfs/personal on 10.0.0.1/.~lock.test-gvfs-fuse-smb.odt#", F_OK) = 0 24804 lstat("/home/ccheney/.gvfs/personal on 10.0.0.1/.~lock.test-gvfs-fuse-smb.odt#", {st_mode=S_IFREG|0700, st_size=70, ...}) = 0
Thats truncating to something other than 0, which is not supported on all backends.
Would it be possible to support this somehow? It appears that at least OOo relies on this behavior and possibly other applications could as well.
Alex mentioned on irc that many backends could not support this type of truncation but that some could but probably do not at the moment. So my request is that for the backends that can support it that it would be nice to have this support so that OOo (and other apps) will work when they attempt to save to those backends.
I think this feature could be 'emulated' even on backends that don't actually support it by downloading the file then truncating it and then sending it back to the server. Yuck, I agree, but would help make gvfs-fuse more posix compliant.
The most commonly used backend is probably the smb backend and the kernel version of smbfs works for ftruncate != 0 so this one at least could probably be fixed (I think).
chris: Its very hard to emulate posix compliance on non-posix system. One approach like you say is to download and use a local file to emulate operations on. However, then you suddenly get a problem wrt stat() on the path vs the local copy of the file being different (size and whatnot). Furthermore, you can run into issues with stale caches when updates can happen from other users too. Its just not generally possible to have posix semantics on non-local filesystems...
Alex: In this particular instance it would be useful to have ftruncate() support for smb as the linux kernel also supports this. Though perhaps it would not be possible for the rest of the filesystems that gvfs supports due to the issues you have brought up. I am looking into why OOo actually needs ftruncate() to see if I can get rid of its need for it. For whatever reason it fails to save the first time but passes on the second time which seems to indicate to me that it may not actually need to use ftruncate at all.
Alex, From what I can tell in the OOo case it appears to be writing out 70 bytes of data to the file then ftruncate() it to 70 bytes. It appears it doesn't really need to do the ftruncate in this case since it is effectively a NOP but it does anyway since it is uses a generic file writing class. Perhaps it would be possible for gvfs to accept and NOP a truncate to the same size as the file actually happens to be?
chris: That may well be possible. We need to be able to tell the current length, which is possible at least for some backends with query_info on the stream, or it could possibly also be tracked by the fuse backend itself.
Created attachment 129998 [details] [review] Succed when truncated to current size (if we know it)
chris: Can you test that patch?
Your patch allows OOo to save without problem to both sftp and smb, probably other backends as well but those were the only two I had already setup locally. Thanks! Chris
The patch is commited, but we should still add full support for ftruncate.
Created attachment 143306 [details] [review] nop truncate That patch didn't work for me when I backported it to gvfs-1.0.2, but with this patch things are good. Maybe there was a bug in gio reading stream sizes in my version and with current gio it would work without my patch?
Created attachment 257497 [details] [review] Implement truncate support for output streams Backends receive a TRUNCATE message which contains a size parameter. Truncation is signaled with a TRUNCATED message (which contains no other useful information). In more detail: Change the dbus reply of OpenForWrite to include a can_truncate boolean which gets stored in GDaemonFileOutputStream. Implement the can_truncate and truncate_fn GDaemonFileOutputStream methods. Add two new message types to the daemon socket protocol: G_VFS_DAEMON_SOCKET_PROTOCOL_REQUEST_TRUNCATE G_VFS_DAEMON_SOCKET_PROTOCOL_REPLY_TRUNCATED Add a new job type, GVfsJobTruncate. Add two new methods to GVfsBackend which backend classes can implement: truncate and try_truncate
Created attachment 257498 [details] [review] localtest: Implement truncate support for output streams
Created attachment 257499 [details] [review] Require libsmbclient from Samba 3.4.0 or higher Also remove the old libsmb-compat.h header file.
Created attachment 257500 [details] [review] smb: Implement truncate support for output streams
Created attachment 257502 [details] [review] sftp: Implement truncate support for output streams
Created attachment 257503 [details] [review] afp: Implement truncate support for output streams
The attached patch series implements truncate support for gvfs and in particular, for the localtest, smb, sftp and afp backends.
Review of attachment 257497 [details] [review]: ::: common/org.gtk.vfs.xml @@ +197,3 @@ <arg type='h' name='fd_id' direction='out'/> <arg type='b' name='can_seek' direction='out'/> + <arg type='b' name='can_truncate' direction='out'/> FYI, breaking the d-bus API is always painful, even though we manifest it as private with right to break anytime. But things will be broken until all clients and the daemon are restarted. Furthemore, mixing environments either chrooted or in separate prefixes will have the same problem, until all the versions are same. This is a small change that may not be that visible though.
(In reply to comment #26) > Review of attachment 257497 [details] [review]: > > ::: common/org.gtk.vfs.xml > @@ +197,3 @@ > <arg type='h' name='fd_id' direction='out'/> > <arg type='b' name='can_seek' direction='out'/> > + <arg type='b' name='can_truncate' direction='out'/> > > FYI, breaking the d-bus API is always painful, even though we manifest it as > private with right to break anytime. But things will be broken until all > clients and the daemon are restarted. Furthemore, mixing environments either > chrooted or in separate prefixes will have the same problem, until all the > versions are same. > > This is a small change that may not be that visible though. I did it the same way that can_seek was implemented. I'm not sure if there is a better way, but I think breaking the (private) API during a cycle should be acceptable, if not ideal. Hopefully it wouldn't be an issue an end user would ever come across, whereas unexpected lacking truncate support could be.
Created attachment 257598 [details] [review] afc: Implement truncate support for output streams
Created attachment 258527 [details] [review] Implement truncate support for output streams Backends receive a TRUNCATE message which contains a size parameter. Truncation is signaled with a TRUNCATED message (which contains no other useful information). In more detail: Add a new dbus method, OpenForWriteFlags, which has a flags parameter to implement can_seek and can_truncate. These flags are used in GDaemonFileOutputStream. Compatability with old clients is maintained. Implement the can_truncate and truncate_fn GDaemonFileOutputStream methods. Add two new message types to the daemon socket protocol: G_VFS_DAEMON_SOCKET_PROTOCOL_REQUEST_TRUNCATE G_VFS_DAEMON_SOCKET_PROTOCOL_REPLY_TRUNCATED Add a new job type, GVfsJobTruncate. Add two new methods to GVfsBackend which backend classes can implement: truncate and try_truncate
Created attachment 258528 [details] [review] localtest: Implement truncate support for output streams
Created attachment 258529 [details] [review] Require libsmbclient from Samba 3.4.0 or higher Also remove the old libsmb-compat.h header file.
Created attachment 258530 [details] [review] smb: Implement truncate support for output streams
Created attachment 258531 [details] [review] sftp: Implement truncate support for output streams
Created attachment 258532 [details] [review] afp: Implement truncate support for output streams
Created attachment 258533 [details] [review] afc: Implement truncate support for output streams
I have updated the patch series based on feedback from Tomas (comment 26) and Alex (https://mail.gnome.org/archives/gvfs-list/2013-October/msg00010.html). The main change is that a new dbus method is added instead of changing the old one, maintaining compatibility with older clients.
Created attachment 260045 [details] [review] Implement truncate support for output streams Backends receive a TRUNCATE message which contains a size parameter. Truncation is signaled with a TRUNCATED message (which contains no other useful information). In more detail: Add a new dbus method, OpenForWriteFlags, which has a flags parameter to implement can_seek and can_truncate. These flags are used in GDaemonFileOutputStream. Compatability with old clients is maintained. Implement the can_truncate and truncate_fn GDaemonFileOutputStream methods. Add two new message types to the daemon socket protocol: G_VFS_DAEMON_SOCKET_PROTOCOL_REQUEST_TRUNCATE G_VFS_DAEMON_SOCKET_PROTOCOL_REPLY_TRUNCATED Add a new job type, GVfsJobTruncate. Add two new methods to GVfsBackend which backend classes can implement: truncate and try_truncate
Created attachment 260046 [details] [review] localtest: Implement truncate support for output streams
Created attachment 260047 [details] [review] Require libsmbclient from Samba 3.4.0 or higher Also remove the old libsmb-compat.h header file.
Created attachment 260048 [details] [review] smb: Implement truncate support for output streams
Created attachment 260049 [details] [review] sftp: Implement truncate support for output streams
Created attachment 260050 [details] [review] afp: Implement truncate support for output streams
Created attachment 260051 [details] [review] afc: Implement truncate support for output streams
Created attachment 260052 [details] [review] mtp: Implement truncate support for output streams
Created attachment 260053 [details] [review] gphoto2: Implement truncate support for output streams
The updated patch series: * Rebased against master to fix a few conflicts * Adds support for the mtp and gphoto2 backends. I think I've now implemented backend support for all the backends that could support truncate. * Updates the coding style to better match the one used by GVfs. It would be nice to get this in soon. The patches are mostly fairly straightforward; perhaps the only interesting part is the addition of a new dbus method maintaining backwards compatibility as per https://mail.gnome.org/archives/gvfs-list/2013-October/msg00015.html
Review of attachment 260045 [details] [review]: otherwise this looks good ::: common/gvfsdaemonprotocol.h @@ +22,3 @@ +/* Flags for the OpenForWriteFlags method */ +#define OPEN_FOR_WRITE_FLAG_CAN_SEEK 0 +#define OPEN_FOR_WRITE_FLAG_CAN_TRUNCATE 1 Its more typically to define these as bitmasks. I.e. 1<<0 and 1<<1 Then you can just use it like: if (foo & OPEN_FOR_WRITE_FLAG_CAN_SEEK) bar()
Review of attachment 260046 [details] [review]: ack
Review of attachment 260047 [details] [review]: ack
Review of attachment 260048 [details] [review]: ack
Review of attachment 260049 [details] [review]: ack
Review of attachment 260050 [details] [review]: ack
Review of attachment 260051 [details] [review]: ack
Review of attachment 260052 [details] [review]: ack
Review of attachment 260053 [details] [review]: ::: daemon/gvfsbackendgphoto2.c @@ +3241,3 @@ + + /* ensure we have enough room */ + if (handle->cursor + size > handle->allocated_size) This doesn't seem right. size is an abosolute offset here, and should not be affected by the current file position.
Thanks. Pushed to master (after addressing the comments above) as c3b6615e9..d274d6146.
Review of attachment 260050 [details] [review]: ::: daemon/gvfsbackendafp.c @@ +796,3 @@ + g_vfs_job_failed_from_error (G_VFS_JOB (job), err); + g_error_free (err); + afp_handle_free (afp_handle); Should the handle really be freed here? Isn't it possible for the handle to be used in other operations even though truncate failed?
(In reply to comment #57) > Review of attachment 260050 [details] [review]: > > ::: daemon/gvfsbackendafp.c > @@ +796,3 @@ > + g_vfs_job_failed_from_error (G_VFS_JOB (job), err); > + g_error_free (err); > + afp_handle_free (afp_handle); > > Should the handle really be freed here? Isn't it possible for the handle to be > used in other operations even though truncate failed? True, I'll just remove that line.
Pushed a fix for afp as 91cf41328a2df886ec59c533ffa4991f92059e59. Thanks!