For non-compressed non-hole file extent items, the ram_bytes should
match disk_num_bytes.
But due to kernel bugs, we have several cases where ram_bytes is not
correctly updated.
Thankfully this is really a very minor mismatch and can never cause data
corruption since the kernel does not utilize ram_bytes for
non-compressed extents at all.
So here we just detect and repair it for original mode.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
For non-compressed non-hole file extent items, the ram_bytes should
match disk_num_bytes.
But due to kernel bugs, we have several cases where ram_bytes is not
correctly updated.
Thankfully this is really a very minor mismatch and can never cause data
corruption since the kernel does not utilize ram_bytes for
non-compressed extents at all.
So here we just detect and repair it for lowmem mode.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
There is a new release candidate for e2fsprogs https://github.com/tytso/e2fsprogs/releases/tag/v1.47.1-rc2
Linking btrfs-progs v6.8 against this version of e2fsprogs leads to the following compile error:
convert/source-ext2.c: In function 'ext4_copy_inode_timespec_extra':
convert/source-ext2.c:733:13: warning: implicit declaration of function 'inode_includes' [-Wimplicit-function-declaration]
733 | if (inode_includes(inode_size, i_ ## xtime ## _extra)) { \
| ^~~~~~~~~~~~~~
convert/source-ext2.c:769:9: note: in expansion of macro 'EXT4_COPY_XTIME'
769 | EXT4_COPY_XTIME(atime, dst, tv_sec, tv_nsec);
| ^~~~~~~~~~~~~~~
convert/source-ext2.c:733:40: error: 'i_atime_extra' undeclared (first use in this function)
733 | if (inode_includes(inode_size, i_ ## xtime ## _extra)) { \
| ^~
convert/source-ext2.c:769:9: note: in expansion of macro 'EXT4_COPY_XTIME'
769 | EXT4_COPY_XTIME(atime, dst, tv_sec, tv_nsec);
| ^~~~~~~~~~~~~~~
convert/source-ext2.c:733:40: note: each undeclared identifier is reported only once for each function it appears in
733 | if (inode_includes(inode_size, i_ ## xtime ## _extra)) { \
| ^~
convert/source-ext2.c:769:9: note: in expansion of macro 'EXT4_COPY_XTIME'
769 | EXT4_COPY_XTIME(atime, dst, tv_sec, tv_nsec);
| ^~~~~~~~~~~~~~~
convert/source-ext2.c:733:40: error: 'i_mtime_extra' undeclared (first use in this function)
733 | if (inode_includes(inode_size, i_ ## xtime ## _extra)) { \
| ^~
convert/source-ext2.c:770:9: note: in expansion of macro 'EXT4_COPY_XTIME'
770 | EXT4_COPY_XTIME(mtime, dst, tv_sec, tv_nsec);
| ^~~~~~~~~~~~~~~
convert/source-ext2.c:733:40: error: 'i_ctime_extra' undeclared (first use in this function)
733 | if (inode_includes(inode_size, i_ ## xtime ## _extra)) { \
| ^~
convert/source-ext2.c:771:9: note: in expansion of macro 'EXT4_COPY_XTIME'
771 | EXT4_COPY_XTIME(ctime, dst, tv_sec, tv_nsec);
| ^~~~~~~~~~~~~~~
convert/source-ext2.c:774:40: error: 'i_crtime_extra' undeclared (first use in this function)
774 | if (inode_includes(inode_size, i_crtime_extra)) {
| ^~~~~~~~~~~~~~
from tytso/e2fsprogs@ca8bc92
Fix inode_includes() macro to properly wrap "inode" parameter,
and rename to ext2fs_inode_includes() to avoid potential name
clashes. Use this to check inode field inclusion in debugfs
instead of bare constants for inode field offsets.
To fix that use the new prefixed macro and add backward compatibility that
would still use inode_includes().
Issue: #785
Signed-off-by: David Sterba <dsterba@suse.com>
The inode_cache functionality is long gone and the 'rescue' group
provides the clearning functionality, no point keeping it in check.
Move the --clear-space-cache option to the deprecaeted section so it can
be removed soon.
Signed-off-by: David Sterba <dsterba@suse.com>
The test case relies on `--delete-qgroup` option which has been removed.
The feature needs to be implemented in kernel as it's more complex than
just calling an ioctl. The test case does not take the complexity of
subvolume dropping into consideration and only tested the simplest
cases.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
This reverts commit 9da773aa46.
There are several problems related to the --delete-qgroup option:
- Currently kernel doesn't allow to delete non-empty qgroups
- A qgroup can only be empty after fully dropped and a transaction is
committed
The tool doesn't take either factor into consideration
- Things like drop_subtree_threshold or other operations can mark qgroup
inconsistent and skip accounting
This can mean the target qgroup will never be empty until next rescan
On the other hand, even we do it the proper way, it would hugely delay
the command (wait until the subvolume to be cleaned).
Furthermore, even if the waiting is handled properly,
drop_subtree_threshold can still prevent us deleting the qgroup (qgroup
numbers are inconsistent, and accounting is skipped completely).
So the qgroup cleanup needs kernel to make it work properly.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
[BUG]
Since commit 8049446bb0 ("btrfs-progs: docs: placeholder for
contents.rst file on older sphinx version"), on systems with much newer
sphinx-build, "make" would not work for Documentation directory:
$ make clean-all && ./autogen.sh && ./configure --prefix=/usr/ && make -j12
$ ls -alh Documentation/_build
ls: cannot access 'Documentation/_build': No such file or directory
The sphinx-build has a much newer version:
$ sphinx-build --version
sphinx-build 7.2.6
[CAUSE]
On systems which don't need the workaround, the phony target of
contents.rst seems to cause a dependency loop:
GNU Make 4.4.1
Built for x86_64-pc-linux-gnu
Copyright (C) 1988-2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Reading makefiles...
Reading makefile 'Makefile'...
Updating makefiles....
Considering target file 'Makefile'.
Looking for an implicit rule for 'Makefile'.
Trying pattern rule '%:' with stem 'Makefile'.
Found implicit rule '%:' for 'Makefile'.
Finished prerequisites of target file 'Makefile'.
No need to remake target 'Makefile'.
Updating goal targets....
Considering target file 'contents.rst'.
File 'contents.rst' does not exist.
Finished prerequisites of target file 'contents.rst'.
Must remake target 'contents.rst'.
Makefile:35: update target 'contents.rst' due to: target is .PHONY
if [ "$(sphinx-build --version | cut -d' ' -f2)" \< "1.7.7" ]; then \
touch contents.rst; \
fi
Putting child 0x64ee81960130 (contents.rst) PID 66833 on the chain.
Live child 0x64ee81960130 (contents.rst) PID 66833
Reaping winning child 0x64ee81960130 PID 66833
Removing child 0x64ee81960130 PID 66833 from chain.
Successfully remade target file 'contents.rst'.
All the default make doing is just try to generate contents.rst, but
since we have much newer version, we won't generate the file at all.
[FIX]
Instead of a phony target, just move the contents.rst generation into
man page target so that we won't cause loop target on contents.rst.
Fixes: 8049446bb0 ("btrfs-progs: docs: placeholder for contents.rst file on older sphinx version")
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Add some missing entries. Changes to supported levels:
- increase to 6.8 from 6.7 where applicable, there were fixes to squota
and temp-fsid
- raid-stripe-tree declares support from 6.7, however this is still
behind CONFIG_BTRFS_DEBUG option in kernel, there are some bugs
and the known lack of RAID56 support
[ci skip]
Signed-off-by: David Sterba <dsterba@suse.com>
Use the objectid, type, offset natural order as it's more readable and
we're used to read keys like that.
Signed-off-by: David Sterba <dsterba@suse.com>
A user who wants to shrink a btrfs filesystem within some other logical
device (like a partition) will likely want to adapt the size of the
underlying device, too.
This commit adds documentation that describes how the length of the
portion that btrfs uses of some device can be found out.
Thanks go out to Roman Mamedov <rm@romanrm.net> for hinting `btrfs
filesystem show` as alternative command.
Note: the granularity is one sectorsize and the input values are
silently rounded down to avoid bugs from converted filesystems that
would not adhere to the native btrfs constraints.
[ci skip]
Pull-request: #775
Signed-off-by: Christoph Anton Mitterer <mail@christoph.anton.mitterer.name>
Signed-off-by: David Sterba <dsterba@suse.com>
Being able to expand (“can”) the partition beforehand is not enough – it must
actually be done.
Pull-request: #775
Signed-off-by: Christoph Anton Mitterer <mail@christoph.anton.mitterer.name>
Signed-off-by: David Sterba <dsterba@suse.com>
Copy the images from wiki so that we don't need to jump around the web
search results.
[ci skip]
Pull-request: #771
Signed-off-by: Austin Chang <austin880625@gmail.com>
Signed-off-by: David Sterba <dsterba@suse.com>
This is a followup to 884a609a77 ("btrfs-progs: add basename
wrappers for unified semantics"). Test cli/019-subvolume-create-parents
fails as there are paths with trailing slashes.
The GNU semantics does not change the argument of basename(3) but this
is problematic with trailing slashes. This is not uncommon and could
potentially break things.
To minimize impact of the basename behaviour depending on the include of
libgen.h use the single wrapper in path utils that has to include libgen
anyway for dirname. Our code passes writable buffers to basename.
Issue: #778
Signed-off-by: David Sterba <dsterba@suse.com>
What basename(3) does with the argument depends on _GNU_SOURCE and
inclusion of libgen.h. This is problematic on Musl (1.2.5) as reported.
We want the GNU semantics that does not modify the argument. Common way
to make it portable is to add own helper. This is now implemented in
path_basename() that does not use the libc provided basename but preserves
the semantics. The path_dirname() is just for parity, otherwise same as
dirname().
Sources:
- https://bugs.gentoo.org/926288
- https://git.musl-libc.org/cgit/musl/commit/?id=725e17ed6dff4d0cd22487bb64470881e86a92e7
Issue: #778
Signed-off-by: David Sterba <dsterba@suse.com>
Some steps don't seem to have a message printed when they start, like
the tree-log clearing or checksum fill phase.
Signed-off-by: David Sterba <dsterba@suse.com>
There's an early check of some critical roots right after opening the
filesystem but there's only one message. Check the same roots but print
message for each so it's more specific.
Signed-off-by: David Sterba <dsterba@suse.com>
Use the right helper for unrecognized options so only the unknown one is
printed instead of the whole help text. Also move the case to the end as
is common elsewhere.
Signed-off-by: David Sterba <dsterba@suse.com>
Reported by 'gcc -fanalyzer':
cmds/inspect.c:1193:1: warning: leak of ‘ctx.stats’ [CWE-401] [-Wanalyzer-malloc-leak]
There are mixed returns and gotos for error handling and the returns
miss freeing of the ctx.stats. Unify all paths to the single label that
frees the buffers and rename it.
Signed-off-by: David Sterba <dsterba@suse.com>
Reported by 'gcc -fanalyzer':
cmds/scrub.c:1150:25: warning: use of possibly-NULL ‘path’ where non-null expected [CWE-690] [-Wanalyzer-possible-null-argument]
Initialization of the datafile path is done from a static string but the
strdup() call is not handled. Store the path directly to the buffer,
it's later modified by mkdir_p().
Signed-off-by: David Sterba <dsterba@suse.com>
Reported by 'gcc -fanalyzer':
cmds/subvolume.c:1078:39: warning: use of possibly-NULL ‘name’ where non-null expected [CWE-690] [-Wanalyzer-possible-null-argument]
The failure name duplication is not handled and can potentially lead to
a NULL dereference later. Handle the error properly and return template
error message.
Signed-off-by: David Sterba <dsterba@suse.com>
Reported by 'gcc -fanalyzer':
libbtrfsutil/subvolume.c:415:20: warning: dereference of NULL ‘subvol’ [CWE-476] [-Wanalyzer-null-dereference]
The analyzer found a path where the NULL pointer passed as argument to
btrfs_util_subvolume_info_fd() could be dereferenced. This is unlikely
unless there's a corruption on the disk as the header->offset would have
to be 0. Pass a valid temporary buffer so this does not happen but
there's no use of it otherwise.
Signed-off-by: David Sterba <dsterba@suse.com>
Reported by 'gcc -fanalyzer':
common/utils.c:1203:9: warning: use of uninitialized value ‘data’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
There are several return parameters passed to
btrfs_get_string_for_multiple_profiles(), in case it fails early no
values are assigned so the free() would be called on some stack
initialization value. Initialize all the pointers.
Signed-off-by: David Sterba <dsterba@suse.com>
Reported by 'gcc -fanalyzer':
kernel-shared/print-tree.c:1745:12: warning: check of ‘eb’ for NULL after already dereferencing it [-Wanalyzer-deref-before-check]
The fs_info is initialized before we check 'eb' but we always get a
valid one so no need to validate it.
Signed-off-by: David Sterba <dsterba@suse.com>
Reported by 'gcc -fanalyzer':
kernel-shared/extent_io.c: In function ‘read_raid56’:
./include/kerncompat.h:393:18: warning: dereference of NULL ‘pointers’ [CWE-476] [-Wanalyzer-null-dereference]
After allocation of the pointers array fails it's dereferenced in the
exit block. We can return immediately instead.
Signed-off-by: David Sterba <dsterba@suse.com>
Reported by 'gcc -fanalyzer':
cmds/replace.c:357:17: warning: double ‘close’ of file descriptor ‘fdmnt’ [CWE-1341] [-Wanalyzer-fd-double-close]
The first close is done right before going to the label
'leave_with_error' but the variable is not reset to -1 so in the exit
block close() is called again.
Signed-off-by: David Sterba <dsterba@suse.com>
Reported by 'gcc -fanalyzer':
common/format-output.c:168:1: warning: missing call to ‘va_end’ [-Wanalyzer-va-list-leak]
There's a temporary va_list used infmt_set_unquoted() but va_copy() must
be paired with va_end(), which is missing.
Signed-off-by: David Sterba <dsterba@suse.com>
Reported by 'gcc -fanalyzer':
common/path-utils.c:401:16: warning: use of possibly-NULL ‘curr_dir’ where non-null expected [CWE-690] [-Wanalyzer-possible-null-argument]
There's an unhandled strdup() call in path_is_in_dir() so tmp could be
potentially NULL and passed down in the function. This is in the path
utilities so we assume the buffer is a path and can use the safe copy.
Signed-off-by: David Sterba <dsterba@suse.com>
Reported by 'gcc -fanalyzer:
common/string-table.c:62:17: warning: leak of ‘msg’ [CWE-401] [-Wanalyzer-malloc-leak]
The 'msg' still allocated when returning from the function due to error,
free it.
Signed-off-by: David Sterba <dsterba@suse.com>
Reported by 'gcc -fanalyzer':
common/device-scan.c:222:20: warning: dereference of NULL ‘device’ [CWE-476] [-Wanalyzer-null-dereference]
If the allocation of device fails then we can't free device->zone_info
at the out label. To fix that return immediately as it's at the
beginning of the function.
Signed-off-by: David Sterba <dsterba@suse.com>
The 'box' target is not that different from the default one regarding
dependencies so build it as well by default for better coverage.
Signed-off-by: David Sterba <dsterba@suse.com>
Add a new test case to make sure:
- btrfstune can convert a zoned btrfs with extent tree to bgt
- btrfstune can convert a zoned btrfs with bgt back to extent tree
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
[BUG]
There is a report that, for zoned devices btrfstune is unable to convert
it to block group tree.
# btrfstune /dev/nullb0 --convert-to-block-group-tree
Error reading 1342193664, -1
Error reading 1342193664, -1
ERROR: cannot read chunk root
ERROR: open ctree failed
[CAUSE]
For read-write opened zoned devices, all the read/write has to be
aligned to its sector size.
However btrfs stores its metadata by extent_buffer::data[], which has
all the structures before it, thus never aligned to zoned device sector
size.
Normally we would require btrfs_pread() and btrfs_pwrite() to do the
extra alignment, but during open_ctree(), we are not aware if a device
is zoned or not.
Thus we rely on if the fd is opened with O_DIRECT flag, if the fd has
O_DIRECT, then we would temporarily set fs_info->zoned for chunk tree
read.
Unforunately not all open_ctree_fd() callers have the flags set
properly, and btrfstune is one of the missing call site.
This makes all the read not properly aligned and cause read failure.
[FIX]
Just manually check if the target device is a zoned one, and set
O_DIRECT accordingly.
Issue: #765
Pull-request: #767
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
[BUG]
There is a bug report that mkfs.btrfs can not specify block-group-tree
feature along with zoned devices:
# mkfs.btrfs /dev/nullb0 -O block-group-tree,zoned
btrfs-progs v6.7.1
See https://btrfs.readthedocs.io for more information.
Resetting device zones /dev/nullb0 (40 zones) ...
NOTE: several default settings have changed in version 5.15, please make sure
this does not affect your deployments:
- DUP for metadata (-m dup)
- enabled no-holes (-O no-holes)
- enabled free-space-tree (-R free-space-tree)
ERROR: error during mkfs: Invalid argument
[CAUSE]
During mkfs, we need to write all the 7 or 8 tree blocks into the
metadata zone, and since it's zoned device, we need to fulfill all the
requirement for zoned writes, including:
- All writes must be in sequential bytenr
- Buffer must be aligned to sector size
The sequential bytenr requirement is already met by the mkfs design, but
the second requirement on memory alignment is never met for metadata, as
we put the contents of a leaf in extent_buffer::data[], which is after a
lot of small members.
Thus metadata IO buffer would never be aligned to sector size (normally
4K).
And we require btrfs_pwrite() and btrfs_pread() to handle the memory
alignment for us.
However in create_block_group_tree() we didn't use btrfs_pwrite(), but
plain pwrite() call directly, which would lead to -EINVAL error due to
memory alignment problem.
[FIX]
Just call btrfs_pwrite() instead of the plain pwrite() in
create_block_group_tree().
Issue: #765
Pull-request: #767
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
There is a missing newline for a successful
--convert-from-block-group-tree run, meanwhile
--convert-to-block-group-tree has the correct newline.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
It was reported in https://lore.kernel.org/linux-btrfs/e7ce9995-93cb-4904-875c-684d4494765f@web.de/
that compression does not happen on direct io files. This is incorrectly
documented that it works but this is not true. Compression works on
buffered writes and relies on page cache, while direct io avoids that
and takes a different path in code.
[ci skip]
Pull-request: #764
Author: HAN Yuwei <hrx@bupt.moe>
Signed-off-by: David Sterba <dsterba@suse.com>
Use the templated error message for transaction failures, use the same
pattern assigning the ret and errno.
Signed-off-by: David Sterba <dsterba@suse.com>
The send v3 protocol is enabled in kernel by a different config option
than in btrfs-progs to actually work. Now v3 can be tested when
configured and built with --enable-experimental.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: Boris Burkov <boris@bur.io>
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo
David Sterba
Christoph Anton Mitterer
Chris Mayo
Austin Chang
oluceps
HAN Yuwei
Thomas Bertels
Boris Burkov