Darrick J. Wong [Tue, 14 Apr 2020 16:56:20 +0000 (12:56 -0400)]
xfs_scrub: don't set WorkingDirectory= in systemd job
Somewhere between systemd 237 and 245, they changed the order in which a
job has its uid/gid set; capabilities applied; and working directory
set. Whereas before they did it in an order such that you could set the
working directory to a path inaccessible to 'nobody' (either because
they did it before changing the uid or after adding capabilities), now
they don't and users instead get a service failure:
xfs_scrub@-boot.service: Changing to the requested working directory failed: Permission denied
xfs_scrub@-boot.service: Failed at step CHDIR spawning /usr/sbin/xfs_scrub: Permission denied
xfs_scrub@-boot.service: Main process exited, code=exited, status=200/CHDIR
Regardless, xfs_scrub works just fine with PWD set to /, so remove that
directive.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Eric Sandeen <sandeen@redhat.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Wed, 8 Apr 2020 13:48:15 +0000 (09:48 -0400)]
xfs_scrub: fix type error in render_ino_from_handle
render_ino_from_handle is passed a struct xfs_bulkstat, not xfs_bstat.
Fix this.
Fixes: 4cca629d6ae3807 ("misc: convert xfrog_bulkstat functions to have v5 semantics") Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Allison Collins <allison.henderson@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Wed, 8 Apr 2020 13:48:15 +0000 (09:48 -0400)]
xfs_repair: fix dir_read_buf use of libxfs_da_read_buf
xfs_da_read_buf dropped the 'mappedbno' argument in favor of a flags
argument. Foolishly, we're passing that parameter (which is -1 in all
callers) to xfs_da_read_buf, which gets us the wrong behavior.
Since mappedbno == -1 meant "complain if we fall into a hole" (which is
the default behavior of xfs_da_read_buf) we can fix this by passing a
zero flags argument and getting rid of mappedbno entirely.
Coverity-id: 1457898 Fixes: 5f356ae6d ("xfs: remove the mappedbno argument to xfs_da_read_buf") Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Allison Collins <allison.henderson@oracle.com> Reviewed-by: Eric Sandeen <sandeen@redhat.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Wed, 8 Apr 2020 13:48:15 +0000 (09:48 -0400)]
xfs_db: clean up the salvage read callsites in set_cur()
Clean up the LIBXFS_READBUF_SALVAGE call sites in set_cur so that we
use the return value directly instead of scraping it out later.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Allison Collins <allison.henderson@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Wed, 8 Apr 2020 13:48:15 +0000 (09:48 -0400)]
libxfs: check return value of device flush when closing device
Although the libxfs_umount function flushes all devices when unmounting
the incore filesystem, the libxfs io code will flush the device again
when the application close them. Check and report any errors that might
happen, though this is unlikely.
Coverity-id: 1460464 Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Allison Collins <allison.henderson@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Wed, 8 Apr 2020 13:48:15 +0000 (09:48 -0400)]
libxfs: don't barf in libxfs_bwrite on a null buffer ops name
Don't crash if we failed to write a buffer that had no buffer verifier.
This should be rare in practice, but coverity found a valid bug.
Coverity-id: 1460462 Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Allison Collins <allison.henderson@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Dave Chinner [Wed, 8 Apr 2020 13:48:15 +0000 (09:48 -0400)]
xfs_io: set exitcode on failure appropriately
Many operations don't set the exitcode when they fail, resulting
in xfs_io exiting with a zero (no failure) exit code despite the
command failing and returning an error. The command return code is
really a boolean to tell the libxcmd command loop whether to
continue processing or not, while exitcode is the actual xfs_io exit
code returned to the parent on exit.
This patchset just makes the code do the right thing. It's not the
nicest code, but it's a start at producing correct behaviour.
Signed-Off-By: Dave Chinner <dchinner@redhat.com> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Dave Chinner [Wed, 8 Apr 2020 13:48:04 +0000 (09:48 -0400)]
xfsprogs: fix silently broken option parsing
When getopt() is passed an option string like "-m -n" and the
parameter m is defined as "m:", getopt returns a special error
to indication that the optstring started with a "-". Any getopt()
caller that is just catching the "?" error character will not
not catch this special error, so it silently eats the parameter
following -m.
Lots of getopt loops in xfsprogs have this issue. Convert them all
to just use a "default:" to catch anything unexpected.
Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Dave Chinner [Tue, 7 Apr 2020 18:29:39 +0000 (14:29 -0400)]
xfsprogs: LDFLAGS comes from configure, not environment
When doing:
$ LDFLAGS=foo make
bad things happen because we don't initialise LDFLAGS to an empty
string in include/builddefs.in and hence make takes wahtever is in
the environment and runs with it. This causes problems with linker
options specified correctly through configure.
We don't support overriding build flags (like CFLAGS) though the
make environment, so it was an oversight 13 years ago to allow
LDFLAGS to be overridden when adding support to custom LDFLAGS being
passed from the the configure script. This ensures we only ever use
linker flags from configure, not the make environment.
Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Dave Chinner [Tue, 7 Apr 2020 18:29:39 +0000 (14:29 -0400)]
xfsprogs: Fix --disable-static option build
Internal xfsprogs libraries are linked statically to binaries as
they are not shipped libraries. Using --disable-static prevents the
internal static libraries from being built and this breaks dead code
elimination and results in linker failures from link dependencies
introduced by dead code.
We can't remove the --disable-static option that causes this as it
is part of the libtool/autoconf generated infrastructure. We can,
however, override --disable-static on a per-library basis inside the
build by passing -static to the libtool link command. Therefore, add
-static to all the internal libraries we build and link statically
to the shipping binaries.
This build command now succeeds:
$ make realclean; make configure; ./configure --disable-static ; make
Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Dave Chinner [Tue, 7 Apr 2020 18:29:39 +0000 (14:29 -0400)]
mkfs: use cvtnum from libfrog
Move the checks for zero block/sector size to the libfrog code
and return -1LL as an invalid value instead. Catch the invalid
value in mkfs and error out there instead of inside cvtnum.
Also rename the libfrog block/sector size variables so they don't
shadow the mkfs global variables of the same name and mark the
string being passed in as a const.
Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Drop the null buffer pointer checks in all code that calls
xfs_alloc_read_agf and doesn't pass XFS_ALLOC_FLAG_TRYLOCK because
they're no longer necessary.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Refactor xfs_read_agf and xfs_alloc_read_agf to return EAGAIN if the
caller passed TRYLOCK and we weren't able to get the lock; and change
the callers to recognize this.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Remove the xfs_btree_get_bufs and xfs_btree_get_bufl functions, since
they're pretty trivial oneliners.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Convert xfs_trans_get_buf() to return numeric error codes like most
everywhere else in xfs.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Convert xfs_trans_get_buf_map() to return numeric error codes like most
everywhere else in xfs.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Convert xfs_buf_read() to return numeric error codes like most
everywhere else in xfs.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Convert xfs_buf_get_uncached() to return numeric error codes like most
everywhere else in xfs.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Convert xfs_buf_get() to return numeric error codes like most
everywhere else in xfs.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Convert xfs_buf_read_map() to return numeric error codes like most
everywhere else in xfs. This involves moving the open-coded logic that
reports metadata IO read / corruption errors and stales the buffer into
xfs_buf_read_map so that the logic is all in one place.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sat, 14 Mar 2020 03:01:00 +0000 (23:01 -0400)]
libxfs: make libxfs_buf_read_map return an error code
Make libxfs_buf_read_map() and libxfs_readbuf() return an error code
instead of making callers guess what happened based on whether or not
they got a buffer back.
Add a new SALVAGE flag so that certain utilities (xfs_db and xfs_repair)
can attempt salvage operations even if the verifiers failed, which was
the behavior before this change.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Increase XFS_BLF_DATAMAP_SIZE by 1 to fill in the implied padding at the
end of struct xfs_buf_log_format. This makes the size consistent so
that we can check it in xfs_ondisk.h, and will be needed once we start
logging attribute values.
On amd64 we get the following pahole:
struct xfs_buf_log_format {
short unsigned int blf_type; /* 0 2 */
short unsigned int blf_size; /* 2 2 */
short unsigned int blf_flags; /* 4 2 */
short unsigned int blf_len; /* 6 2 */
long long int blf_blkno; /* 8 8 */
unsigned int blf_map_size; /* 16 4 */
unsigned int blf_data_map[16]; /* 20 64 */
/* --- cacheline 1 boundary (64 bytes) was 20 bytes ago --- */
struct xfs_buf_log_format {
short unsigned int blf_type; /* 0 2 */
short unsigned int blf_size; /* 2 2 */
short unsigned int blf_flags; /* 4 2 */
short unsigned int blf_len; /* 6 2 */
long long int blf_blkno; /* 8 8 */
unsigned int blf_map_size; /* 16 4 */
unsigned int blf_data_map[16]; /* 20 64 */
/* --- cacheline 1 boundary (64 bytes) was 20 bytes ago --- */
Notice how the amd64 compiler inserts 4 bytes of padding to the end of
the structure to ensure 8-byte alignment. Prior to "xfs: fix memory
corruption during remote attr value buffer invalidation" we would try to
write to blf_data_map[17], which is harmless on amd64 but really bad on
i386.
This shouldn't cause any changes in the ondisk logging formats because
the log code writes out the log vectors with the appropriate size for
the log item's map_size, and log recovery treats the data_map array as a
VLA.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Now that we know we don't have to take a transaction to stale the incore
buffers for a remote value, get rid of the unnecessary memory allocation
in the leaf walker and call the rmt_stale function directly. Flatten
the loop while we're at it.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
While running generic/103, I observed what looks like memory corruption
and (with slub debugging turned on) a slub redzone warning on i386 when
inactivating an inode with a 64k remote attr value.
On a v5 filesystem, maximally sized remote attr values require one block
more than 64k worth of space to hold both the remote attribute value
header (64 bytes). On a 4k block filesystem this results in a 68k
buffer; on a 64k block filesystem, this would be a 128k buffer. Note
that even though we'll never use more than 65,600 bytes of this buffer,
XFS_MAX_BLOCKSIZE is 64k.
This is a problem because the definition of struct xfs_buf_log_format
allows for XFS_MAX_BLOCKSIZE worth of dirty bitmap (64k). On i386 when we
invalidate a remote attribute, xfs_trans_binval zeroes all 68k worth of
the dirty map, writing right off the end of the log item and corrupting
memory. We've gotten away with this on x86_64 for years because the
compiler inserts a u32 padding on the end of struct xfs_buf_log_format.
Fortunately for us, remote attribute values are written to disk with
xfs_bwrite(), which is to say that they are not logged. Fix the problem
by removing all places where we could end up creating a buffer log item
for a remote attribute value and leave a note explaining why. Next,
replace the open-coded buffer invalidation with a call to the helper we
created in the previous patch that does better checking for bad metadata
before marking the buffer stale.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Hoist the code that invalidates remote extended attribute value buffers
into a separate helper function. This prepares us for a memory
corruption fix in the next patch.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
xfs_check_ondisk_structs() verifies that the sizes of the data types
used by xfs are correct via the XFS_CHECK_STRUCT_SIZE() macro.
Since the structures padding can vary depending on the ABI (e.g. on
ARM OABI structures are padded to multiple of 32 bits), it may happen
that xfs_dir2_sf_entry_t size check breaks the compilation with the
assertion below:
In file included from linux/include/linux/string.h:6,
from linux/include/linux/uuid.h:12,
from linux/fs/xfs/xfs_linux.h:10,
from linux/fs/xfs/xfs.h:22,
from linux/fs/xfs/xfs_super.c:7:
In function ‘xfs_check_ondisk_structs’,
inlined from ‘init_xfs_fs’ at linux/fs/xfs/xfs_super.c:2025:2:
linux/include/linux/compiler.h:350:38:
error: call to ‘__compiletime_assert_107’ declared with attribute
error: XFS: sizeof(xfs_dir2_sf_entry_t) is wrong, expected 3
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
Restore the correct behavior adding __packed to the structure definition.
Cc: Darrick J. Wong <darrick.wong@oracle.com> Suggested-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Introduce a new #define for the maximum supported file block offset.
We'll use this in the next patch to make it more obvious that we're
doing some operation for all possible inode fork mappings after a given
offset. We can't use ULLONG_MAX here because bunmapi uses that to
detect when it's done.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
This helps to pre-simplify the extra handling of the null terminator in
delayed operations which use memcpy rather than strlen. Later
when we introduce parent pointers, attribute names will become binary,
so strlen will not work at all. Removing uses of strlen now will
help reduce complexities later
Signed-off-by: Allison Collins <allison.henderson@oracle.com> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Brian Foster <bfoster@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
XFS_ATTR_INCOMPLETE is a flag in the on-disk attribute format, and thus
in a different namespace as the ATTR_* flags in xfs_da_args.flags.
Switch to using a XFS_DA_OP_INCOMPLETE flag in op_flags instead. Without
this users might be able to inject this flag into operations using the
attr by handle ioctl.
Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Don't allow passing arbitrary flags as they change behavior including
memory allocation that the call stack is not prepared for.
Fixes: ddbca70cc45c ("xfs: allocate xattr buffer on demand") Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Sparse warns about a shadow variable in this function after the
Fixed: commit added another int i; with larger scope. It's safe
to remove the one with the smaller scope to fix this shadow,
although the shadow itself is harmless.
Fixes: 2c813ad66a72 ("xfs: support btrees with overlapping intervals for keys") Signed-off-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Dave Chinner [Fri, 13 Mar 2020 14:22:07 +0000 (10:22 -0400)]
xfsprogs: don't warn about packed members
gcc 9.2.1 throws lots of new warnings during the build like this:
xfs_format.h:790:3: warning: taking address of packed member of ‘struct xfs_agfl’ may result in an unaligned pointer value [-Waddress-of-packed-member]
790 | &(XFS_BUF_TO_AGFL(bp)->agfl_bno[0]) : \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
xfs_alloc.c:3149:13: note: in expansion of macro ‘XFS_BUF_TO_AGFL_BNO’
3149 | agfl_bno = XFS_BUF_TO_AGFL_BNO(mp, agflbp);
| ^~~~~~~~~~~~~~~~~~~
We know this packed structure aligned correctly, so turn off this
warning to shut gcc up.
Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
[sandeen: kernel has done this globally as well in 6f303d60534] Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Eric Sandeen [Tue, 3 Mar 2020 01:38:12 +0000 (20:38 -0500)]
xfs_admin: revert online label setting ability
"xfs_admin can't print both label and UUID for mounted filesystems"
https://bugzilla.kernel.org/show_bug.cgi?id=206429
alerted us to the problem that if /any/ options that use xfs_io get
specified to xfs_admin, they are the /only/ ones that get run:
# Try making the changes online, if supported
if [ -n "$IO_OPTS" ] && mntpt="$(find_mntpt_for_arg "$1")"
then
eval xfs_io -x -p xfs_admin $IO_OPTS "$mntpt"
test "$?" -eq 0 && exit 0
fi
and thanks to the exit, the xfs_db operations don't get run at all.
We could move on to the xfs_db commands after executing the xfs_io
commands, but we build them all up in parallel at this time:
so we'd need to keep track of these, and not re-run them in xfs_db.
Another issue is that prior to this commit, we'd run commands in
command line order.
So I experimented with building up an array of commands, invoking xfs_db
or xfs_io one command at a time as needed for each, and ... it got overly
complicated.
It's broken now, and so far a clean solution isn't evident, and I hate to
leave it broken across another release. So revert it for now.
Reverts: 3f153e051a ("xfs_admin: enable online label getting and setting") Signed-off-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:34:11 +0000 (12:34 -0500)]
libxfs: convert buffer priority get/set macros to functions
Convert these shouty macros to proper functions. We can't make them
static inline functions unless I f the 'libxfs_bcache' reference.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
[sandeen: fix a couple long lines] Reviewed-by: Eric Sandeen <sandeen@redhat.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:34:11 +0000 (12:34 -0500)]
libxfs: straighten out libxfs_writebuf naming confusion
libxfs_writebuf is not a well named function -- it marks the buffer
dirty and then releases the caller's reference. The actual write comes
when the cache is flushed, either because someone explicitly told the
cache to flush or because we started buffer reclaim.
Make the buffer release explicit in the callers and rename the function
to say what it actually does -- it marks the buffer dirty outside of
transaction context.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:34:11 +0000 (12:34 -0500)]
libxfs: use uncached buffers for initial mkfs writes
Teach mkfs to use uncached buffers to write the start and end of the
data device, the initial superblock, and the end of the realtime device
instead of open-coding uncached buffers. This means we can get rid of
libxfs_purgebuf since we handle the state from the start now.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:34:10 +0000 (12:34 -0500)]
libxfs: convert libxfs_log_clear to use uncached buffers
Convert the log clearing function to use uncached buffers like
everything else, instead of using the raw buffer get/put functions.
This will eventually enable us to hide them more effectively.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Eric Sandeen <sandeen@redhat.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:34:10 +0000 (12:34 -0500)]
libxfs: move log functions for convenience
Move libxfs_log_clear and libxfs_log_header to the bottom of the file so
that we avoid having to create advance declarations of static functions
in the next patch. No functional changes.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Eric Sandeen <sandeen@redhat.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:34:10 +0000 (12:34 -0500)]
xfs_copy: use uncached buffer reads to get the superblock
Upon startup, xfs_copy needs to read the filesystem superblock to mount
the filesystem. We cannot know the filesystem sector size until we read
the superblock, but we also do not want to introduce aliasing in the
buffer cache. Convert this code to the new uncached buffer read API so
that we can stop open-coding it.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:34:10 +0000 (12:34 -0500)]
xfs_db: use uncached buffer reads to get the superblock
Upon startup, xfs_db needs to check if it is even looking at an XFS
filesystem, and it needs the AG 0 superblock contents to initialize the
incore mount. We cannot know the filesystem sector size until we read
the superblock, but we also do not want to introduce aliasing in the
buffer cache. Convert this code to the new uncached buffer read API so
that we can stop open-coding it.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:34:10 +0000 (12:34 -0500)]
libxfs: introduce libxfs_buf_read_uncached
Introduce an uncached read function so that userspace can handle them in
the same way as the kernel. This also eliminates the need for some of
the libxfs_purgebuf calls (and two trips into the cache code).
Refactor the get/read uncached buffer functions to hide the details of
uncached buffer-ism in rdwr.c.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:34:10 +0000 (12:34 -0500)]
libxfs: replace libxfs_readbuf with libxfs_buf_read
Change all the libxfs_readbuf calls to libxfs_buf_read to match the
kernel interface. This enables us to hide libxfs_readbuf and simplify
the userspace buffer interface further.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:33:38 +0000 (12:33 -0500)]
libxfs: remove LIBXFS_EXIT_ON_FAILURE
Now that the read-side users of LIBXFS_EXIT_ON_FAILURE are gone and the
only write-side callers are in mkfs which now checks for buffer write
failures, get rid of LIBXFS_EXIT_ON_FAILURE.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:33:38 +0000 (12:33 -0500)]
libxfs: open-code "exit on buffer read failure" in upper level callers
Make all functions that use LIBXFS_EXIT_ON_FAILURE to abort on buffer
read errors implement that logic themselves. This also removes places
where libxfs can abort the program with no warning.
Note that in libxfs_mount, the "!(flags & DEBUGGER)" code would
indirectly select LIBXFS_EXIT_ON_FAILURE, so we're replacing the hidden
library exit(1) with a null xfs_mount return, which should cause the
utilities to exit with an error.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:33:38 +0000 (12:33 -0500)]
libfrog: always fsync when flushing a device
Always call fsync() when we're flushing a device, even if it is a block
device. It's probably redundant to call fsync /and/ BLKFLSBUF, but the
latter has odd behavior so we want to make sure the standard flush
methods have a chance to run first.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Brian Foster <bfoster@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:33:38 +0000 (12:33 -0500)]
xfs_repair: check that metadata updates have been committed
Make sure that any metadata that we repaired or regenerated has been
written to disk. If that fails, exit with 1 to signal that there are
still errors in the filesystem.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Brian Foster <bfoster@redhat.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:33:38 +0000 (12:33 -0500)]
mkfs: check that metadata updates have been committed
Make sure that all the metadata we wrote in the process of formatting
the filesystem have been written correctly, or exit with failure.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Brian Foster <bfoster@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:33:38 +0000 (12:33 -0500)]
libxfs: flush all dirty buffers and report errors when unmounting filesystem
Teach libxfs_umount to flush all dirty buffers when unmounting the
filesystem, to log write verifier errors and IO errors, and to return an
error code when things go wrong. Subsequent patches will teach critical
utilities to exit with EXIT_FAILURE when this happens.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Brian Foster <bfoster@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:33:38 +0000 (12:33 -0500)]
libxfs: return flush failures
Modify platform_flush_device so that we can return error status when
device flushes fail.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Allison Collins <allison.henderson@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:33:38 +0000 (12:33 -0500)]
libxfs: complain when write IOs fail
Complain whenever a metadata write fails.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Allison Collins <allison.henderson@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Sun, 1 Mar 2020 17:33:38 +0000 (12:33 -0500)]
libxfs: libxfs_buf_delwri_submit should write buffers immediately
The whole point of libxfs_buf_delwri_submit is to submit a bunch of
buffers for write and wait for the response. Unfortunately, while it
does mark the buffers dirty, it doesn't actually flush them and lets the
cache mru flusher do it. This is inconsistent with the kernel API,
which actually writes the buffers and returns any IO errors.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Allison Collins <allison.henderson@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Eric Biggers [Sun, 1 Mar 2020 17:33:38 +0000 (12:33 -0500)]
xfs_io/encrypt: support passing a keyring key to add_enckey
Add a '-k' option to the 'add_enckey' xfs_io command to allow exercising
the key_id field that is being added to struct fscrypt_add_key_arg.
This is needed for the corresponding test in xfstests.
For more details, see the corresponding xfstests patches as well as
kernel commit 93edd392cad7 ("fscrypt: support passing a keyring key to
FS_IOC_ADD_ENCRYPTION_KEY").
Signed-off-by: Eric Biggers <ebiggers@google.com> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Eric Sandeen [Fri, 28 Feb 2020 04:20:42 +0000 (23:20 -0500)]
xfs_repair: join realtime inodes to transaction only once
fill_rbmino() and fill_rsumino() can join the inode to the transactions
multiple times before committing, which is not permitted.
This leads to cache purge errors when running repair:
"cache_purge: shake on cache 0x92f5c0 left 129 nodes!?"
Move the libxfs_trans_ijoin out of the while loop to avoid this.
Fixes: e2dd0e1cc ("libxfs: remove libxfs_trans_iget") Signed-off-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Thu, 27 Feb 2020 20:49:27 +0000 (15:49 -0500)]
xfs_scrub: fix reporting of EINVAL for online repairs
The arguments to str_corrupt() are in the wrong order. Fix that.
Fixes: de5d20ece73f579 ("libfrog: convert scrub.c functions to negative error codes") Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Eric Sandeen <sandeen@redhat.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Thu, 27 Feb 2020 20:19:51 +0000 (15:19 -0500)]
libxfs: clean up libxfs_destroy
It's weird that libxfs_init opens the three devices passed in via the
libxfs_xinit structure but libxfs_destroy doesn't actually close them.
Fix this inconsistency and remove all the open-coded device closing.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Brian Foster <bfoster@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Eric Sandeen [Thu, 27 Feb 2020 20:05:48 +0000 (15:05 -0500)]
libxfs: use FALLOC_FL_ZERO_RANGE in libxfs_device_zero
I had a request from someone who cared about mkfs speed over
a slower network block device to look into using faster zeroing
methods, particularly for the log, during mkfs.
Using FALLOC_FL_ZERO_RANGE is faster in this case than writing
a bunch of zeros across a wire.
Signed-off-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Thu, 27 Feb 2020 20:05:47 +0000 (15:05 -0500)]
xfs_repair: try to correct sb_unit value from secondaries
If the primary superblock's sb_unit leads to a rootino calculation that
doesn't match sb_rootino /but/ we can find a secondary superblock whose
sb_unit does match, fix the primary.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Thu, 27 Feb 2020 20:05:47 +0000 (15:05 -0500)]
xfs_repair: check plausibility of root dir pointer before trashing it
If sb_rootino doesn't point to where we think mkfs should have allocated
the root directory, check to see if the alleged root directory actually
looks like a root directory. If so, we'll let it live because someone
could have changed sunit since formatting time, and that changes the
root directory inode estimate.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Thu, 27 Feb 2020 20:05:00 +0000 (15:05 -0500)]
xfs_repair: use libxfs function to calculate root inode location
Use libxfs_ialloc_calc_rootino to compute the location of the root
inode, and improve the function comments while we're at it.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Brian Foster <bfoster@redhat.com> Reviewed-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Thu, 27 Feb 2020 20:04:29 +0000 (15:04 -0500)]
xfs_repair: refactor fixed inode location checks
Refactor the checking and resetting of fixed-location inodes (root,
rbmino, rsumino) into a helper function.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Brian Foster <bfoster@redhat.com> Reviewed-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Thu, 27 Feb 2020 19:50:01 +0000 (14:50 -0500)]
xfs_repair: enforce that inode btree chunks can't point to AG headers
xfs_repair has a very old check that evidently excuses the AG 0 inode
btrees pointing to blocks that are already marked XR_E_INUSE_FS* (e.g.
AG headers). mkfs never formats filesystems that way and it looks like
an error, so purge the check. After this, we always complain if inodes
overlap with AG headers because that should never happen.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de>
[sandeen: removed unused switch cases that fall through to default] Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Thu, 27 Feb 2020 19:49:55 +0000 (14:49 -0500)]
mkfs: check root inode location
Make sure the root inode gets created where repair thinks it should be
created.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Brian Foster <bfoster@redhat.com> Reviewed-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Thu, 27 Feb 2020 19:22:19 +0000 (14:22 -0500)]
xfs_repair: replace verify_inum with libxfs inode validators
Repair uses the verify_inum function to validate inode numbers that it
finds in the superblock and in directories. libxfs now has validator
functions to cover that kind of thing, so remove verify_inum(). As a
side bonus, this means that we will flag directories that point to the
quota/realtime metadata inodes.
This fixes a regression found by fuzzing u3.sfdir3.hdr.parent.i4 to
lastbit (aka making a directory's .. point to the user quota inode) in
xfs/384.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Eric Sandeen <sandeen@redhat.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Thu, 27 Feb 2020 19:22:19 +0000 (14:22 -0500)]
xfs_repair: don't corrupt a attr fork da3 node when clearing forw/back
In process_longform_attr, we enforce that the root block of the
attribute index must have both forw or back pointers set to zero.
Unfortunately, the code that nulls out the pointers is not aware that
the root block could be in da3 node format.
This leads to corruption of da3 root node blocks because the functions
that convert attr3 leaf headers to and from the ondisk structures
perform some interpretation of firstused on what they think is an attr1
leaf block.
Found by using xfs/402 to fuzz hdr.info.hdr.forw.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
In process_longform_attr, replace the agcount check with a call to the
fsblock verification function in libxfs. Now we can also catch blocks
that point to static FS metadata.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Allison Collins <allison.henderson@oracle.com>
[sandeen: update error message] Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Thu, 27 Feb 2020 19:22:19 +0000 (14:22 -0500)]
libfrog: remove libxfs.h dependencies in fsgeom.c and linux.c
libfrog isn't supposed to depend on libxfs, so don't include the header
file in the libfrog source code.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Allison Collins <allison.henderson@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Eric Sandeen [Thu, 27 Feb 2020 19:21:45 +0000 (14:21 -0500)]
xfs_repair: fix bad next_unlinked field
As of xfsprogs-4.17 we started testing whether the di_next_unlinked field
on an inode is valid in the inode verifiers. However, this field is never
tested or repaired during inode processing.
So if, for example, we had a completely zeroed-out inode, we'd detect and
fix the broken magic and version, but the invalid di_next_unlinked field
would not be touched, fail the write verifier, and prevent the inode from
being properly repaired or even written out.
Fix this by checking the di_next_unlinked inode field for validity and
clearing it if it is invalid.
Reported-by: John Jore <john@jore.no> Fixes: 2949b4677 ("xfs: don't accept inode buffers with suspicious unlinked chains") Signed-off-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Thu, 30 Jan 2020 18:41:06 +0000 (13:41 -0500)]
xfs_io: fix integer over/underflow handling in timespec_from_string
When we're filling out the struct timespec, make sure we detect when the
string value cannot be represented by a (potentially 32-bit) seconds
field in struct timespec.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Thu, 30 Jan 2020 18:41:01 +0000 (13:41 -0500)]
xfs_repair: fix totally broken unit conversion in directory invalidation
Your humble author forgot that xfs_dablk_t has the same units as
xfs_fileoff_t, and totally screwed up the directory buffer invalidation
loop in dir_binval. Not only is there an off-by-one error in the loop
conditional, but the unit conversions are wrong.
The pre-5.5 xfs_da_get_buf implementation mostly hides the off-by-one
error because dir_binval turns on "don't complain if no mapping" mode,
but on dirblocksize > fsblocksize filesystems the incorrect units can
cause us to miss invalidating some blocks, which can lead to other
buffer cache errors later.
Fixes: f9c559f4e4fb4 ("xfs_repair: invalidate dirty dir buffers when we zap a directory") Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
Darrick J. Wong [Thu, 30 Jan 2020 18:40:54 +0000 (13:40 -0500)]
xfs_io: fix pwrite/pread length truncation on 32-bit systems
The pwrite and pread commands in xfs_io accept an operation length that
can be any quantity that fits in a long long int; and loops to handle
the cases where the operation length is larger than the IO buffer.
Weirdly, the do_ functions contain code to shorten the operation to the
IO buffer size but the @count parameter is size_t, which means that for
a large argument on a 32-bit system, we rip off the upper bits of the
length, turning your 8GB write into a 0 byte write, which does nothing.
This was found by running generic/175 and observing that the 8G test
file it creates has zero length after the operation:
wrote 0/8589934592 bytes at offset 0
0.000000 bytes, 0 ops; 0.0001 sec (0.000000 bytes/sec and 0.0000 ops/sec)
Fix this by pushing long long count all the way through the call stack.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>