When searching the extent tree for the target extent item, we can miss it
if the extent item is the first item in a leaf and if there is a previous
leaf in the extent tree.
For example, if we call btrfs-map-logical like this:
$ btrfs-map-logical -l 5382144 /dev/sdc
And we have the following extent tree layout:
leaf 5386240 items 26 free space 2505 generation 7 owner EXTENT_TREE
leaf 5386240 flags 0x1(WRITTEN) backref revision 1
(...)
item 25 key (5373952 METADATA_ITEM 0) itemoff 3155 itemsize 33
refs 1 gen 7 flags TREE_BLOCK
tree block skinny level 0
(176 0x5) tree block backref root FS_TREE
leaf 5480448 items 56 free space 276 generation 7 owner EXTENT_TREE
leaf 5480448 flags 0x1(WRITTEN) backref revision 1
(...)
item 0 key (5382144 METADATA_ITEM 0) itemoff 3962 itemsize 33
refs 1 gen 7 flags TREE_BLOCK
tree block skinny level 0
(176 0x7) tree block backref root CSUM_TREE
(...)
Then the following happens:
1) We enter map_one_extent() with search_forward == 0 and
*logical_ret == 5382144;
2) We search for the key (5382144 0 0) which leaves us with a path
pointing to leaf 5386240 at slot 26 - one slot beyond the last item;
3) We then call:
btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0])
Which is not valid since there's no item at that slot, but since the
area of the leaf where an item at that slot should be is zeroed out,
we end up getting a key of (0 0 0);
4) We then enter the "if" statement bellow, since key.type is 0, and call
btrfs_previous_extent_item(), which leaves at slot 25 of leaf 5386240,
point to the extent item of the extent 5373952.
The requested extent, 5382144, is the first item of the next leaf
(5480448), but we totally miss it;
5) We return to the caller, the main() function, with 'cur_logical'
pointing to the metadata extent at 5373952, and not to the requested
one at 5382144.
In the last while loop of main() we have 'cur_logical' == 5373952,
which makes the loop have no iterations and therefore the local
variable 'found' remains with a value of 0, and then the program fails
like this:
$ btrfs-map-logical -l 5382144 /dev/sdc
ERROR: no extent found at range [5382144,5386240)
Fix this by never accessing beyond the last slot of a leaf. If we ever end
up at a slot beyond the last item in a leaf, just call btrfs_next_leaf()
and process the first item in the returned path.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
The inode_cache and involved on-disk formats are deprecated and will
have no effect since v5.11 kernel.
And in v6.11 kernel, new tree-checker will even reject data extents
belonging to those deprecated inode cache.
Lowmem check can detect such deprecated inode cache from the beginning.
This images are generated by 5.10 LTS kernels with inode cache.
Signed-off-by: Qu Wenruo <wqu@suse.com>
[BUG]
There are reports about deprecated inode cache causing newer kernels to
rejecting them.
Such inode cache is rarely utilized and already fully deprecated since
v5.11, and newer kernel will reject data extents of inode cache since
v6.11.
But original mode btrfs check won't detect nor report them as error.
Meanwhile lowmem mode can properly detect and report them:
ERROR: root 5 INODE[18446744073709551604] nlink(1) not equal to inode_refs(0)
ERROR: invalid imode mode bits: 00
ERROR: invalid inode generation 18446744073709551604 or transid 1 for ino 18446744073709551605, expect [0, 72)
ERROR: root 5 INODE[18446744073709551605] is orphan item
Since those inode cache paid no attention to properly maintain all the
numbers, they are easy targets for more recent lowmem mode.
[CAUSE]
For original mode, it has extra hardcoded hacks to avoid nlink checks
for inode cache inode.
Furthermore original mode doesn't check the mode bits nor its
generation.
[FIX]
For original mode, remove the hack for inode cache so that the
deprecated inode cache can be reported as an error.
For both modes, add extra global message to direct the affected users to
use 'btrfs rescue clear-ino-cache' to clear the deprecated cache.
Pull-request: #891
Signed-off-by: Qu Wenruo <wqu@suse.com>
[BUG]
There is one report about `btrfs rescue clear-ino-cache` failed with
tree block level mismatch:
# btrfs rescue clear-ino-cache /dev/mapper/rootext
Successfully cleaned up ino cache for root id: 5
Successfully cleaned up ino cache for root id: 257
Successfully cleaned up ino cache for root id: 258
corrupt node: root=7 block=647369064448 slot=0, invalid level for leaf, have 1 expect 0
node 647369064448 level 1 items 252 free space 241 generation 6065173 owner CSUM_TREE
node 647369064448 flags 0x1(WRITTEN) backref revision 1
fs uuid e6614f01-6f56-4776-8b0a-c260089c35e7
chunk uuid f665f535-4cfd-49e0-8be9-7f94bf59b75d
key (EXTENT_CSUM EXTENT_CSUM 3714473984) block 677126111232 gen 6065002
[...]
key (EXTENT_CSUM EXTENT_CSUM 6192357376) block 646396493824 gen 6065032
ERROR: failed to clear ino cache: Input/output error
[CAUSE]
During `btrfs rescue clear-ino-cache`, btrfs-progs will iterate through
all the subvolumes, and clear the inode cache inode from each subvolume.
The problem is in how we iterate the subvolumes.
We hold a path of tree root, and go modifiy the fs for each found
subvolume, then call btrfs_next_item().
This is not safe, because the path to tree root is not longer reliable
if we modified the fs.
So the btrfs_next_item() call will fail because the fs is modified
halfway, resulting the above problem.
[FIX]
Instead of holding a path to a subvolume root item, and modify the fs
halfway, here introduce a helper, find_next_root(), to locate the root
item whose objectid >= our target rootid, and return the found item key.
The path to root tree is only hold then released inside
find_next_root().
By this, we won't hold any unrelated path while modifying the
filesystem.
And since we're here, also adding back the missing new line when all ino
cache is cleared.
Pull-request: #890
Reported-by: Archange <archange@archlinux.org>
Link: https://lore.kernel.org/linux-btrfs/4803f696-2dc5-4987-a353-fce1272e93e7@archlinux.org/
Signed-off-by: Qu Wenruo <wqu@suse.com>
There is an internal report that, during btrfs-convert to block-group
tree, by accident some systemd events triggered the mount of the target
fs.
This leads to double mount (one by kernel and one by the btrfs-progs),
which seems to cause quite some problems.
To avoid such accident, exclusively opens all devices if btrfs-progs is
doing write operations.
Pull-request: #888
Reported-by: pandada8 <pandada8@gmail.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
btrfs-ioctl.rst was laid out like it should be a man page, including
having a section number, but it wasn't getting installed because there
was not enough content.
Pull-request: #892
Signed-off-by: Mark Harmstone <maharmstone@fb.com>
Signed-off-by: David Sterba <dsterba@suse.com>
The new test case will:
- Create a symbolic which contains a 4095 bytes sized target on ext4
- Convert the ext4 to btrfs
- Make sure we can still read the symbolic link
For unpatched btrfs-convert, the resulted symbolic link will be rejected
by kernel and fail.
Signed-off-by: Qu Wenruo <wqu@suse.com>
[BUG]
There is a recent bug that btrfs/012 fails and kernel rejects to read a
symbolic link which is backed by a regular extent.
Furthremore in that case, "btrfs check --mode=lowmem" doesn't detect such
problem at all.
[CAUSE]
For symbolic links, we only allow inline extents, and this means we should
only have a symbolic link target which is smaller than 4K.
But lowmem mode btrfs check doesn't handle symbolic link inodes any
differently, thus it doesn't check if the file extents are inlined or not,
nor reporting this problem as an error.
[FIX]
When processing data extents, if we find the owning inode is a symbolic
link, and the file extent is regular/preallocated, report an error for
the bad file extent item.
Signed-off-by: Qu Wenruo <wqu@suse.com>
symbolic links
[BUG]
There is a recent bug that btrfs/012 fails and kernel rejects to read a
symbolic link which is backed by a regular extent.
Furthremore in that case, "btrfs check" doesn't detect such problem at
all.
[CAUSE]
For symbolic links, we only allow inline file extents, and this means we
should only have a symbolic link target which is smaller than 4K.
But btrfs check doesn't handle symbolic link inodes any differently, thus
it doesn't check if the file extents are inlined or not, nor reporting
this problem as an error.
[FIX]
When processing data extents, if we find the owning inode is a symbolic
link, and the file extent is regular/preallocated, mark the inode with
I_ERR_FILE_EXTENT_TOO_LARGE error.
Signed-off-by: Qu Wenruo <wqu@suse.com>
[BUG]
Sometimes test case btrfs/012 fails randomly, with the failure to read a
symlink:
QA output created by 012
Checking converted btrfs against the original one:
-OK
+readlink: Structure needs cleaning
Checking saved ext2 image against the original one:
OK
Furthermore, this will trigger a kernel error message:
BTRFS critical (device dm-2): regular/prealloc extent found for non-regular inode 133081
[CAUSE]
For that specific inode 133081, the tree dump looks like this:
item 127 key (133081 INODE_ITEM 0) itemoff 40984 itemsize 160
generation 1 transid 1 size 4095 nbytes 4096
block group 0 mode 120777 links 1 uid 0 gid 0 rdev 0
sequence 0 flags 0x0(none)
item 128 key (133081 INODE_REF 133080) itemoff 40972 itemsize 12
index 2 namelen 2 name: l3
item 129 key (133081 EXTENT_DATA 0) itemoff 40919 itemsize 53
generation 4 type 1 (regular)
extent data disk byte 2147483648 nr 38080512
extent data offset 37974016 nr 4096 ram 38080512
extent compression 0 (none)
Note that, the symlink inode size is 4095 at the max size (PATH_MAX,
removing the terminating NUL).
But the nbytes is 4096, exactly matching the sector size of the btrfs.
Thus it results the creation of a regular extent, but for btrfs we do
not accept a symlink with a regular/preallocated extent, thus kernel
rejects such read and failed the readlink call.
The root cause is in the convert code, where for symlinks we always
create a data extent with its size + 1, causing the above problem.
I guess the original code is to handle the terminating NUL, but in btrfs
we never need to store the terminating NUL for inline extents nor
file names.
Thus this pitfall in btrfs-convert leads to the above invalid data
extent and fail the test case.
[FIX]
- Fix the ext2 and reiserfs symbolic link creation code
To remove the terminating NUL.
- Add extra checks for the size of a symbolic link
Btrfs has extra limits on the size of a symbolic link, as btrfs must
store symbolic link targets as inlined extents.
This means for 4K node sized btrfs, the size limit is smaller than the
usual PATH_MAX - 1 (only around 4000 bytes instead of 4095).
So for certain nodesize, some filesystems can not be converted to
btrfs.
(this should be rare, because the default nodesize is 16K already)
- Split the symbolic link and inline data extent size checks
For symbolic links the real limit is PATH_MAX - 1 (removing the
terminating NUL), but for inline data extents the limit is
sectorsize - 1, which can be different from 4096 - 1 (e.g. 64K sector
size).
Pull-request: #884
Signed-off-by: Qu Wenruo <wqu@suse.com>
Currently the transaction log is more or less ignored by btrfs check,
meaning that it's possible for a FS with a corrupt log to pass btrfs
check, but be immediately corrupted by the kernel when it's mounted.
Adds a check that if there's an inode in the log, any pending
non-inlined csumed writes also have corresponding csum entries.
Pull-request: #879
Signed-off-by: Mark Harmstone <maharmstone@fb.com>
[ Small commit message update. ]
Signed-off-by: Qu Wenruo <wqu@suse.com>
Added 0x prefix to HEX numbers and transform some tables to new format.
Pull-request: #881
Signed-off-by: Yuwei Han <hrx@bupt.moe>
[ Fix RST grammar errors ]
Signed-off-by: Qu Wenruo <wqu@suse.com>
Remove last newline in the output of 'btrfs filesystem show', keep the
line between two filesystems so the devices are visually grouped
togehter.
Pull-request: #866
Author: Matt Langford <github@matt.boats>
Signed-off-by: David Sterba <dsterba@suse.com>
Add limit parameter so workflows are not skipped if they don't fit the
default limit 10. Add more workflows to clean up after recent updates.
Signed-off-by: David Sterba <dsterba@suse.com>
There's a report that newly added --rootdir print too many warnings for
hardlinks, which is maybe not that uncommon. We still want to let the
user know about that so print it just once and count how many were
found:
$ mkfs.btrfs --rootdir ...
WARNING: '/tmp/btrfs-progs-mkfs-rootdir-hardlinks.7RcdfR/rootdir/inside_link' has extra hardlinks, they will be converted into new inodes
WARNING: 12 hardlinks were detected in /tmp/btrfs-progs-mkfs-rootdir-hardlinks.7RcdfR/rootdir, all converted to new inodes
Link: https://github.com/kdave/btrfs-progs/pull/872#issuecomment-2289096125
Signed-off-by: David Sterba <dsterba@suse.com>
The way the CRC32C checksum used for btrfs-send differs from the way
it's used elsewhere in btrfs. Without making the distinction, it's easy
to make the flawed assumption that CRC32C always refers to the same, and
end up with code that produces the wrong checksums.
This small note should guide the reader to the right function.
The best notes on the protocol I found are here:
https://archive.kernel.org/oldwiki/btrfs.wiki.kernel.org/index.php/Design_notes_on_Send/Receive.html
The crc32c might be used in two meanings and this could be confusing
when implementing the send stream protocol.
Rust code describing the algorithm for the crc crate that worked for me:
pub const CRC_32_BTRFS_SEND: crc::Algorithm<u32> = crc::Algorithm {
width: 32, poly: 0x1edc6f41, init: 0, refin: true, refout: true,
xorout: 0, check: 0xe3069283, residue: 0xb798b438
};
(it's a slight variation on the one used in ISCSI)
Note: Documentation/dev/dev-send-stream.rst briefly mentions that
Pull-request: #794
Author: rhn <gihu.rhn@porcupinefactory.org>
[ rephrase changelog and copy text from pull request and add link to
developer documentation of the send stream ]
Signed-off-by: David Sterba <dsterba@suse.com>
process_clone() only searches the received_uuid, but could exist in an
earlier uuid that isn't the received_uuid. Mirror what process_snapshot
does and search both the received_uuid and if that fails look up by
normal uuid.
Fixes: https://github.com/kdave/btrfs-progs/issues/606
Issue: #606
Pull-request: #643
Pull-request: #862
Signed-off-by: Arsenii Skvortsov <ettavolt@gmail.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Copy linux.git/include/linux/container_of.h definition of container_of
and the const variant (currently unused)
Signed-off-by: David Sterba <dsterba@suse.com>
Sync up with kernel and fix warnings reported by -Wcast-qual. eg.
Most of the change is due to extent_buffer::data, which is a direct
struct member, unlike in kernel where it's an array of pages. The
const qualifier cannot be used the same way so it's dropped in affected
herlpers.
Signed-off-by: David Sterba <dsterba@suse.com>
There are no functional changes, only cleanup of header files. This
could lead to build failures in case the headers were used as a
convenience outside of scope of libbtrfs just because of the kernel
compatibility.
- removed various definitions of variables, types, helpers and macros
from kerncompat.h that are neither used nor needed for libbtrfs code
- file list.h no longer shipped
- file rbtree.h no longer shipped
Signed-off-by: David Sterba <dsterba@suse.com>
None of the public API uses the rb-tree code besides definitions, so
change the includes in ctree.h and drop rbtree.h, this is used only by
internal implementation in send-utils.c. We could remove it in the
future but last time it was not possible due to 3rd party code depending
on it.
Removed in 83ab92512e ("libbtrfs: remove the support for fs without
uuid tree") and reverted again in f9b0da8e78 ("Revert "libbtrfs:
remove the support for fs without uuid tree"")
Signed-off-by: David Sterba <dsterba@suse.com>
The list_head is used in struct definitions but otherwise not at all as
it was copied from kernel code. For ctree.h add stub definition that
won't change the containing structure size.
Drop list.h from libbtrfs. This may break some builds if they used the
header, though this was never meant to be exported.
Signed-off-by: David Sterba <dsterba@suse.com>
The BUILD_ASSERT macro checks what _Static_assert can do. Remove it as
it's not really used in ioctl.h as it defines a stub. The assertions
still remain in the code outside of libbtrfs, we can delete it here as
the API is frozen and won't be changed.
Signed-off-by: David Sterba <dsterba@suse.com>
The stack trace and BUG_ON related reporting was inherited from the
tools, this should not be part of libbtrfs and was not intended to be
exported.
BUG() is still in used in ctree.h and send-utils.c so replace it with a
bare error report and remove the rest.
Keep __always_inline as it's needed for Musl.
Signed-off-by: David Sterba <dsterba@suse.com>
Commit bf0f3db765 ("btrfs-progs: introduce UASSERT() for purely
userspace code") added UASERT to distinguish ASSERT macro from user
space code. This was wrongly added to libbtrfs/ and pulled the
common/messages.h include too.
Signed-off-by: David Sterba <dsterba@suse.com>
Remove BUG_ONs that seem to be sanity checks that are done in other
places. We want to remove them from public header so BUG_ON can be
removed completely.
Signed-off-by: David Sterba <dsterba@suse.com>
There are many definitions of types etc that are not used in libbtrfs
but used to be in other code before the separation. Remove it as it's
not meant to be exported.
Signed-off-by: David Sterba <dsterba@suse.com>
Commit 14ac1a6051 ("btrfs-progs: mkfs: add support for squota")
mistakenly added ctree.h from libbtrfs/ but this is not supposed to be
used outside of the library. Moreover the correct ctree.h was already
there.
Signed-off-by: David Sterba <dsterba@suse.com>
The new test case creates a special layout like this:
rootdir/ (fs1 ino=256)
|- dir1/ (fs1 ino=257)
| |- dir1/ (fs2 ino=257)
| |- dir2/ (fs2 ino=258)
| |- file1 (fs2 ino=259)
| |- file2 (fs2 ino=260)
|- dir2/ (fs1 ino=258)
|- file1 (fs1 ino=259)
|- file2 (fs2 ino=259)
This layout intentionally creates inode number conflicts, which will
make the old "mkfs.btrfs --rootdir" to fail.
But newer reworked one will successfully handle them, just leave a test
case to avoid to hit the old bugs.
Signed-off-by: Qu Wenruo <wqu@suse.com>
The test case will create the following directory layout:
.
|- rootdir/
| |- inside_link
|- outside_link
Both inside_link and outside_link are hard links of each other.
And use rootdir/ as the rootdir for mkfs.
This is to ensure the nlink of inside_link is correctly set to 1.
Inspired by the recent rework which fixes the handling of hard links.
Signed-off-by: Qu Wenruo <wqu@suse.com>
The recent rework changes how we detect hard links.
[OLD BEHAVIOR]
We trusted st_nlink and st_ino, reuse them without extra sanity
checks.
That behavior has problems handling cross mount-point or hard links out
of the rootdir cases.
[NEW BEHAVIOR]
The new refactored code will treat every inode, no matter if it's a
hardlink, as a new inode.
This means we will break the hard link detection, and every hard link
will be created as a different inode.
For the most common use case, like populating a rootfs, it's totally
fine.
[EXTRA WARNING]
But for cases where the user have extra hard links inside the rootdir,
output a warning just to inform the end user.
This will not cause any content difference, just breaking the hard links
into new inodes.
Signed-off-by: Qu Wenruo <wqu@suse.com>
[PITFALLS]
There are several hidden pitfalls of the existing traverse_directory():
- Hand written preorder traversal
There is already a better written standard library function, nftw()
doing exactly what we need.
- Over-designed path list
To properly handle the directory change, we have structure
directory_name_entry, to record every inode until rootdir.
But it has two string members, dir_name and path, which is a little
confusing and overkilled.
As for preorder traversal, we will never need to read the parent's
filename, just its btrfs inode number.
And it's exported while no one utilizes it out of mkfs/rootdir.c.
- Weird inode numbers
We use the inode number from st->st_ino, with an extra offset.
This by itself is not safe, if the rootdir has child directories in
another filesystem.
And this results very weird inode numbers, e.g:
item 0 key (256 INODE_ITEM 0) itemoff 16123 itemsize 160
item 6 key (88347519 INODE_ITEM 0) itemoff 15815 itemsize 160
item 16 key (88347520 INODE_ITEM 0) itemoff 15363 itemsize 160
item 20 key (88347521 INODE_ITEM 0) itemoff 15119 itemsize 160
item 24 key (88347522 INODE_ITEM 0) itemoff 14875 itemsize 160
item 26 key (88347523 INODE_ITEM 0) itemoff 14700 itemsize 160
item 28 key (88347524 INODE_ITEM 0) itemoff 14525 itemsize 160
item 30 key (88347557 INODE_ITEM 0) itemoff 14350 itemsize 160
item 32 key (88347566 INODE_ITEM 0) itemoff 14175 itemsize 160
Which is far from a regular fs created by copying the data.
- Weird directory inode size calculation
Unlike kernel, which updated the directory inode size every time new
child inodes are added, we calculate the directory inode size by
searching all its children first, then later new inodes linked to this
directory won't touch the inode size.
- Bad hard link detection and cross mount point handling
The hard link detection is purely based on the st_ino returned from
the host filesystem, this means we do not have extra checks whether
the inode is even inside the same fs.
And we directly reuse st_nlink from the host filesystem, if there
is a hard link out of rootdir, the st_nlink will be incorrect and
cause a corrupted fs.
Enhance all these points by:
- Use nftw() to do the preorder traversal
It also provides the extra level detection, which is pretty handy.
- Use a simple local inode_entry to record each parent
The only value is a u64 to record the inode number.
And one simple rootdir_path structure to record the list of
inode_entry, alone with the current level.
This rootdir_path structure along with two helpers,
rootdir_path_push() and rootdir_path_pop(), along with the
preorder traversal provided by nftw(), are enough for us to record
all the parent directories until the rootdir.
- Grab new inode number properly
Just call btrfs_get_free_objectid() to grab a proper inode number,
other than using some weird calculated value.
- Treat every inode as a new one
This means we will have no hard link support for now.
But I still believe it's a good trade-off, especially considering the
old handling is buggy for several corner cases.
- Use btrfs_insert_inode() and btrfs_add_link() to update directory
inode automatically
With all the refactoring, the code is shorter and easier to read.
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Qu Wenruo <wqu@suse.com>
We use the UASSERT() wrapper instead of the plain assert() as this can
be tuned to print the stack trace too if supported.
Signed-off-by: David Sterba <dsterba@suse.com>
All issues have been fixed in latest master, enable the checks for devel
too. It takes about 17m. Also rename the file, drop the "ci-" prefix.
Signed-off-by: David Sterba <dsterba@suse.com>
Fixes so 'python3 -m build' works and package can be uploaded to pypi
(https://pypi.org/project/btrfsutil/).
- setup.py is still used for local build (make)
- for pypi it must be done by 'python3 -m build' that is build in a
temporary directory
- btrfsutilpy.h must be also distributed
- version is set manually (the git VERSION file is not accessible)
- the project page metadata is empty, the README.md should be added
Issue: #310
Signed-off-by: David Sterba <dsterba@suse.com>
David Sterba
Filipe Manana
Qu Wenruo
Mark Harmstone
Han Yuwei
Matt Langford
rhn
ettavolt