Dave Chinner [Fri, 16 Nov 2012 01:14:48 +0000 (01:14 +0000)]
xfs_quota: correctly initialise the default path
When we initial xfs_quota, we place lots of information into the
fs_table. This includes all the devices/mount points the user has
specified as a global command line parameter to report on, as well
as all the paths under project quota control.
There is a "current path" pointer (fs_path) maintained by the code
that points somewhere into the fs_table. After the table is
initialised, fs_path always points to the last entry in the table,
and hence has to be re-initialised to point at the desired entry
before it can be used properly.
In the case of xfs_quota, if the command passed on the command line
is a non-global command, the command is called multiple times, each
time after the libxcmd args_command() callback is run. That starts
with an index of 0, and until the callback returns zero it will keep
passing whatever the last returned value was into the callback.
xfs_quota supplies such a callback, and it's purpose is to iterate
over the fs_table setting fs_path to the next mount point in the
table. IOWs, non-global quota functions get called once for each
mount point specified on the command line. However, it also means
that for global functions, the fs_path pointer is not
re-initialised and hence if there are project quotas configured the
fs_path pointer does not point to a mount point andhence commands
may malfunction..
The problem that demonstrated this is the report function. It does
it's own fs_table iteration if the command requires it, and so only
should be called once to avoid outputting the same information
multiple times. That's what the previous patch fixed by making the
command global, but this now has the effect of making commands that
need to operate on the device specified on the global command rely
on the fs_path variable pointing at that device.
Further, commands executed by the interactive method are always
treated as global commands, so the report command never worked as a
global command in the presence of a configured project quota setup.
Fix the problem by initialising the fs_path pointer correctly.
Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Mark Tinguely <tinguely@sgi.com> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
Dave Chinner [Fri, 9 Nov 2012 07:02:58 +0000 (07:02 +0000)]
xfs_quota: fix report command parsing
The report command line needs to be parsed as a whole not as
individual elements - report_f() is set up to do this correctly.
When treated as non-global command line, the report function is
called once for each command line arg, resulting in reports being
issued multiple times.
Set the command to be a global command so that it is only called
once.
Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Mark Tinguely <tinguely@sgi.com> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
Dave Chinner [Fri, 9 Nov 2012 07:02:57 +0000 (07:02 +0000)]
xfs_db: flush devices before exiting
Test 287 uses xfs_db to change 32-bit project ID support while the
filesystem is unmounted. On a large filesystem the test was failing
due to the mount not seeing the feature bit in the superblock.
xfs_db uses a different address space to the filesystem when it is
mounte dby the kernel, so the only way to keep them coherent is to
ensure that all buffered data is written to disk before the other
entity tries to read it. xfs_db uses buffered IO, but does not close
the devices when it exits, thereby leaving changes it has written in
the block device cache rather than on disk. Hence when th ekernel
tries to mount the filesystem, it reads what is on disk and does not
see xfs_db's changes.
Fix this by ensuring that xfs_db flushes it's changes to disk before
it exits by caling libxfs_device_close(). This fsyncs the data and
flushes the caches to ensure that it is present on disk before
xfs_db exits.
Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Mark Tinguely <tinguely@sgi.com> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
Mike Frysinger [Mon, 24 Sep 2012 23:39:38 +0000 (19:39 -0400)]
xfsprogs: install shared libs with +x bits
These are shared libs w/executable code, so make sure they have +x bits
set on them. Some kernels will proactively disallow executable mmaps if
the files lack +x bits. It's also the right thing to do.
Signed-off-by: Mike Frysinger <vapier@gentoo.org> Reviewed-by: Ben Myers <bpm@sgi.com> Signed-off-by: Ben Myers <bpm@sgi.com>
Eric Sandeen [Wed, 10 Oct 2012 03:40:11 +0000 (03:40 +0000)]
xfs_io: include headers for preadv/pwritev
We need to include uio.h to avoid:
[CC] pread.o
pread.c: In function `do_pread':
pread.c:198: warning: implicit declaration of function `preadv'
[CC] pwrite.o
pwrite.c: In function `do_pwrite':
pwrite.c:85: warning: implicit declaration of function `pwritev'
Signed-off-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
Carlos Maiolino [Mon, 16 Apr 2012 20:56:56 +0000 (20:56 +0000)]
mkfs: Set a clean output in case of invalid inode size
Remove an unnecessary usage() call after a mkfs failure due an invalid inode
size.
A call to usage() at this point confuses the output message which may cause the
user to think it used wrong arguments to mkfs, instead of an invalid inode size.
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> Reviewed-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Mark Tinguely <tinguely@sgi.com> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
Mike Frysinger [Sat, 25 Aug 2012 23:07:30 +0000 (23:07 +0000)]
libxcmd: link against readline
This library uses readline funcs (the input.c file), so we need to link
this shared library against it.
URL: https://bugs.gentoo.org/432644 Reported-by: David Badia <dbadia@gmail.com> Signed-off-by: Mike Frysinger <vapier@gentoo.org> Reviewed-by: Mark Tinguely <tinguely@sgi.com> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
Dave Chinner [Wed, 25 Jul 2012 22:30:50 +0000 (22:30 +0000)]
xfs_io: implement pwritev for vectored writes
When looking at KVM based direct IO patterns, I noticed that it was
using preadv and pwritev, and I could not use xfs_io to simulate
these IO patterns. Extend the pwrite command to be able to issue
vectored write IO to enable use to simulate KVM style direct IO.
Also document the new parameters as well as all the missing pwrite
command parameters in the xfs_io(8) man page.
Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Mark Tinguely <tinguely@sgi.com> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
Dave Chinner [Wed, 25 Jul 2012 22:30:49 +0000 (22:30 +0000)]
xfs_io: implement preadv for vectored reads
When looking at KVM based direct IO patterns, I noticed that it was
using preadv and pwritev, and I could not use xfs_io to simulate
these IO patterns. Extend the pread command to be able to issue
vectored read IO to enable use to simulate KVM style direct IO.
Also document the new parameters as well as all the missing pread
command parameters in the xfs_io(8) man page.
Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
Dave Chinner [Wed, 25 Jul 2012 22:30:48 +0000 (22:30 +0000)]
xfs_io: add sync_file_range support
Add sync_file_range support to xfs_io to allow fine grained control
of data writeback and syncing on a given file.
Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Mark Tinguely <tinguely@sgi.com> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
Peter Watkins [Thu, 2 Aug 2012 22:27:16 +0000 (17:27 -0500)]
xfs_db: bmap dump uses wrong btree key/ptr macro
When dumping the bmap with extents in btree form, the traversal
code should use XFS_BMBT_ macros instead of XFS_BMDR_ macros to
access the key and pointer fields below the root node.
Signed-off-by: Peter Watkins <treestem@gmail.com> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
which does not account for fragmented multiblock dir2.
ndbno was blindly being advanced by m_dirblkfsbs, but then
blkmap_next_off() would return the logical block of the next
mapped extent in blkmap, which may be within the current
(fragmented) dir2 multi-block, not the next multi-block,
because the extent index t hadn't been advanced.
Fix this by calling blkmap_next_off() until ndbno has advanced
into the next multiblock dir2 block, thereby keeping
the extent index t straight while properly advancing
ndbno.
Signed-off-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Ben Myers <bpm@sgi.com>
blkmap_next_off() was cryptic (to me), so document what it does.
Also catch cases when the passed in extent index 't' is beyond
the number of extents in the blkmap, so that:
ext = blkmap->exts + *t;
doesn't walk off the end of the array into garbage.
Signed-off-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Ben Myers <bpm@sgi.com>
Eric Sandeen [Tue, 5 Jun 2012 18:24:55 +0000 (13:24 -0500)]
xfs_repair: Fix fragmented multiblock dir2 handling in blkmap_getn()
blkmap_getn() contains a loop which populates an array of extents
with mapping information for a dir2 "block," which may consist
of multiple filesystem blocks.
As written, the loop re-allocates the array for each new extent,
leaking the previously allocated memory and worse, losing the
previously filled-in extent information.
Fix this by only allocating the array once, for the maximum
possible number of extents - the number of fs blocks in the dir
block.
Signed-off-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Ben Myers <bpm@sgi.com>
Eric Sandeen [Thu, 29 Mar 2012 03:23:11 +0000 (22:23 -0500)]
mkfs.xfs: print std info if agcount makes agsize out of bounds
When specifying a too-small agcount with stripe geometry,
mkfs.xfs can fail with a somewhat unexpected message:
$ mkfs.xfs -f -d file,name=fsfile,size=9764864000b,agcount=31,su=512k,sw=20
Allocation group size (314995613) is not a multiple of the stripe unit (128)
This strikes me as especially odd because normally, mkfs.xfs
tries to fix up the agsize to be a stripe multiple. The only way
we get to the above error message is if ag _size_ is out of bounds;
exiting with an error about alignment rather than about size
seems odd.
Maybe below is too clever, but if by the time we've decided that
agsize is out of bounds after rounding it both up and down,
as necessary, to get to a stripe-width multiple, calling
validate_ag_geometry() will give us the same standard message as
if we had specified no stripe geometry:
$ mkfs/mkfs.xfs -f -d file,name=fsfile,size=9764864000b,agcount=31,su=512k,sw=20
agsize (314995613b) too big, maximum is 268435455 blocks
Usage: mkfs.xfs
...
$ mkfs/mkfs.xfs -f -d file,name=fsfile,size=9764864000b,agcount=31
agsize (314995613b) too big, maximum is 268435455 blocks
Usage: mkfs.xfs
...
Also, tidy up error message to explicitly state "blocks" not "b"
Signed-off-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Dave Chinner <dchinner@redhat.com>
Eric Sandeen [Thu, 8 Mar 2012 22:45:07 +0000 (16:45 -0600)]
xfs_io: allow -F in open args, remove from help
Now that -F ("foreign") is automagic, we should no longer list
it in the help output for open, but we should still accept
it for compatibility; esp. as it is still in the case statement.
Oops.
Remove the -F option from the manpage open section as well.
Reported-by: Mark Tinguely <tinguely@sgi.com> Signed-off-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Mark Tinguely <tinguely@sgi.com>
Link counts are unsigned and need to be printed as such. Also only
print the varning about upgrading the inode version if the inode was
version 1 before.
Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
It looks like we currently never grow the variable-width nlink array
if only the on-disk nlink size overflows 8 bits. This leads to a major
mess in nlink counting, and eventually an assert in phase7.
Replace the indirect all mess with a union that allows doing proper
array arithmetics while we're at it.
Reviewed-by: Ben Myers <bpm@sgi.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
repair: fix incorrect use of thread local data in dir and attr code
The attribute and dirv1 code use pthread thread local data incorrectly in
a few places, which will make them fail in horrible ways when using the
ag_stride options.
Replace the use of thread local data with simple local allocations given
that there is no needed to micro-optimize these allocations as much
as e.g. the extent map. The added benefit is that we have to allocate
less memory, and can free it quickly.
Reviewed-by: Dave Chinner <dchinner@redhat.com> Reported-by: Tom Crane <T.Crane@rhul.ac.uk> Tested-by: Tom Crane <T.Crane@rhul.ac.uk> Signed-off-by: Christoph Hellwig <hch@lst.de>
Make sure we do not reject an XFS root mount just because /dev/root is also
listed in /proc/mounts. The root cause for this was the awkward getmntany
function, which is replaced with a broader reach find_mountpoint function
which replace getmntany and the surrounding code from the main routine in
a structured way. This changes the flow from finding a mounted filesystem
matching the argument and checking that it's XFS to find a mounted XFS
filesystem and thus fixes the bug.
Based on analysis and an earlier patch from
Carlos Maiolino <cmaiolino@redhat.com>.
Reviewed-by: Eric Sandeen <sandeen@redhat.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
Dave Chinner [Fri, 2 Mar 2012 08:34:58 +0000 (08:34 +0000)]
xfs_io: fix fiemap loop continuation
When the fiemap command needs to retrieve more extents from the
kernel via a subsequent IO, it calculates the next logical block to
retrieve in file system block units. the fiemap needs the start
offset in bytes, not filesystem blocks. Hence if the fiemap command
can loop forever retrieving the same blocks if the logical offset
offset of the next block in filesystem block units is smaller than
the number of bytes in a filessytem block. i.e. it will just loop
retreiving the first 32 extents from offset block zero.
Signed-off-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
Eric Sandeen [Fri, 2 Mar 2012 04:46:35 +0000 (22:46 -0600)]
mkfs.xfs: properly handle physical sector size
This splits the fs_topology structure "sectorsize" into
logical & physical, and gets both via blkid_get_topology().
This primarily allows us to default to using the physical
sectorsize for mkfs's "sector size" value, the fundamental
size of any IOs the filesystem will perform.
We reduce mkfs.xfs's "sector size" to logical if
a block size < physical sector size is specified.
This is suboptimal, but permissable.
For block size < sector size, differentiate the error
message based on whether the sector size was manually
specified, or deduced.
Signed-off-by: Eric Sandeen <sandeen@redhat.com> Reviewed-by: Dave Chinner <dchinner@redhat.com>
Dave Chinner [Thu, 2 Feb 2012 06:21:14 +0000 (17:21 +1100)]
xfs_io: fix fiemap loop continuation
When the fiemap command needs to retrieve more extents from the
kernel via a subsequent IO, it calculates the next logical block to
retrieve in file system block units. the fiemap needs the start
offset in bytes, not filesystem blocks. Hence if the fiemap command
can loop forever retrieving the same blocks if the logical offset
offset of the next block in filesystem block units is smaller than
the number of bytes in a filessytem block. i.e. it will just loop
retreiving the first 32 extents from offset block zero.
Signed-off-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
Eric Sandeen [Fri, 27 Jan 2012 19:26:19 +0000 (13:26 -0600)]
xfs_quota: check for size parsing errors
Doing something like
# xfs_quota -x -c 'limit -u bhard=1.2g ...
will cause cvtnum to fail and return a value of -1LL (because it
cannot parse the decimal), but the quota caller doesn't check
for this error value, casts it to U64, shifts right, and we end
up with an answer of 16 petabytes rather than erroring out.
Fix this.
Reviewed-by: Mark Tinguely <tinguely@sgi.com> Reported-by: James Lawrie <james@jdlawrie.co.uk> Signed-off-by: Eric Sandeen <sandeen@redhat.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
repair: update extent count after zapping duplicate blocks
When we find a duplicate extent in an extern format inode we do not zap
the whole inode, but just truncate it to the point where the duplicate
extent was found. But the current code only updates di_nblocks for the
new size, but no di_nextents/di_anextents. In most cases this isn't noticed,
but when moving such an inode to the lost+found directoy the consistency
check in xfs_iformat trips over it. Fix this by updating the on-disk
extent count as part of the inode repair.
Note that we zap btree format inodes with duplicate block completely
at this point, so this fix doesn't apply to them.
Reviewed-by: Mark Tinguely <tinguely@sgi.com> Reported-by: Arkadiusz Mi??kiewicz <arekm@maven.pl> Tested-by: Arkadiusz Mi??kiewicz <arekm@maven.pl> Signed-off-by: Christoph Hellwig <hch@lst.de>
I see "use xfs_repair instead of xfs_check" hint on xfs@irc, mailing
lists and other places but the first source of information (xfs_check
man page) doesn't mention this. Improve that.
Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl> Signed-off-by: Christoph Hellwig <hch@lst.de>
Remove indirections in the inode record bit manipulation macros and flatten
them to a single level of inlines. Also use a common IREC_MASK define
instead of duplicating it for every bitmask.
Reviewed-by: Mark Tinguely <tinguely@sgi.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
Nathan Scott [Tue, 31 Jan 2012 23:11:16 +0000 (10:11 +1100)]
xfsprogs: extend fiemap configure check
Make the fiemap configure check consistent with the other
libc interface checks - perform a compile and link with a
complete set of symbols, macros and interfaces needed, as
opposed to a build with just the headers.
Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Nathan Scott <nathans@debian.org> Signed-off-by: Christoph Hellwig <hch@lst.de>
Add a build dependency on linux-libc-dev, to ensure we build
packages with have_fiemap set to true if the headers support
it. Noticed by Dave, some package builds didn't enable this
when they should have.
Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Nathan Scott <nathans@debian.org> Signed-off-by: Christoph Hellwig <hch@lst.de>
repair: handle filesystems with the log in allocation group 0
Sindre Skogen reported that repair chokes on a very small filesystem created
by mkfs.xfs from xfsprogs 2.9.4. It turned out that for some reason this
filesystem had the log in allocation group 0 and thus repairs validation
of the root inode number was off. Fix this by adding the log blocks if
the log is allocated in allocation group 0.
Reviewed-by: Dave Chinner <dchinner@redhat.com> Reported-by: Sindre Skogen <sindre@workzone.no> Signed-off-by: Christoph Hellwig <hch@lst.de>
The inode prefetching code has a fixed limit of inodes that might are
submitted at a time. Unfortunately the buffers for them get locked
once the prefetching starts. That way the threads processing the inode
might get stuck on buffer locked, but not submitted for reading yet.
Fix this by kicking the queue as soon as we would have to wait on the
ra_count semaphore.
Reviewed-by: Dave Chinner <dchinner@redhat.com> Reported-by: Arkadiusz Miśkiewicz <arekm@maven.pl> Tested-by: Arkadiusz Miśkiewicz <arekm@maven.pl> Signed-off-by: Christoph Hellwig <hch@lst.de>
On a sufficiently corrupt filesystem walking the btree nodes might hit the
same node node again, which currently will deadlock. Use a recursion
counter to avoid the direct deadlock and let them normal loop detection
(two bad nodes and out) do its work. This is how repair behaved before
we added the lock when implementing buffer prefetching.
Reviewed-by: Dave Chinner <dchinner@redhat.com> Reported-by: Arkadiusz Miśkiewicz <arekm@maven.pl> Tested-by: Arkadiusz Miśkiewicz <arekm@maven.pl> Signed-off-by: Christoph Hellwig <hch@lst.de>
repair: allocate and free extent records individually
Instead of allocating inode records in chunks and keeping a freelist of them
which gets released to the system memory allocator in one go use plain malloc
and free for them. The freelist just means adding a global lock instead
of relying on malloc and free which could be implemented lockless. In
addition smart allocators like tcmalloc have far less overhead than our
chunk and linked list.
Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
repair: allocate and free inode records individually
Instead of allocating inode records in chunks and keeping a freelist of them
which never gets released to the system memory allocator use plain malloc
and free for them. The freelist just means adding a global lock instead
of relying on malloc and free which could be implemented lockless, and the
freelist is almost completely worthless as we are done allocating new
inode records once we start freeing them in major quantities.
Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
Stefan Pfetzing reported a bug where xfs_repair got stuck eating 100% CPU in
phase3. We track it down to a loop in the unlinked inode list, apparently
caused by memory corruption on an iSCSI target.
I looked into tracking if we already saw a given unlinked inode, but given
that we keep walking even for inodes where we can't find an allocation btree
record that seems infeasible. On the other hand these inodes had their
final unlink and thus were dead even before the system went down. There
really is no point in adding them to the uncertain list and looking for
references to them later.
So the simplest fix seems to be to simply remove the unlinked inode list
walk and just clear it - when we rebuild the inode allocation btrees these
will simply be marked free.
Reported-by: Stefan Pfetzing <stefan.pfetzing@1und1.de> Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
Carlos Maiolino [Tue, 6 Dec 2011 16:45:18 +0000 (14:45 -0200)]
mkfs: refuse to initialize a misaligned device if not forced using libblkid
This is a new version of a patch to fix the problem about the usage of 4k
sector devices when the device is not properly aligned. It makes mkfs to
refuse to initialize a xfs filesystem if the -f option is not passed at the
command line, and forces a 512b sector size if the user chooses to force
the device initialization.
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
repair: avoid ABBA deadlocks on prefetched buffers
Both the prefetch threads and actual repair processing threads can have
multiple buffers at a time locked, but they do no use a common locker
order, which can lead to ABBA deadlocks while trying to lock the buffers.
Switch the prefetch code to do a trylock and skip buffers that have
already been locked to avoid this deadlock.
Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
Nathan Scott [Thu, 17 Nov 2011 22:47:26 +0000 (16:47 -0600)]
Workaround the Debian build dependency handling for libreadline5.
Evidently the build daemons process dependencies differently than local builds,
and expect the first of optional dependencies to be resolved. Flip the
ordering to match this dependency.
Signed-off-by: Nathan Scott <nathans@debian.org> Signed-off-by: Alex Elder <aelder@sgi.com> Signed-off-by: Ben Myers <bpm@sgi.com>
Carlos Maiolino [Wed, 9 Nov 2011 16:54:07 +0000 (14:54 -0200)]
repair: properly mark lost+found inode as used
This patch makes mk_orphanage() to properly set the inode link count of
the recently allocated inode in the AVL tree, avoiding the lost+found
directory to be bypass the link count check in phase7 and possibly leaving
lost+found directory with a wrong link count.
Reviewed-by: Eric Sandeen <sandeen@redhat.com> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
Carlos Maiolino [Wed, 9 Nov 2011 16:54:06 +0000 (14:54 -0200)]
repair: add inline function to get ino tree node
Add get_inode_offset() inline function, which will return the offset
of a specific node in the AVL tree avoiding the need to calculate the
the offset each time it needs to be used.
Reviewed-by: Eric Sandeen <sandeen@redhat.com> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
Alex Elder [Thu, 13 Oct 2011 18:50:14 +0000 (18:50 +0000)]
libxfs: Don't forget to initialize the radix tree subsystem
The libxfs code uses radix tree routines to manage a mount
point's m_perag_tree. But the radix tree routines assume
that radix_tree_init() has been called to initialize the
height_to_maxindex[] global array, and this was not being
done.
This showed up when running mkfs.xfs on an ia64 system. Since
it wasn't initialized, the array was filled with zeroes. The
first time radix_tree_extend() got called (with index 0), the
height would be set to 1 and all would seem fine.
The *second* time it got called (with index 1) a problem would
arise--though we were apparently "lucky" enough for it not to
matter. The following loop would simply reference invalid slots
beyond the end of the array until it happened upon one that was
non-zero. (I've expanded the function radix_tree_maxindex() here.)
/* Figure out what the height should be. */
height = root->height + 1;
while (index > height_to_maxindex[height])
height++;
As an example, this looped 1937 times before it found a non-zere
value that would cause it to break out of the loop.
Even that *seemed* to be OK. But at the end of mkfs.xfs, when
it calls libxfs_umount(), non-initialized "slots" are dereferenced
and we hit a fault.
Wow.
Signed-off-by: Alex Elder <aelder@sgi.com> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Dave Chinner [Mon, 10 Oct 2011 01:08:35 +0000 (01:08 +0000)]
repair: prevent blkmap extent count overflows
Fix a bunch of invalid read/write errors due to excessive blkmap
allocations when inode forks are corrupted. These show up some time
after making a blkmap allocation for 536870913 extents on i686,
which is followed some time later by a crash caused bymemory
corruption.
This blkmap allocation size overflows 32 bits in such a
way that it results in a 32 byte allocation and so access to the
second extent results in access beyond the allocated memory and
corrupts random memory.
==5419== Invalid write of size 4
==5419== at 0x80507DA: blkmap_set_ext (bmap.c:260)
==5419== by 0x8055CF4: process_bmbt_reclist_int (dinode.c:712)
==5419== by 0x8056206: process_bmbt_reclist (dinode.c:813)
==5419== by 0x80579DA: process_exinode (dinode.c:1324)
==5419== by 0x8059B77: process_dinode_int (dinode.c:2036)
==5419== by 0x805ABE6: process_dinode (dinode.c:2823)
==5419== by 0x8052493: process_inode_chunk.isra.4 (dino_chunks.c:777)
==5419== by 0x8054012: process_aginodes (dino_chunks.c:1024)
==5419== by 0xFFF: ???
==5419== Address 0x944cfb8 is 0 bytes after a block of size 32 alloc'd
==5419== at 0x48E1102: realloc (in
/usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5419== by 0x80501F3: blkmap_alloc (bmap.c:56)
==5419== by 0x80599F5: process_dinode_int (dinode.c:2027)
==5419== by 0x805ABE6: process_dinode (dinode.c:2823)
==5419== by 0x8052493: process_inode_chunk.isra.4 (dino_chunks.c:777)
==5419== by 0x8054012: process_aginodes (dino_chunks.c:1024)
==5419== by 0xFFF: ???
Add overflow detection code into the blkmap allocation code to avoid
this problem.
Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Alex Elder <aelder@sgi.com>
Dave Chinner [Mon, 10 Oct 2011 01:08:34 +0000 (01:08 +0000)]
repair: don't cache large blkmap allocations
We currently use thread local storage for storing blkmap allocations
from one inode to another as a way of reducing the number of short
term allocations we do. However, the stored allocations can only
ever grow, so once we've done a large allocation we never free than
memory even if we never need that much memory again. This can occur
if we have corrupted extent counts in inodes, and can greatly
increase the memory footprint of the repair process.
Hence if the cached blkmap array id greater than a reasonable number
of extents (say 100,000), then don't store the blkmap in TLS and
instead free it.
Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Alex Elder <aelder@sgi.com>
Dave Chinner [Mon, 10 Oct 2011 01:08:33 +0000 (01:08 +0000)]
repair: handle memory allocation failure from blkmap_grow
If blkmap_grow fails to allocate a new chunk of memory, it returns
with a null blkmap. The sole caller of blkmap_grow does not check
for this failure, and so will segfault if this error ever occurs.
Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Alex Elder <aelder@sgi.com>
Dave Chinner [Mon, 10 Oct 2011 01:08:31 +0000 (01:08 +0000)]
repair: handle repair of image files on large sector size filesystems
Because repair uses direct IO, it cannot do IO smaller than a sector
on the underlying device. When repairing a filesystem image, the
filesystem hosting the image may have a sector size larger than the
sector size of the image, and so single image sector reads and
writes will fail.
To avoid this, when checking a file and there is a sector size
mismatch like this, turn off direct IO. While there, fix a compile
bug in the IO_DEBUG option for libxfs which was found during triage.
Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Alex Elder <aelder@sgi.com>
Alex Elder [Mon, 3 Oct 2011 12:49:20 +0000 (12:49 +0000)]
xfsprogs: libxcmd: ignore errors when initializing fs_table
When initializing fs_table, the full set of mounted filesystems and
the full set of defined projects are (or may be) examined. If an
error ever occurs looking at one of these entries, the processing
loop just quits, skipping all remaining mounts or projects.
One mount or project being problematic is no reason to give
up entirely. It may be that it is completely unrelated to
the mount point or project that the user wants to operate on.
So instead of quitting when an error occurs while adding
something to fs_table, proceed until all entries are added.
Meanwhile, the two affected functions are used for either
installing one entry in the table or for initializing the
table based on the full set of mounts or projects. In
the former case, once the entry matching that was requested
has been found there is no need to continue searching for
other entries, so break out of the loop immediately in
that case.
It so happens that these two changes affect the exact
same portion of the code...
Alex Elder [Mon, 3 Oct 2011 12:49:19 +0000 (12:49 +0000)]
xfsprogs: libxcmd: avoid exiting when an error occurs
In a number of spots handling setting up fs_table, libxcmd simply
prints a message and exits if an error occurs. There should be no
real need to exit in these cases. Notifying the user that something
went wrong is appropriate but this should not preclude continued
operation. In a few cases the contents of fs_table built up so far
are discarded as well, and this too can be avoided.
Make it so errors do not lead to exits, nor do they result in
destroying fs_table. Doing this requires returning a value from
fs_extract_mount_options() so its caller can skip other processing
in this case. But in most cases we simply no longer exit, and no
longer destroy the fs_table. This means there is no more use for
fs_table_destroy(), so it can be removed.
There is a sort of short-circuit exit in fs_table_insert_project()
that is unnecessary as well, so get rid of it.
Signed-off-by: Alex Elder <aelder@sgi.com> Reviewed-by: Christoph Hellwig <hch@lst.de>
Alex Elder [Mon, 3 Oct 2011 12:49:18 +0000 (12:49 +0000)]
xfsprogs: libxcmd: isolate strdup() calls to fs_table_insert()
Calls to fs_table_insert() are made in four places, and in all four
the mount directory and device name arguments passed are the result
of calls to strdup(). Rather than have all the callers handle
allocating and freeing of these strings, consolidate that into
fs_table_insert().
Only one place passes non-null values for the fslog and fsrt
arguments, and in that case it's easier to keep the allocation of
duplicate strings where they are in the caller. Add a comment in
fs_table_insert() to ensure that's understood.
Note also that fs_table_insert() is always called with both its
dir and fsname arguments non-null, so drop a check for that at
the top of the function.
Signed-off-by: Alex Elder <aelder@sgi.com> Reviewed-by: Christoph Hellwig <hch@lst.de>
Change fs_table_initialise() so it takes an array of mount points
and an array of project identifiers as arguments (along with their
respective sizes).
Change the quota code to provide fs_table_initialise() these arrays
rather than doing the individual mount point and project insertion
by itself. Other users just pass 0 counts, which results in filling
fs_table with entries for all mounted filesystems and all defined
projects.
This allows a few fs_table functions to be given private scope.
Signed-off-by: Alex Elder <aelder@sgi.com> Reviewed-by: Christoph Hellwig <hch@lst.de>
Alex Elder [Mon, 3 Oct 2011 12:49:16 +0000 (12:49 +0000)]
xfsprogs: libxcmd: avoid using strtok()
The strtok() library routine overwrites delimiting bytes in the
string it is supplied. It is also not length-constrained.
Since we're making a duplicate of the string anyway, and since we
are only finding the end of a single token, we can do both without
the need to modify the passed-in mount entry structure.
Add checking for memory allocation failures, and if one occurs just
exit (as is the practice elsewhere in this file).
Signed-off-by: Alex Elder <aelder@sgi.com> Reviewed-by: Christoph Hellwig <hch@lst.de>
Alex Elder [Wed, 28 Sep 2011 11:44:34 +0000 (11:44 +0000)]
xfsprogs: xfs_quota: kill local variable "type" from free_f()
Only one value is ever really used for the "type" variable in
free_f(), and it indicates that either type of entry in fs_table
is wanted. Just get rid of the variable and make use of the
ability to provide 0 to fs_cursor_initialise() to indicate that.
Signed-off-by: Alex Elder <aelder@sgi.com> Reviewed-by: Christoph Hellwig <hch@lst.de>
Alex Elder [Wed, 28 Sep 2011 11:44:33 +0000 (11:44 +0000)]
xfsprogs: libxcmd: allow 0 as a wildcard fs_table entry type selector
In libxcmd a table is used to represent filesystems and directories
that could be subject to quota operations. A cursor mechanism is
used to search that table, and it includes a flag that indicates
whether the type of entry desired represents a directory (for project
quotas) or a mount point (otherwise). It also allows a search for
either type.
There is only call to fs_cursor_initialise() where both mount points
and project paths are requested--all others just requested one or
the other.
Change it so when searching fs_table (in fs_table_lookup() and
fs_cursor_next_entry()), a zero "flags" value is interpreted as a
wildcard, matching either type of entry.
Also add some commentary explaining the use of 0 as a wildcard, and
simplify fs_cursor_next_entry() a bit in the process.
Signed-off-by: Alex Elder <aelder@sgi.com> Reviewed-by: Christoph Hellwig <hch@lst.de>
Alex Elder [Wed, 28 Sep 2011 11:15:37 +0000 (11:15 +0000)]
xfsprogs: xfs_quota: don't print invalid quota file inode number
When the state of quota files is dumped, xfs_quota blindly shows
whatever inode number is returned by the kernel. If one of the
quota types is not enabled or enforced, the inode number provided
is an invalid value ((__u64) -1). Rather than print a meaningless
large integer, print "N/A" in its place to make interpreting the
result it a little more obvious.
Signed-off-by: Alex Elder <aelder@sgi.com> Reviewed-by: Christoph Hellwig <hch@lst.de>
Alex Elder [Wed, 28 Sep 2011 10:57:12 +0000 (10:57 +0000)]
xfsprogs: libxcmd: use fs_device_number() consistently
The libxcmd code builds up a table that records information about
all filesystems that might be subject to quotas, as well as a set
of directories that are the roots of project quota trees.
When building the table, the device number for each affected
filesystem is determined (in fs_device_number()) using a call to
stat64(). It turns out that in all cases when doing this, a
directory path (and *not* a device special file path) is specified,
in which case the appropriate filesystem device id is found in the
st_dev field produce by the call to stat64() (i.e., the device id
for the mounted filesystem containing the path). Accordingly,
fs_device_number() always returns the st_dev field.
Another routine, fs_table_lookup(), looks up an entry in this table
based on the path name provided. However this function allows a
path to a device special file be provided. In that case the right
device id to use is found in the st_rdev field returned by stat64().
I found this to be confusing, and it took a while to convince
myself that this wasn't actually bug. (It wasn't initially clear
that device special files were never passed to fs_device_number().)
In order to prevent myself and others from ever wasting time like
this again, use fs_device_number() every time a device number is
needed, and in doing so determine it consistently in all cases (that
is--use st_rdev for device special files and st_dev otherwise).
In the process, change fs_device_number() to return an zero on
success (or an errno) rather than its first argument (or NULL).
Signed-off-by: Alex Elder <aelder@sgi.com> Reviewed-by: Christoph Hellwig <hch@lst.de>
Alex Elder [Wed, 28 Sep 2011 10:57:10 +0000 (10:57 +0000)]
xfsprogs: libxcmd: kill "search" arg in fs_device_number()
The function fs_device_number() in libxcmd allows the caller to
optionally "search" in /dev for a given device path in order to look
up the dev_t that represents that device path.
If set, all that function does is prepend "/dev/" to the path to see
if that produces a device path that works. So it appears this might
have been to support providing just the basename of a device as a
shorthand for its full path.
In practice, the paths passed to this function with "search" set are
those used in the mount options for a mounted XFS filesystem for the
optional log and real-time device paths. When such paths are used
in the XFS mount path, they will have been subject to a AT_FDCWD
path lookup, so unless the process mounting the filesystem was
sitting in /dev no relative path would ever be specified as just the
basename.
Even though the "mounting with CWD=/dev" is a conceivable scenario,
I think it is not likely enough to warrant the special handling to
cover that case in fs_device_number().
So delete the code that retries with a "/dev" prepended, eliminate
the "search" argument that enables it, and fix the callers
accordingly.
Signed-off-by: Alex Elder <aelder@sgi.com> Reviewed-by: Christoph Hellwig <hch@lst.de>
Alex Elder [Wed, 28 Sep 2011 10:57:08 +0000 (10:57 +0000)]
xfsprogs: libxcmd: don't clobber fs_table on realloc()
In fs_table_insert(), realloc() is called to resize the global
fs_table. If it fails, it overwrites a previously valid fs_table
pointer with NULL.
Instead, assign the return value to a local temporary and overwrite
fs_table only if the realloc() call succeeds. The only defined
errno value for a realloc() failure is ENOMEM, so return that
explicitly in the event it fails.
Signed-off-by: Alex Elder <aelder@sgi.com> Reviewed-by: Christoph Hellwig <hch@lst.de>
Add a b_error field to struct xfs_buf so that we can return the
exact error fro libxfs_readbuf. And explicit error return would be
nice, but this requires large changes to common code that should be
done on the kernel side first.
Signed-off-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Alex Elder <aelder@sgi.com>
libxfs: handle read errors in libxfs_trans_read_buf
Libxfs_readbuf may return a NULL buffer to indicate that an
error happend during the read, but we currently ignore that
if libxfs_trans_read_buf is called with a NULL transaction
pointer. Fix this by copying the relevant code from the
kernel version of the routine, and also tidy the code up a
bit by using a common exit label.
This fixes a regression that was introduced in xfsprogs 3.0.0 by
commit:
"Implement buffer and inode caching in libxfs, groundwork
for a parallel version of xfs_repair."
Signed-off-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Alex Elder <aelder@sgi.com>
projects / thirdparty / xfsprogs-dev.git / log
