[BUG]
Btrfs-progs sometimes fails to find certain extent backref when
committing transaction.
The most reliable way to reproduce it is fsck-test/013 on 64K page sized
system:
[...]
adding new data backref on 315859712 root 287 owner 292 offset 0 found 1
btrfs unable to find ref byte nr 31850496 parent 0 root 2 owner 0 offset 0
Failed to find [30867456, 168, 65536]
Also there are some github bug reports related to this problem.
[CAUSE]
Commit 909357e867 ("btrfs-progs: Wire up delayed refs") introduced
delayed refs in btrfs-progs.
However in that commit, delayed refs are not run at correct timing.
That commit calls btrfs_run_delayed_refs() before
btrfs_write_dirty_block_groups(), which needs to update
BLOCK_GROUP_ITEMs in extent tree, thus could cause new delayed refs.
This means each time we commit a transaction, we may screw up the extent
tree by dropping some pending delayed refs, like:
Transaction 711:
btrfs_commit_transaction()
|- btrfs_run_delayed_refs()
| Now all delayed refs are written to extent tree
|
|- btrfs_write_dirty_block_groups()
| Needs to update extent tree root
| ADD_DELAYED_REF to 315859712.
| Delayed refs are attached to current trans handle.
|
|- __commit_transaction()
|- write_ctree_super()
|- btrfs_finish_extent_commit()
|- kfree(trans)
Now delayed ref for 315859712 are lost
Transaction 712:
Tree block 315859712 get dropped
btrfs_commit_transaction()
|- btrfs_run_delayed_refs()
|- run_one_delayed_ref()
|- __free_extent()
As previous ADD_DELAYED_REF to 315859712 is lost, extent tree
doesn't have any backref for 315859712, causing the bug
In fact, commit c31edf610c ("btrfs-progs: Fix false ENOSPC alert by
tracking used space correctly") detects the tree block leakage, but in
the reproducer we have too much noise, thus nobody notices the leakage
warning.
[FIX]
We can't just move btrfs_run_delayed_refs() after
btrfs_write_dirty_block_groups(), as during btrfs_run_delayed_refs(), we
can re-dirty block groups.
Thus we need to exhaust both delayed refs and dirty blocks.
This patch will call btrfs_write_dirty_block_groups() and
btrfs_run_delayed_refs() in a loop until both delayed refs and dirty
blocks are exhausted. Much like what we do in commit_cowonly_roots() in
kernel.
Also, to prevent such problem from happening again (and not to debug
such problem again), add extra check on delayed refs before freeing the
transaction handle.
Reported-by: Klemens Schölhorn <klemens@schoelhorn.eu>
Issue: #187
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
At least in Debian, default build flags include -Werror=format-security,
for good reasons in most cases. Here, the string comes from strftime --
and though I don't suspect any locale would be crazy enough to have %X
include a '%' char, the compiler has no way to know that.
Signed-off-by: Adam Borowski <kilobyte@angband.pl>
Signed-off-by: David Sterba <dsterba@suse.com>
The test cli-tests/008-subvolume-get-set-default fails when compiled
with 'D=ubsan', the access to search header items does not follow the
type alignment, so use the accessors.
The error:
subvolume get-default: default id is not 256, but
libbtrfsutil/subvolume.c:361:13: runtime error: member access within
misaligned address 0x7ffc147e4b6f for type 'const struct
btrfs_ioctl_search_header', which requires 8 byte alignment
Note that using the accessors does not fix the ubsan warning, as it
warns on taking the address of a member whose _base_ type is unaligned,
ie. it's the 'sh'.
Fixing that would need to play tricks with pointers to do &sh->type
manually, but to avoid triggering ubsan.
Signed-off-by: David Sterba <dsterba@suse.com>
Add helpers that do proper unaligned access of search heade items. This
is done in the non-libbtrfsutil code already, use the same helpers here
too. We can't use the get_unaligned_* helpers that are defined in
kerncompat, so use plain memcpy that will work everywhere.
Signed-off-by: David Sterba <dsterba@suse.com>
'make clean' followed by 'make test-clean' will fail due to the sanity
check for 'btrfs' binary. We don't need that for cleaning the test, so
turn the error into a warning.
Signed-off-by: David Sterba <dsterba@suse.com>
Add support for changing all integer items in the superblock. Sort them
in the order of defintion in the structure.
Signed-off-by: David Sterba <dsterba@suse.com>
The field type bit width is now only used to describe the members, but
let's add all of the integer ones.
Signed-off-by: David Sterba <dsterba@suse.com>
The CI build prints a lot of warnings
[CC] btrfs.o
In file included from volumes.h:22,
from btrfs.c:22:
kerncompat.h:39: warning: "__always_inline" redefined
#define __always_inline __inline __attribute__ ((__always_inline__))
so define the macro conditionally.
Signed-off-by: David Sterba <dsterba@suse.com>
The shell keyword function is not necessary and not used in many tests,
remove it from the few places that use it right now.
Signed-off-by: David Sterba <dsterba@suse.com>
The comman 'btrfs inspect dump-tree <dev>' will scan all the devices
from the filesystem by defaul.
So as of now you can not inspect each mirrored device independently.
This patch adds option --noscan, which when used won't scan the system
for the partner devices, instead it just uses the devices provided in
the argument.
For example:
btrfs inspect dump-tree --noscan <dev> [<dev>..]
This helps to debug degraded raid1 and raid10.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Build several standalone tools into one binary and switch the function
by name (symlink or hardlink).
* btrfs
* mkfs.btrfs
* btrfs-image
* btrfs-convert
* btrfstune
The static target is also supported. The name of resulting boxed
binaries is btrfs.box and btrfs.box.static . All the binaries can be
built at the same time without prior configuration.
text data bss dec hex filename
822454 27000 19724 869178 d433a btrfs
927314 28816 20812 976942 ee82e btrfs.box
2067745 58004 44736 2170485 211e75 btrfs.static
2627198 61724 83800 2772722 2a4ef2 btrfs.box.static
File sizes:
857496 btrfs
968536 btrfs.box
2141400 btrfs.static
2704472 btrfs.box.static
Standalone utilities:
512504 btrfs-convert
495960 btrfs-image
471224 btrfstune
491864 mkfs.btrfs
1747720 btrfs-convert.static
1411416 btrfs-image.static
1304256 btrfstune.static
1361696 mkfs.btrfs.static
So the shared 900K binary saves ~2M, or ~5.7M for static build.
Signed-off-by: David Sterba <dsterba@suse.cz>
The documentation lacks clarity about depth to which recursive
'fi du' goes, and was pointed out by a user.
Add test that creates another mount inside a filesystem and verifies
that 'fi du' does not go there.
Issue: #185
Signed-off-by: David Sterba <dsterba@suse.com>
State that defrag by default does not descend to subvolumes, this has
been hidden in implementation details.
Issue: #185
Signed-off-by: David Sterba <dsterba@suse.com>
The documentation lacks clarity about depth to which recursive
defragmentation go, and was pointed out by a user.
The problem here is that the subvolume behaves the same as mount point
regarding path traversal. The nftw stops on mount boundary (FTW_MOUNT).
Add test that verifies this behaviour. Defrag has to be updated to allow
descending to subvolumes (and not mountpoints).
Issue: #185
Signed-off-by: David Sterba <dsterba@suse.com>
fgets consumes n-1 bytes from input buffer.
When a user types y\n, the newline is left in the buffer. As a result,
the next fgets uses that \n as answer without waiting for the user to
type.
This patch also fix a bug that dereference the ret without checking if
it's NULL.
* Consumes the `\n` from stdin buffer
* Avoid NULL pointer dereference: treat EOF as default value
Pull-request: #182
Author: pjw91 <mail6543210@yahoo.com.tw>
Signed-off-by: David Sterba <dsterba@suse.com>
The travis-ci fails at test misc-tests/021-image-multi-devices because
the 'btrfs check' is not run with root permissions, unlike all the other
commands. The check is read-only by default, so that should be safe.
Signed-off-by: David Sterba <dsterba@suse.com>
The transfer lines from dd bloat the logs and other lines may not fit.
Disable xfer in all dd commands but still allow errors to be caught.
Signed-off-by: David Sterba <dsterba@suse.com>
Rename the directory for continuous integration scripts to a more
generic name as we're going to use more than one. The base image on
travis has an old kernel. It's not possible to use a newer one and some
tests fail making the coverage unreliable.
Signed-off-by: David Sterba <dsterba@suse.com>
Gcc version 9.1.1 reports:
In file included from /usr/include/string.h:494,
from kerncompat.h:25,
from ctree.h:26,
from send-utils.c:26:
In function ‘memset’,
inlined from ‘btrfs_read_root_item’ at send-utils.c:165:3,
inlined from ‘subvol_uuid_search2’ at send-utils.c:494:8:
/usr/include/bits/string_fortified.h:71:10: warning: ‘__builtin_memset’ offset [248, 439] from the object at ‘root_item’ is out of the bounds of referenced subobject ‘generation_v2’ with type ‘long long unsigned int’ at offset 239 [-Warray-bounds]
71 | return __builtin___memset_chk (__dest, __ch, __len, __bos0 (__dest));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
That's correct in case the intent is to overwrite just the generation_v2
member, but we want to zero the rest of the root item structure starting
from the generation_v2. No typecasts can obscure that from gcc so the
starting address is calculated as base pointer + member offset.
Signed-off-by: David Sterba <dsterba@suse.com>
When a scrub completes or is cancelled, statistics are updated for
reporting in a later btrfs scrub status command and for resuming the
scrub. Most statistics (such as bytes scrubbed) are additive so scrub
adds the statistics from the current run to the saved statistics.
However, the last_physical statistic is not additive. The value from the
current run should replace the saved value. The current code incorrectly
adds the last_physical from the current run to the previous saved value.
This bug causes the resume point to be incorrectly recorded, so large
areas of the disk are skipped when the scrub resumes. As an example,
assume a disk had 1000000 bytes and scrub was cancelled and resumed each
time 10% (100000 bytes) had been scrubbed.
Run | Start byte | bytes scrubbed | kernel last_physical | saved last_physical
1 | 0 | 100000 | 100000 | 100000
2 | 100000 | 100000 | 200000 | 300000
3 | 300000 | 100000 | 400000 | 700000
4 | 700000 | 100000 | 800000 | 1500000
5 | 1500000 | 0 | immediately completes| completed
In this example, only 40% of the disk is actually scrubbed.
This patch changes the saved/displayed last_physical to track the last
reported value from the kernel.
Signed-off-by: Graham R. Cobb <g.btrfs@cobb.uk.net>
Signed-off-by: David Sterba <dsterba@suse.com>
Since the commmit 8dd3e5dc2d
("btrfs-progs: tests: fix misc-tests/029 to run on NFS") added the
compatibility of NFS, it called run_mayfail() in the last of the test.
However, run_mayfail() always return the original code. If the test
case is not running on NFS, the last `run_mayfail rmdir "$SUBVOL_MNT"`
will fail with return value 1 then the test fails:
================================================================
====== RUN MAYFAIL rmdir btrfs-progs/tests/misc-tests/029-send-p-different-mountpoints/subvol_mnt
rmdir: failed to remove 'btrfs-progs/tests/misc-tests/029-send-p-different-mountpoints/subvol_mnt': No such file or director
failed (ignored, ret=1): rmdir btrfs-progs/tests/misc-tests/029-send-p-different-mountpoints/subvol_mnt
test failed for case 029-send-p-different-mountpoints
=================================================================
Every instrument in this script handles its error well, so do exit 0
manually in the last.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=202645
Fixes: 8dd3e5dc2d ("btrfs-progs: tests: fix misc-tests/029 to run on NFS")
Signed-off-by: Su Yue <Damenly_Su@gmx.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Add structures and API for unified output definition and multiple
formatting backends. Currently there's plain text and json.
The format of each row is defined in struct rowspec, selected using a
key and formatted according to the type. There are extended types for
eg. UUID or pretty size, while direct printf format specifiers work too.
Due to different nature of the outputs, the context structure members
are not always used.
* text output mostly uses indentation and formats the name to a given
width
* json output tracks nesting depth and keeps stack of previous groups
(list or array) and how many member have been printed, as the
separators are allowed only between values and must not preced the
group closing bracket
the nesting depth is hardcoded to 16, counting the global group
The API provides functions to print simple values and some helpers to
format more complex structures.
Signed-off-by: David Sterba <dsterba@suse.com>
Global options should be printed right after the command options, but
there could be text following the options. Add a marker that will allow
to order the options before that text.
Signed-off-by: David Sterba <dsterba@suse.com>
This adds a global --format option to request extended output formats
from each command.
We currently only support text mode. Command help reports what
output formats are available for each command. Global help reports
what valid formats are.
If an invalid format is requested, an error is reported and lists the
valid formats.
Each command sets a bitmask that describes which formats it is capable
of outputting. If a globally valid format is requested of a command
that doesn't support it, an error is reported and command usage dumped.
Commands don't need to specify that they support text output. All
commands are required to output text.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
[ use global config instead of passing cmd_context ]
Signed-off-by: David Sterba <dsterba@suse.com>
For options that do not have the long description, the empty string is
required to mark where the options start. Some commands were missing
that.
Signed-off-by: David Sterba <dsterba@suse.com>
Qu Wenruo