Bug 220185 - >> operator does not append in Fuse mounts
Summary: >> operator does not append in Fuse mounts
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: Conrad Meyer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-21 13:53 UTC by Ben RUBSON
Modified: 2017-06-28 13:58 UTC (History)
1 user (show)

See Also:


Attachments
Completely untested patch for the issue (3.33 KB, patch)
2017-06-25 02:05 UTC, Conrad Meyer
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ben RUBSON 2017-06-21 13:53:30 UTC
Hello,

So as shown below, >> operator does not seem to correctly append in Fuse mounts.
No issue however opening files in >> mode with Perl.
Tested with Fuse FS such as EncFS and SSHFS.

Initial EncFS issue :
https://github.com/vgough/encfs/issues/334

Using direct_io (or r317273) does not help.

# uname -sr
FreeBSD 11.0-RELEASE-p8

# echo -e "line1\nline2\nline3" > /my_fuse_mount/test.txt
# cat /my_fuse_mount/test.txt 
line1
line2
line3
# echo line4 >> /my_fuse_mount/test.txt 
# cat /my_fuse_mount/test.txt 
line4
line2
line3

# cat test.pl 
open(my $out, ">", "/my_fuse_mount/perltest.txt");
print($out "line1\nline2\nline3\n");
close($out);
open($out, ">>", "/my_fuse_mount/perltest.txt");
print($out "line4\n");
close($out);
# 
# perl test.pl 
# cat /my_fuse_mount/perltest.txt 
line1
line2
line3
line4

Thank you !

Ben
Comment 1 Conrad Meyer freebsd_committer freebsd_triage 2017-06-25 02:03:50 UTC
I can reproduce this problem with lklfuse (ext4 image).  Exact same steps as Ben provided.

$ truss sh -c 'echo line4 >> test.txt'
...
openat(AT_FDCWD,"test.txt",O_WRONLY|O_APPEND|O_CREAT,0666) = 3 (0x3)
dup2(0x3,0x1)                                    = 1 (0x1)
close(3)                                         = 0 (0x0)
write(1,"line4\n",6)                             = 6 (0x6)

From lklfuse -o debug:
unique: 2, opcode: LOOKUP (1), nodeid: 1, insize: 49, pid: 7005
LOOKUP /test.txt
getattr /test.txt
   NODEID: 2
   unique: 2, success, outsize: 136
unique: 3, opcode: OPEN (14), nodeid: 2, insize: 48, pid: 7005
open flags: 0x1 /test.txt
   open[0] flags: 0x1 /test.txt
   unique: 3, success, outsize: 32

// Flag 0x1 == O_WRONLY.

I'm not sure if FUSE clients are supposed to handle O_APPEND or if FUSE is supposed to emulate it on their behalf.  Either way, it seems to be getting dropped.  That said — it doesn't look like UFS does anything special with the APPEND flag either.  After all, multiple file handles can reference the same vnode at different offsets.  So the offset should be a property of the file handle, not the vnode.

This suggests some higher layer is misbehaving; not individual FUSE filesystems.  (Not really surprising, I know.)

More from lklfuse -o debug:

unique: 4, opcode: GETATTR (3), nodeid: 2, insize: 40, pid: 7005
getattr /test.txt
   unique: 4, success, outsize: 112
unique: 5, opcode: WRITE (16), nodeid: 2, insize: 70, pid: 7005
write[0] 6 bytes to 0 flags: 0x0
   write[0] 6 bytes to 0
   unique: 5, success, outsize: 24

// Note: "to 0" (offset 0), "flags: 0x0" (no IO_APPEND)

That's a problem.

kern_openat() installs O_APPEND from open(2) into the struct file referring to test.txt.  Then in vn_write(), f_flag means that ioflag has IO_APPEND added.  This is passed into VOP_WRITE.  What does fs/fuse do with that?

Well, there's a check in fuse_write_biobackend() that sets the uio offset to the end of the file if that flag is present.

Either we aren't hitting that path, or the uio's offset isn't getting communicated to fuse filesystems.

VOP_WRITE -> fuse_vnop_write -> fuse_iop_dispatch().  From there we enter fuse_write_directbackend for IO_DIRECT uio's, or fuse_write_biobackend otherwise.

The DIRECT backend completely ignores IO_APPEND.  I'd guess we're not hitting that path as the open did not use O_DIRECT.

I'm pretty confused by what fuse_write_biobackend() is doing, but it looks like it may be correct.  Maybe we're seeing the DIRECT path.

...haha, yup.  fuse_vnop_open():

1147                 /*
1148                  * For WRONLY opens, force DIRECT_IO.  This is necessary
1149                  * since writing a partial block through the buffer cache
1150                  * will result in a read of the block and that read won't
1151                  * be allowed by the WRONLY open.
1152                  */
1153                 if (fufh_type == FUFH_WRONLY ||
1154                     (fvdat->flag & FN_DIRECTIO) != 0)
1155                         fuse_open_flags = FOPEN_DIRECT_IO;

Sigh.
Comment 2 Conrad Meyer freebsd_committer freebsd_triage 2017-06-25 02:05:13 UTC
Created attachment 183775 [details]
Completely untested patch for the issue
Comment 3 Conrad Meyer freebsd_committer freebsd_triage 2017-06-25 02:20:37 UTC
With patch:

unique: 3, opcode: OPEN (14), nodeid: 2, insize: 48, pid: 1054
open flags: 0x1 /test.txt
   open[0] flags: 0x1 /test.txt
   unique: 3, success, outsize: 32
unique: 4, opcode: GETATTR (3), nodeid: 2, insize: 40, pid: 1054
getattr /test.txt
   unique: 4, success, outsize: 112
unique: 5, opcode: WRITE (16), nodeid: 2, insize: 70, pid: 1054
write[0] 6 bytes to 18 flags: 0x0
                 ^^^^^
   write[0] 6 bytes to 18
   unique: 5, success, outsize: 24

$ cat test.txt
line4
line2
line3
line4

So yeah, I think that fixes it.
Comment 4 Conrad Meyer freebsd_committer freebsd_triage 2017-06-25 02:22:28 UTC
See also the earlier https://svnweb.freebsd.org/base?view=revision&revision=245164 .
Comment 5 Conrad Meyer freebsd_committer freebsd_triage 2017-06-25 02:23:59 UTC
r299753 added the forced DIRECT_IO.
Comment 6 Conrad Meyer freebsd_committer freebsd_triage 2017-06-25 02:28:38 UTC
https://reviews.freebsd.org/D11348
Comment 7 Ben RUBSON 2017-06-25 08:49:44 UTC
Conrad, I just tested your patch and it works as expected.
Thank you very much for having investigated this (furthermore, so quickly) !
Ben
Comment 8 commit-hook freebsd_committer freebsd_triage 2017-06-28 13:56:26 UTC
A commit references this bug:

Author: cem
Date: Wed Jun 28 13:56:16 UTC 2017
New revision: 320451
URL: https://svnweb.freebsd.org/changeset/base/320451

Log:
  Complete support for IO_APPEND flag in fuse

  This finishes what r245164 started and makes open(..., O_APPEND) work again
  after r299753.

  - Pass ioflags, incl. IO_APPEND, down to the direct write backend (r245164
    added it to only the bio backend).
  - (r299753 changed the WRONLY backend from bio to direct.)

  PR:		220185
  Reported by:	Ben RUBSON <ben.rubson at gmail.com>
  Reviewed by:	bapt@, rmacklem@
  Sponsored by:	Dell EMC Isilon
  Differential Revision:	https://reviews.freebsd.org/D11348

Changes:
  head/sys/fs/fuse/fuse_io.c