Commit 4db925911c ("btrfs-progs: use strncpy_null everywhere") did
not properly convert the subvolume name copying to strncpy_null() and
trimmed the last character.
Issue: #829
Signed-off-by: David Sterba <dsterba@suse.com>
Remove the not needed encoding and reserved fields in struct
raid_stripe_extent.
This saves 8 bytes per stripe extent.
Note: this is a format change and previously created filesystems with
raid-stripe-tree will not be accessible. Similar patch is needed in
kernel.
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Enhance the rescue mount option group by:
- Add a simple explanation on each rescue option
- Add the new 'ignoremetacsums' option
Signed-off-by: Qu Wenruo <wqu@suse.com>
The CI lcov generation fails due to:
Processing ./common/device-utils.gcda
geninfo: ERROR: Unexpected negative count '-6' for /home/runner/work/btrfs-progs/btrfs-progs/common/device-utils.h:69.
Perhaps you need to compile with '-fprofile-update=atomic
(use "geninfo --ignore-errors negative ..." to bypass this error)
Use a safer way to gather the profile stats.
Signed-off-by: David Sterba <dsterba@suse.com>
There's a report on the CI after base ubuntu image update:
geninfo: WARNING:
/home/runner/work/btrfs-progs/btrfs-progs/common/device-scan.c:429:
unexecuted block on non-branch line with non-zero hit count. Use
"geninfo --rc geninfo_unexecuted_blocks=1 to set count to zero.
(use "geninfo --ignore-errors gcov,gcov ..." to suppress this warning)
Signed-off-by: David Sterba <dsterba@suse.com>
Free memory after errors in sum(), this is reported by gcc 13 on the CI.
This was reproduced by:
$ make D=asan TEST=019-receive-clones-on-mounted-subvol test-misc
Signed-off-by: David Sterba <dsterba@suse.com>
Test the following:
- Create a btrfs with crc32c csum
- Populate the filesystem
- Convert the filesystem to the following csums:
* xxhash
* blake2
* sha256
* crc32c
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
[BUG]
'btrfstune --csum' would always fail for a newly created btrfs:
# truncate -s 1G test.img
# ./mkfs.btrfs -f test.img
# ./btrsftune --csum xxhash test.img
WARNING: Experimental build with unstable or unfinished features
WARNING: Switching checksums is experimental, do not use for valuable data!
Proceed to switch checksums
ERROR: failed to start transaction: Unknown error -28
ERROR: failed to start transaction: No space left on device
ERROR: failed to generate new data csums: No space left on device
ERROR: btrfstune failed
[CAUSE]
After commit e79f18a4a7 ("btrfs-progs: introduce a basic metadata free
space reservation check"), btrfs_start_transaction() would check the
metadata space.
But at the time of introduction of csum conversion, the parameter for
btrfs_start_transaction() was incorrect.
The 2nd parameter is the *number* of items to be added (if we're
deleting items, just pass 1).
However commit 08a3bd7694 ("btrfs-progs: tune: add the ability to
generate new data checksums") is using the item size, not the number of
items to be added.
This means we're passing a number 8 * nodesize times larger than the
original size, no wonder we would error out with -ENOSPC.
[FIX]
Use proper calcuation to convert the new csum item size to number of
leaves needed, and double it just in case.
Pull-request: #820
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
The separator of key=value is only one or more space character, the
'encoded_write' also uses ',' which is inconsistent with the rest.
Signed-off-by: David Sterba <dsterba@suse.com>
The xattr names are user strings but still can potentially contain
special characters (as reported). There doesn't seem to be a restriction
on the name defined.
The xattr values care length-encoded byte arrays so escaping needs be
done.
The clone source is a path and by mistake lacked the encoding.
Issue: #818
Signed-off-by: David Sterba <dsterba@suse.com>
Use the safe version of strncpy that makes sure the string is
terminated.
To be noted:
- the conversion in scrub path handling was skipped
- sizes of device paths in some ioctl related structures is
BTRFS_DEVICE_PATH_NAME_MAX + 1
Recently gcc 13.3 started to detect problems with our use of strncpy
potentially lacking the null terminator, warnings like:
cmds/inspect.c: In function ‘cmd_inspect_logical_resolve’:
cmds/inspect.c:294:33: warning: ‘__builtin_strncpy’ specified bound 4096 equals destination size [-Wstringop-truncation]
294 | strncpy(mount_path, mounted, PATH_MAX);
| ^
Signed-off-by: David Sterba <dsterba@suse.com>
Now that there's only __strncpy_null we can drop the underscore and move
it to string-utils as it's a generic string function rather than
something for paths.
Signed-off-by: David Sterba <dsterba@suse.com>
The macro strncpy_null uses sizeof the first argument for the length,
but there are no checks and this works only for buffers with static
length, i.e. not pointers. This is error prone. Use the open coded
variant that makes the sizeof visible.
Signed-off-by: David Sterba <dsterba@suse.com>
The label is of a fixed size 256 bytes and expects the zero terminator.
Using __strncpy_null is correct as it makes sure there's always the zero
termination but the argument passed in skips the last character.
Signed-off-by: David Sterba <dsterba@suse.com>
This test case will check the following behviour:
- Regular zoned supported data/metadata profiles
Should success
- RST with zoned feature
Should fail for non-experimental builds
- zoned with data profiles that only RST supports
Should fail for non-experimental builds.
Meanwhile for experimental builds it should success and enable RST
feature automatically (verified in mkfs/030)
Pull-request: #813
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Currently raid-stripe-tree feature is still experimental as it requires
a BTRFS_DEBUG kernel to recognize it. To avoid confusion move it back
to experimental so regular users won't incorrectly set it.
And since RST is no longer supported by default, also change the RST
profile detection so that for non-experimental build we won't enable RST
according to the data profiles.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Currently we unconditionally run mkfs/029 and mkfs/030 as we export
RST feature through mkfs.btrfs as supported features.
But considering the mkfs.btrfs features don't match kernel support (only
a BTRFS_DEBUG kernel can support that), we're going to hide RST behind
experimental builds.
In that case RST related tests cases also need to be behind experimental
builds as regular builds of mkfs.btrfs will no longer support RST
feature.
Introduce two helpers:
- _test_config()
To verify if certain config is set to 1
- check_experimental_build()
A wrapper of "_test_config EXPERIMENTAL", and skip the test if
btrfs-progs has no experimental features.
So that test cases can be skipped for non-experimental builds.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
The raid-stripe-tree (RST) feature is for zoned devices to support extra
data profiles, and is not yet a stable feature (still requires
CONFIG_BTRFS_DEBUG enabled kernel to support it).
Furthermore the supported filesystems (ext*, reiserfs and ntfs) don't
even support zoned devices, and even we force RST support for
btrfs-convert, we would only create an empty tree for RST, as btrfs
convert would only result SINGLE data profile with SINGLE/DUP metadata
profile, neither needs RST at all.
Enabling RST for btrfs-convert would only cause problems for false test
failures as we incorrectly allow RST feature for btrfs-convert.
Fixes the problem by removing raid-stripe-tree support from
btrfs-convert and remove the test cases support for RST.
This patch is mostly reverting commit 346a381923 ("btrfs-progs:
convert: add raid-stripe-tree to allowed features"), but keeps the test
infrastructure to support bgt features for convert.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
[BUG]
There is a random failure for misc/028 in github CI, but unable to
reproduce locally.
[EXTRA DUMP]
The failure is at mount time, which is mostly out of our control (kernel
is provided by the CI image), but at least we can do extra kernel dump
if a mount fails.
This dump is done inside run_check() where if we detects there is a
"mount" word inside the command string.
The detection is not the best, it may detect other commands that
contains the word "mount", but it should be good enough to detect real
mount failure.
Pull-request: #815
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
[BUG]
There is a random chance that misc/055 would fail with stale qgroups
detected.
[CAUSE]
Commit 82f7d6c1d7 ("btrfs-progs: qgroup: handle stale qgroup deletion
more accurately") changed the behavior of "btrfs qgroup clear-stale"
that it will no longer try to remove qgroup under deletion.
And the test case itself relies on the return value of "btrfs qgroup
clear-stale" to do the extra wait for subvolume deletion.
This means after that commit, the test case would skip the wait if there
is a subvolume waiting for cleanup, resulting in a race window where the
subvolume can be dropped and become stale, and eventually trigger the
stale qgroup detection and cause false alerts.
[FIX]
Fix the test case by always wait for the subvolume to be dropped, so
that later "btrfs qgroup clear-stale" can properly remove all stale
qgroups.
Pull-request: #814
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
The test check/037-freespacetree-repair sometimes fails (does not in the
CI though) that the last unmount does not work due to -EBUSY. This
depends on timing, but what is obviously wrong is the 'rm' before
umount. The quoting of "*" does not remove all the files before
fallocate but is otherwise silent due to '-f'.
Add the run_check so it's recorded in the logs taht it happens and alos
add sudo so it can actually work. This makes the test work more
reliably.
Signed-off-by: David Sterba <dsterba@suse.com>
There's no Centos 9, use one of it's replacemnts for to continue the
familiy of LTS distros. No exceptions for feature support (zoned, udev).
Signed-off-by: David Sterba <dsterba@suse.com>
Fix the following issues in the test suite:
- lack of quoting for variables
- declare function variables local when missing (prevent accidental
overwrite of global variables)
- for variables with underscore in the name use plain "$VAR_NAME"
instead of { } (unless necessary)
- minor style adjustments like moving quotes to the end of the same
string
Signed-off-by: David Sterba <dsterba@suse.com>
The support of Leap 15.4 has ended, not much point to test build there
so move it to 15.6 (upper bound). The lower bound is still 15.3 to catch
potential backward compatibility problems.
The hub image exists (https://hub.docker.com/r/kdave/ci-opensuse-leap-15.6-x86_64).
Signed-off-by: David Sterba <dsterba@suse.com>
There's an update to CI hosted runners,
https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2404-Readme.md
- kernel 6.8
- e2fsprogs 1.47
- gcc 13.2
- clang 18.1.3
Switch the workflow files to use it as ubuntu-latest still points to the
22.04 version. The updated versions let us avoid workarounds due to old
version if e2fsprogs.
The musl 32bit build seems to fail so pin the version to the last one
where it's known to work.
Signed-off-by: David Sterba <dsterba@suse.com>
Although we already have a pretty good array defined for all
super/compat_ro/incompat flags, we still rely on a manually defined mask
to do the printing.
This can lead to easy de-sync between the definition and the flags.
Change it to automatically iterate through the array to calculate the
flags, and add the remaining super flags.
Pull-request: #810
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
[BUG]
There is a bug report that a canceled checksum conversion (still
experimental feature) resulted in unexpected super flags:
csum_type 0 (crc32c)
csum_size 4
csum 0x14973811 [match]
bytenr 65536
flags 0x1000000001
( WRITTEN |
CHANGING_FSID_V2 )
magic _BHRfS_M [match]
While for a filesystem under checksum conversion it should have either
CHANGING_DATA_CSUM or CHANGING_META_CSUM.
[CAUSE]
It turns out that, due to btrfs-progs keeps its own extra flags inside
its own ctree.h headers, not the shared uapi headers, we have
conflicting super flags:
kernel-shared/uapi/btrfs_tree.h:#define BTRFS_SUPER_FLAG_METADUMP_V2 (1ULL << 34)
kernel-shared/uapi/btrfs_tree.h:#define BTRFS_SUPER_FLAG_CHANGING_FSID (1ULL << 35)
kernel-shared/uapi/btrfs_tree.h:#define BTRFS_SUPER_FLAG_CHANGING_FSID_V2 (1ULL << 36)
kernel-shared/ctree.h:#define BTRFS_SUPER_FLAG_CHANGING_DATA_CSUM (1ULL << 36)
kernel-shared/ctree.h:#define BTRFS_SUPER_FLAG_CHANGING_META_CSUM (1ULL << 37)
Note that CHANGING_FSID_V2 is conflicting with CHANGING_DATA_CSUM.
[FIX]
Cross port the proper updated uapi headers into btrfs-progs, and remove
the definition from ctree.h.
This would change the value for CHANGING_DATA_CSUM and
CHANGING_META_CSUM, but considering they are experimental features, and
kernel would reject them anyway, the damage is not that huge and we can
accept such change before exposing it to end users.
Pull-request: #810
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
[BUG]
There is a long existing failure in my local VM that with any newer
kernel (6.x) the test case misc/004 would fail with ENOSPC during
balance:
[TEST] misc-tests.sh
[TEST/misc] 004-shrink-fs
failed: /home/adam/btrfs-progs/btrfs balance start -mconvert=single -sconvert=single -f /home/adam/btrfs-progs/tests/mnt
test failed for case 004-shrink-fs
make: *** [Makefile:547: test-misc] Error 1
[CAUSE]
With more testing, it turns out that just before the balance, the
filesystem still have several empty data block groups.
The reason is the new default discard=async behavior, as it also changes
the empty block groups to be async, this leave the empty block groups
there, resulting no extra space for the convert balance.
[FIX]
I do not understand why for loop block devices we also enable
discard, but at least disable discard for the test case so that we can
ensure the empty block groups get cleaned up properly.
Pull-request: #809
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
There's no obvious reason why there's -O1 instead -O2 which is commonly
used on distro builds. -O1 was enabled in c1690a3832 ("Switch to -O1
for optimizations to enable FORTIFY_SOURCE") for the fortify checks.
Signed-off-by: David Sterba <dsterba@suse.com>
Due to unknown cause the libbtrfsutil and libbtrfs are not built with
sanitizer libraries and the ASAN test does not succeed. This needs to be
analyzed why, for now disable it so CI can continue.
$ make D=asan test-libbtrfsutil
[PY] libbtrfsutil
==235341==ASan runtime does not come first in initial library list; you should either link runtime to your application or manually preload it with LD_PRELOAD.
Signed-off-by: David Sterba <dsterba@suse.com>
The test case always fails in my VM, with the following error:
$ sudo TEST=038\* make test-misc
[TEST] misc-tests.sh
[TEST/misc] 038-backup-root-corruption
Backup 2 not overwritten
test failed for case 038-backup-root-corruption
After more debugging, it turns out that there is nothing wrong except
the final check:
[ "$main_root_ptr" -ne "$backup_new_root_ptr" ] || _fail "Backup 2 not overwritten"
The _fail() is only triggered if the previous check returns false, which
is completely the opposite.
Furthermore on the github CI, the kernel commits 2 instead of 1
transaction, resulting the next slot never to match the current
generation/tree root.
The two bugs combined, github CI always passses the test case, while
for my VM which does the expected one transaction, it would always fail.
Fix it by:
- Use a proper "if [] then; fi" block to check the tree root bytenr
- Use the generation diff to calculate the expected backup root slot
- Log the full super block dump for debug usage
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
There is a bug report that for fuzzed image
bko-155621-bad-block-group-offset.raw, "btrfs check --mode=lowmem
--repair" would lead to an endless loop.
Unlike original mode, lowmem mode relies on the backref walk to properly
go through each root, but unfortunately inside __add_inline_refs() we
doesn't handle unknown backref types correctly, causing it never moving
forward thus deadloop.
Fix it by erroring out to prevent an endless loop.
Issue: #788
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
There is a bug report that with UBSAN enabled, fuzz/006 test case
crashes.
It turns out that the image bko-154021-invalid-drop-level.raw has
invalid dir items, that the name/data len is beyond the item.
And if we try to read beyond the eb boundary, UBSAN got triggered.
Normally in kernel tree-checker would reject such metadata in the first
place, but in btrfs-progs we can not be that strict or we cannot do a
lot of repair.
So here just enhance print_dir_item() to do extra sanity checks for
data/name len before reading the contents.
Issue: #805
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
ASAN build (make D=asan) detects a memory leak in
btrfs-corrupt-block inside debug_corrupt_sector().
This can be reproduced by fsck/013 test case.
The cause is pretty simple, we just malloc a sector and forgot to free
it.
Issue: #806
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Executing the script inside the directories as './test.sh' is not
supposed to work but could happen accidentally. With an exit after
attempting to source the we can fix that. Not all cases have been fixed
in f6bbe06c08 ("btrfs-progs: tests: add protection against running
out of test suite").
Signed-off-by: David Sterba <dsterba@suse.com>
The test case verifies behavior of ext4 unwritten extents:
- Create a unwritten (preallocated) extent on ext4
- Fill the on-disk extent with random garbage
This is to make sure if btrfs tries to read the on-disk data, it would
definitely get some garbage.
As I found sometimes mkfs.ext4 can fill the unused bg with zeros.
- Fill the preallocated file range with some data
This is to make sure btrfs-convert can handle mixed written and
unwritten ranges.
- Save the checksum of the file.
- Convert the fs
- Verify the checksum
For older btrfs-convert, there would be only one regular file extent,
and reading the file would read out some garbage and cause checksum to
mismatch.
For the fixed btrfs-convert, we punch holes for unwritten extents,
thus only the written part would be read out and match the checksum.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
