[BUG]
There is a bug report that, with very weird mount status, there can be
some mount source which can not be accessed:
/path/dev/exports fs 500G 57G 444G 12% /path/dev/exports
Strace shows we can not access the above mount source:
131065 stat("/path/dev/exports", 0x7ffed17b8e20) = -1 EACCES (Permission denied)
And lead to failed mount check:
131065 write(2, "ERROR: ", 7) = 7
131065 write(2, "cannot check mount status of /de"..., 56) = 56
131065 write(2, "\n", 1) = 1
[CAUSE]
The mount check is based on libblkid, which gives the mount source, and
for non-btrfs mounts, we call path_is_reg_or_block_device() to check if
we even need to continue checking.
But in above case, the mount source is another fs, and we can not access
the source.
So we error out causing the check_mounted() to return error.
[FIX]
There is never any guarantee we can access the mount source, but on the
other hand, I do not want to ignore all access failure for the mount
source.
Let test_status_for_mkfs() to only skip check_mounted() error if
@force_overwrite is true.
This would still keep the old strict checks on whether the target is
already mounted, but if the end user really knows that certain mount
source do not need to be checked, they can always pass "-f" option to
skip the false alerts.
Link: https://bugzilla.suse.com/show_bug.cgi?id=1223799
Reported-by: Jiri Belka <jiri.belka@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Recent patches updated stale qgroup handling, using 'unlinked' and
'dropped' where we otherwise use 'deleted' and 'cleaned'.
Signed-off-by: David Sterba <dsterba@suse.com>
Currently `btrfs qgroup show` command shows any 0 level qgroup without a
root backref as `<stale>`, which is not correct.
There are several more cases:
- Under deletion
The subvolume is not yet full dropped, but unlinked.
In that case we would not have a root backref item, but the qgroup is
not stale.
- Squota space holder
This is for squota mode, that a fully dropped subvolume still have
extents accounting on the already-gone subvolume.
In this case it's not stale either, and future accounting relies on
it.
This patch would add above special cases, and add an extra `SPECIAL
PATHS` section to explain all the cases, including `<stale>`.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
The current stale qgroup deletion doesn't handle the following cases at
all:
- It doesn't detect stale qgroups correctly
The current check is using the root backref, which means unlinked but
not yet fully dropped subvolumes would mark its corresponding qgroups
stale.
This is incorrect. The real stale check should be based on the root
item, not root backref.
- Squota non-empty but stale qgroups
Such qgroups can not and should not be deleted, as future accounting
still require them.
- Full accounting mode, stale qgroups but not empty
Since qgroup numbers are inconsistent already, it's common to have
such stale qgroups with non-zero numbers.
Now it's dependent on the kernel to determine whether such qgroup can
be deleted.
Address the above problems:
- Do root_item based detection
So that btrfs_qgroup::stale would properly indicate if there is a
subvolume root item for the qgroup.
- Do not attempt to delete squota stale but non-empty qgroups
- Attempt to delete stale but non-empty qgroups for full accounting mode
And deletion failure would not count as an error.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
This allows the users to identify if the running qgroup mode and whether
the numbers are already inconsistent.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Since qgroup numbers are only updated at transaction commit time, it's
better to do a sync before reading the quota tree, to reduce the chance
of uncommitted qgroup changes.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
The -m | -M option for btrfstune, sounds like metadata_uuid is being
changed which is wrong. The fsid is being changed the original fsid is
being copied into the metadata_uuid. So update the help text.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: David Sterba <dsterba@suse.com>
[BUG]
For simple quota mode btrfs, dump tree does not show the extra flags
correctly:
# mkfs.btrfs -f -O squota $dev
# btrfs inspect dump-tree -t quota $dev | grep QGROUP_STATUS -A1
item 0 key (0 QGROUP_STATUS 0) itemoff 16243 itemsize 40
version 1 generation 10 flags ON scan 0 enable_gen 7
Note just ON is shown, but squota has one extra bit set for it.
[CAUSE]
Just no support for the new flag.
[FIX]
Add the new flag support, also to be consistent with other flags string
output, add output for extra unknown flags.
With a hand crafted image, the output with unknown flags looks like
this:
item 0 key (0 QGROUP_STATUS 0) itemoff 16243 itemsize 40
version 1 generation 10 flags ON|SIMPLE_MODE|UNKNOWN(0xf00) scan 0 enable_gen 7
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
The new test case would contain a file system image, with the following
file:
item 4 key (257 INODE_ITEM 0) itemoff 15883 itemsize 160
generation 7 transid 8 size 16384 nbytes 16384
block group 0 mode 100600 links 1 uid 0 gid 0 rdev 0
sequence 258 flags 0x0(none)
atime 1714635006.328482575 (2024-05-02 17:00:06)
ctime 1714635013.394980640 (2024-05-02 17:00:13)
mtime 1714635013.394980640 (2024-05-02 17:00:13)
otime 1714635006.328482575 (2024-05-02 17:00:06)
item 5 key (257 INODE_REF 256) itemoff 15869 itemsize 14
index 2 namelen 4 name: file
item 6 key (257 EXTENT_DATA 0) itemoff 15816 itemsize 53
generation 7 type 1 (regular)
extent data disk byte 13631488 nr 1048576
extent data offset 0 nr 16384 ram 16384
extent compression 0 (none)
Note the ram bytes, which should be 1048576.
Furthermore, the inode size is truncated to 16K (originally 1M), so that
offset + num_bytes would still be no larger than ram_bytes.
So the only error is the mismatch between ram_bytes and disk_num_bytes
for the non-compressed data extent.
The image is hand crafted for now, as btrfs-corrupt-block doesn't not yet
support corrupting the ram_bytes field.
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 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>
Qu Wenruo
David Sterba
Anand Jain
Christoph Anton Mitterer
Chris Mayo
Austin Chang
oluceps