Coverity complained that I wasn't initializing some class members until the
SetUp method. Do it in the constructor instead.
Reported by: Coverity
Coverity CIDs: 1404352, 1404378
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Where open(2) is expected to fail, the tests should assert or expect that
its return value is -1. These tests all accepted too much but happened to
pass anyway.
Reported by: Coverity
Coverity CID: 1404512, 1404378, 1404504, 1404483
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
When communicating with a FUSE server that implements version 7.8 (or older)
of the FUSE protocol, the FUSE_WRITE request structure is 16 bytes shorter
than normal. The protocol version check wasn't applied universally, leading
to an extra 16 bytes being sent to such servers. The extra bytes were
allocated and bzero()d, so there was no information disclosure.
Reviewed by: emaste
MFC after: 3 days
MFC-With: r350665
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D21557
The fusefs tests deliberately leak file descriptors. To do otherwise would
add extra complications to the tests' mock FUSE server. This annotation
should hopefully convince Coverity to shut up about the leaks.
Reviewed by: uqs
MFC after: 4 days
Sponsored by: The FreeBSD Foundation
Address the following defects reported by Coverity:
* Structurally dead code (CID 1404366): set m_quit before FAIL, not after
* Unchecked return value of sysctlbyname (CID 1404321)
* Unchecked return value of stat(2) (CID 1404471)
* Unchecked return value of open(2) (CID 1404402, 1404529)
* Unchecked return value of dup(2) (CID 1404478)
* Buffer overflows. These are all false positives caused by the fact that
Coverity thinks I'm using a buffer to store strings, when in fact I'm
really just using it to store a byte array that happens to be initialized
with a string. I'm changing the type from char to uint8_t in the hopes
that it will placate Coverity. (CID 1404338, 1404350, 1404367, 1404376,
1404379, 1404381, 1404388, 1404403, 1404425, 1404433, 1404434, 1404474,
1404480, 1404484, 1404503, 1404505)
* False positive file descriptor leak. I'm going to try to fix this with
Coverity modeling, but I'll also change an EXPECT to ASSERT so we don't
perform meaningless assertions after the failure. (CID 1404320, 1404324,
1404440, 1404445).
* Unannotated file descriptor leak. This will be followed up by a Coverity
modeling change. (CID 1404326, 1404334, 1404336, 1404357, 1404361,
1404372, 1404391, 1404395, 1404409, 1404430, 1404448, 1404451, 1404455,
1404457, 1404458, 1404460)
* Uninitialized variables in C++ constructors (CID 1404327, 1404346). In the
case of m_maxphys, this actually led to part of the FUSE_INIT's response
being set to stack garbage during the WriteCluster::clustering test.
* Uninitialized sun_len field in struct sockaddr_un (CID 1404330, 1404371,
1404429).
Reported by: Coverity
Reviewed by: emaste
MFC after: 3 days
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D21457
* A small error in r338152 let to the returned size always being exactly
eight bytes too large.
* The FUSE_LISTXATTR operation works like Linux's listxattr(2): if the
caller does not provide enough space, then the server should return ERANGE
rather than return a truncated list. That's true even though in FUSE's
case the kernel doesn't provide space to the client at all; it simply
requests a maximum size for the list. We previously weren't handling the
case where the server returns ERANGE even though the kernel requested as
much size as the server had told us it needs; that can happen due to a
race.
* We also need to ensure that a pathological server that always returns
ERANGE no matter what size we request in FUSE_LISTXATTR won't cause an
infinite loop in the kernel. As of this commit, it will instead cause an
infinite loop that exits and enters the kernel on each iteration, allowing
signals to be processed.
Reviewed by: cem
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D21287
Attempting to build the fusefs tests WITHOUT_GOOGLETEST will result in an
error if the host system or sysroot doesn't already have googletest headers
in /usr/include/private (e.g. host built/installed WITHOUT_GOOGLETEST, clean
cross-buildworld WITHOUT_GOOGLETEST).
Reviewed by: asomers
Differential Revision: https://reviews.freebsd.org/D21367
The FUSE_LISTXATTR operation always returns the full list of a file's
extended attributes, in all namespaces. There's no way to filter the list
server-side. However, currently FreeBSD's fusefs driver sends a namespace
string with the FUSE_LISTXATTR request. That behavior was probably copied
from fuse_vnop_getextattr, which has an attribute name argument. It's
been there ever since extended attribute support was added in r324620. This
commit removes it.
Reviewed by: cem
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D21280
The entirety of r351061 was a copy/paste error. I'm sorry I've been
comitting so hastily.
Reported by: rpokala
Reviewed by: rpokala
MFC after: 2 weeks
MFC-With: 351061
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D21265
In FUSE protocol 7.9, the size of the FUSE_GETATTR request has increased.
However, the fusefs driver is currently not sending the additional fields.
In our implementation, the additional fields are always zero, so I there
haven't been any test failures until now. But fusefs-lkl requires the
request's length to be correct.
Fix this bug, and also enhance the test suite to catch similar bugs.
PR: 239830
MFC after: 2 weeks
MFC-With: 350665
Sponsored by: The FreeBSD Foundation
The test needs to expect a FUSE_FORGET operation. Most of the time the test
would pass anyway, because by chance FUSE_FORGET would arrive after the
unmount.
MFC after: 2 weeks
MFC-With: 350665
Sponsored by: The FreeBSD Foundation
This is a gcc 8.0+ warning which needed to be silenced on for the riscv
build. amd64-xtoolchain-gcc still uses gcc 6.4.0 and does not understand
this flag.
Reviewed by: asomers
Feedback from: imp
Differential Revision: https://reviews.freebsd.org/D21195
When a fusefs file system is mounted using the writeback cache, the cache
may still be bypassed by opening a file with O_DIRECT. When writing with
O_DIRECT, the cache must be invalidated for the affected portion of the
file. Fix some panics caused by inadvertently invalidating too much.
Sponsored by: The FreeBSD Foundation
FUSE file systems can optionally support interrupting outstanding
operations. However, the file system does not identify to the kernel at
mount time whether it's capable of doing that. Instead it signals its
noncapability by returning ENOSYS to the first FUSE_INTERRUPT operation it
receives. That's a problem for reliable signal delivery, because the kernel
must choose which thread should get a signal before it knows whether the
FUSE server can handle interrupts. The problem is even worse because the
FUSE protocol allows a file system to simply ignore all FUSE_INTERRUPT
operations.
Fix the signal delivery logic by making interruptibility an opt-in mount
option. This will require a corresponding change to libfuse, but not to
most file systems that link to libfuse.
Bump __FreeBSD_version due to the new mount option.
Sponsored by: The FreeBSD Foundation
1) Don't explicitly not mask SIGKILL. kern_sigprocmask won't allow it to be
masked, anyway.
2) Fix an infinite loop bug. If a process received both a maskable signal
lower than 9 (like SIGINT) and then received SIGKILL,
fticket_wait_answer would spin. msleep would immediately return EINTR,
but cursig would return SIGINT, so the sleep would get retried. Fix it
by explicitly checking whether SIGKILL has been received.
3) Abandon the sig_isfatal optimization introduced by r346357. That
optimization would cause fticket_wait_answer to return immediately,
without waiting for a response from the server, if the process were going
to exit anyway. However, it's vulnerable to a race:
1) fatal signal is received while fticket_wait_answer is sleeping.
2) fticket_wait_answer sends the FUSE_INTERRUPT operation.
3) fticket_wait_answer determines that the signal was fatal and returns
without waiting for a response.
4) Another thread changes the signal to non-fatal.
5) The first thread returns to userspace. Instead of exiting, the
process continues.
6) The application receives EINTR, wrongly believes that the operation
was successfully interrupted, and restarts it. This could cause
problems for non-idempotent operations like FUSE_RENAME.
Reported by: kib (the race part)
Sponsored by: The FreeBSD Foundation
* Fix the kernel build with gcc by removing a redundant extern declaration
* In the tests, fix a printf format specifier that assumed LP64
Sponsored by: The FreeBSD Foundation
Previously fusefs would never recycle vnodes. After VOP_INACTIVE, they'd
linger around until unmount or the vnlru reclaimed them. This commit
essentially actives and inlines the old reclaim_revoked sysctl, and fixes
some issues dealing with the attribute cache and multiply linked files.
Sponsored by: The FreeBSD Foundation
closing a file descriptor causes FUSE activity that is superfluous to the
purpose of most tests, but would nonetheless require matching expectations.
Rather than do that, most tests deliberately leak file descriptors instead.
This commit moves the leakage from each test into two trivial functions:
leak and leakdir. Hopefully Coverity will only complain about those
functions and not all of their callers.
Sponsored by: The FreeBSD Foundation
Now the io tests are run in all cache modes. The fusefs test suite can now
get adequate coverage without changing the value of
vfs.fusefs.data_cache_mode, which is only needed for legacy file systems
now.
Sponsored by: The FreeBSD Foundation
As of protocol 7.23, fuse file systems can specify their cache behavior on a
per-mountpoint basis. If they set FUSE_WRITEBACK_CACHE in
fuse_init_out.flags, then they'll get the writeback cache. If not, then
they'll get the writethrough cache. If they set FOPEN_DIRECT_IO in every
FUSE_OPEN response, then they'll get no cache at all.
The old vfs.fusefs.data_cache_mode sysctl is ignored for servers that use
protocol 7.23 or later. However, it's retained for older servers,
especially for those running in jails that lack access to the new protocol.
This commit also fixes two other minor test bugs:
* WriteCluster:SetUp was using an uninitialized variable.
* Read.direct_io_pread wasn't verifying that the cache was actually
bypassed.
Sponsored by: The FreeBSD Foundation
If a server supports a timestamp granularity other than 1ns, it can tell the
client this as of protocol 7.23. The client will use that granularity when
updating its cached timestamps during write. This way the timestamps won't
appear to change following flush.
Sponsored by: The FreeBSD Foundation
I originally thought that the kernel would be responsible for ctime in
protocol 7.23. But now I realize that's not the case. The server is
responsible for ctime. The kernel only sets it when there are dirty writes
cached, because that's when the server can't.
Sponsored by: The FreeBSD Foundation
As of r349396 the kernel will internally update the mtime and ctime of files
on write. It will also flush the mtime should a SETATTR happen before the
data cache gets flushed. Now it will flush the ctime too, if the server is
using protocol 7.23 or higher.
This is the only case in which the kernel will explicitly set a file's
ctime, since neither utimensat(2) nor any other user interfaces allow it.
Sponsored by: The FreeBSD Foundation
Writing should implicitly update a file's mtime and ctime. For fuse, the
server is supposed to do that. But the client needs to do it too, because
the FUSE_WRITE response does not include time attributes, and it's not
desirable to issue a GETATTR after every WRITE. When using the writeback
cache, there's another hitch: the kernel should ignore the mtime and ctime
fields in any GETATTR response for files with a dirty write cache.
Sponsored by: The FreeBSD Foundation
Writes that extend a file should update the file's size. r344185 restricted
that behavior for fusefs to only happen when the data cache was enabled.
That probably made sense at the time because the attribute cache wasn't
fully baked yet. Now that it is, we should always update the cached file
size during write.
Sponsored by: The FreeBSD Foundation
Use the standard facilities for getpages and putpages instead of bespoke
implementations that don't work well with the writeback cache. This has
several corollaries:
* Change the way we handle short reads _again_. vfs_bio_getpages doesn't
provide any way to handle unexpected short reads. Plus, I found some more
lock-order problems. So now when the short read is detected we'll just
clear the vnode's attribute cache, forcing the file size to be requeried
the next time it's needed. VOP_GETPAGES doesn't have any way to indicate
a short read to the "caller", so we just bzero the rest of the page
whenever a short read happens.
* Change the way we decide when to set the FUSE_WRITE_CACHE bit. We now set
it for clustered writes even when the writeback cache is not in use.
Sponsored by: The FreeBSD Foundation
* During TearDown, close the test file before the backing file. That way
the backing file artifact will have the correct contents after the test
completes. It doesn't matter when running in Kyua, but it may when
running the test manually.
* Add a closeopen operation that mimics what FSX does with the "-c" option.
* Skip mmap-related tests when vfs.fusefs.data_cache_mode == 0
Sponsored by: The FreeBSD Foundation
VOP_GETPAGES intentionally tries to read beyond EOF, so fuse_read_biobackend
can't rely on bp->b_resid > 0 indicating a short read. And adjusting
bp->b_count after a short read seems to cause some sort of resource leak.
Instead, store the shortfall in the bp->b_fsprivate1 field.
Sponsored by: The FreeBSD Foundation
A fuse server may return a short read for three reasons:
* The file is opened with FOPEN_DIRECT_IO. In this case, the short read
should be returned directly to userland. We already handled this case
correctly.
* The file was truncated server-side, and the read hit EOF. In this case,
the kernel should update the file size. Fixed in the case of VOP_READ.
Fixing this for VOP_GETPAGES is TODO.
* The file is opened in writeback mode, there are dirty buffers past what
the server thinks is the file's EOF, and the read hit what the server
thinks is the file's EOF. In this case, the client is trying to read a
hole, and should zero-fill it. We already handled this case, and I added
a test for it.
Sponsored by: The FreeBSD Foundation
None of the new features are implemented yet. This commit just adds the new
protocol definitions and adds backwards-compatibility code for pre 7.23
servers.
Sponsored by: The FreeBSD Foundation
r349260 removed some Linuxisms from the FUSE protocol header file in favor
of standard C99 types. This change follows suit in the tests.
Sponsored by: The FreeBSD Foundation
This protocol level adds two new features: the ability for the server to
store or retrieve data into/from the client's cache. But the messages
aren't defined soundly since they identify the file only by its inode,
without the generation number. So it's possible for them to modify the
wrong file's cache. Also, I don't know of any file systems in ports that
use these messages. So I'm not implementing them. I did add a (disabled)
test for the store message, however.
Sponsored by: The FreeBSD Foundation
If the fuse daemon supports FUSE_BMAP, then use that for the block mapping.
Otherwise, use the same technique used by vop_stdbmap. Report large values
for runp and runb in order to maximize read clustering and minimize upcalls,
even if we don't know the true layout.
The major result of this change is that sequential reads to FUSE files will
now usually happen 128KB at a time instead of 64KB.
Sponsored by: The FreeBSD Foundation
* Don't always write the last page synchronously. That's not actually
required. It was probably just masking another bug that I fixed later,
possibly in r349021.
* Enable the NotifyWriteback tests now that Writeback cache is working.
* Add a test to ensure that the write cache isn't flushed synchronously when
in writeback mode.
Sponsored by: The FreeBSD Foundation
fusefs will now use cluster_read. This allows readahead of more than one
cache block. However, it won't yet actually cluster the reads because that
requires VOP_BMAP, which fusefs does not yet implement.
Sponsored by: The FreeBSD Foundation
