Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upqgroups usability v3 #139
Open
qgroups usability v3 #139
Conversation
…btrfs_match_dir_item_name
verify_dir_item() is called in btrfs_match_dir_item_name() to ensure we
won't search beyond item boundary and does extra filetype check.
However in the follow call chain, such extra filetype check can cause
problems:
1) btrfs_add_link()
|- check_dir_conflict()
|- btrfs_lookup_dir_index()
|- btrfs_match_dir_item_name()
And if we have an offending dir index whose filetype is invalid,
btrfs_match_dir_item_name() will return NULL, meaning no match dir
index is found.
So btrfs_add_link() will still try to insert a dir index, which may
have same key->offset and leading to duplicated dir index.
2) btrfs_unlink()
|- btrfs_lookup_dir_index()
|- btrfs_lookup_dir_index()
|- btrfs_match_dir_item_name()
For the same offending dir index with invalid filetype, this will
return NULL, and btrfs_unlink() will just consider there is no
existing dir_index and do nothing.
Leave an orphan and invalid dir_index hanging there forever.
The patch removes the extra filetype check, as "btrfs check" can already
handle invalid filetype correctly for both modes.
And this makes "btrfs check --repair --mode=lowmem" to delete the
offending dir index to repair it correctly.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: Su Yue <suy.fnst@cn.fujitsu.com>
Signed-off-by: David Sterba <dsterba@suse.com>
…handle corrupted name len Function btrfs_delete_one_dir_name() will check if the dir_item is the last content of the item, and delete the whole item if needed. However if @name_len of one dir_item/dir_index is corrupted and larger than the item size, the function will still try to treat it as partly remove, which will screw up the whole leaf. This patch will enhance the item deletion check, to cover corrupted name len, so in that case we just delete the whole item. Signed-off-by: Qu Wenruo <wqu@suse.com> Reviewed-by: Su Yue <suy.fnst@cn.fujitsu.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
In repair_extent_data_item(), path is not released if some errors occurs which causes extent buffer leak. Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
…ee_v2 In lowmem mode with '--repair', check_chunks_and_extents_v2 will fix accounting in block groups and clear the error bit BG_ACCOUNTING_ERROR. However, return value of check_btrfs_root() doesn't contain error bits. If extent tree is on error, lowmem repair always prints error and returns nonzero value even the filesystem is fine after repair. Introduce FATAL_ERROR for lowmem mode to represent negative return values since negative and positive can't be mixed in the bit operations. Then let check_btrfs_root() return error bits. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
…_item The variable @eb is assigned to leaf in fs_tree before insertion of backref. It will cause wrong parent of new inserted backref. Set @parent at beginning to fix the problem. Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
Original --check-data-csum option will skip the extra copy if the first copy matches csum. Since offline scrub (with recoverability report) is still out-of-tree, at least enhance --check-data-csum option to handle multiple copies. Signed-off-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
When --check-data-csum option found csum mismatch, btrfs check still return 0. Fix it so log-replay could automatically pause when it finds csum error. Signed-off-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
Since data csum mismatch is not a fatal error compared to fs/extent trees, continue check. Signed-off-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
…-csum No matter --check-data-csum is passed or not, btrfs check will always output message like "checking csum". This message could be a little confusing, change it according to --check-data-csum option. Signed-off-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
Move pin_down_tree_blocks from main.c to mode-common.c for further patches. And export pin_metadata_blocks to mode-common.h. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
Commit d17d666 ("btrfs-progs: lowmem check: Fix regression which screws up extent allocator") removes pin_metadata_blocks() from lowmem repair. So we have to find another way to exclude extents which should be occupied by existed tree blocks. Modify pin_down_tree_blocks() and rename it to traverse_tree_blocks for sharing cods between new function exclude_metadata_blocks(). * exclude_metadata_blocks() traverses and marks extents of all tree blocks dirty in fs_info->excluded_extents. * cleanup_excluded_extents() is responsible for cleanup. Export them to mode-common.h since they will be used both in original and lowmem modes. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
Excluding or pining all metadata blocks is time-inefficient for large storage filesystems. Here is another way to mark all metadata block groups full and allocate new chunks for COW. Then new reserved extents never overwrite extents. Introduce modify_block_groups_cache() to modify all blocks groups cache state and set all extents in block groups unfree in free space cache. mark/clear_block_groups_full() are wrappers of above function. Suggested-by: Qu Wenruo <wqu@suse.com> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
Introduce create_chunk_and_block_block_group() to allocate a new chunk and corresponding block group. The new function force_cow_in_new_chunk() firstly allocates new chunk and records its start. Then it modifies all metadata block groups cached and full. Finally it marks the new block group uncached and unfree. In the next COW, extents states will be updated automatically by cache_block_group(). New function try_to_force_cow_in_new_chunk() will try to mark block groups full, allocate a new chunk and records the start. If the last allocated chunk is almost full, a new chunk will be allocated. Suggested-by: Qu Wenruo <wqu@suse.com> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
Declare a global u64 variable last_allocated_chunk records start of the last chunk allocated by lowmem repair. Although global variable is not nice, it simplifies the code a lot. avoid_extents_overwrite() prefers to allocate a new chunk first. If it failed because of no space or wrong used bytes (like in fsck-tests/004), then it tries to exclude metadata blocks which costs a lot of time in large filesystems. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
…wmem If options '--init-extent-tree' and '--mode=lowmem' are both input, all metadata blocks will be traversed twice. First one is done by pin_metadata_blocks() in reinit_extent_tree(). Second one is in check_chunks_and_extents_v2(). Excluding instead of pinning metadata blocks before reinitializing th extent tree in lowmem can save some time. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
Since extents can be avoided to be overwrittten by excluding or chunk allocation. It's not necessary to do all repairs in one transaction. This patch removes parameter @trans of repair_extent_data_item(). repair_extent_data_item() calls try_avoid_extents_overwrite() and starts a transaction by itself. Note: This patch and next patches cause error in lowmem repair like: "Error: Commit_root already set when starting transaction". Such error will disappear after removing @trans finished. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
…item This patch removes the parameter @trans of delete_extent_item(). It calls try_avoid_extents_overwrite() and starts a transaction by itself. Note: This patch and next patches cause error in lowmem repair like: "Error: Commit_root already set when starting transaction". Such error will disappear after removing @trans finished. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
This patch removes parameter @trans of repair_chunk_item(). It calls try_avoid_extents_overwrite() and starts a transaction by itself. Note: This patch and next patches cause error in lowmem repair like: "Error: Commit_root already set when starting transaction". Such error will disappear after removing @trans finished. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
…item This patch removes parameter @trans of repair_extent_item(). It calls try_avoid_extents_overwrite() and starts a transaction by itself. Note: This patch and next patches cause error in lowmem repair like: "Error: Commit_root already set when starting transaction". Such error will disappear after removing @trans finished. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
This patch removes parameter @trans of check_leaf_items(). Note: This patch and next patches cause error in lowmem repair like: "Error: Commit_root already set when starting transaction". Such error will disappear after removing @trans finished. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
…ck_ref This patch removes parameter @trans of repair_tree_back_ref(). It calls try_avoid_extents_overwrite() and starts a transaction by itself. Note: This patch and next patches cause error in lowmem repair like: "Error: Commit_root already set when starting transaction". Such error will disappear after removing @trans finished. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
Remove parameters @trans of delete_extent_item() and walk_down_tree_v2(). Note: This patch and next patches cause error in lowmem repair like: "Error: Commit_root already set when starting transaction". Such error will disappear after removing @trans finished. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
Introduce repair_block_accounting() which calls btrfs_fix_block_accounting() to repair block group accouting. Replace btrfs_fix_block_accounting() with the new function. Note: This patch and next patches cause error in lowmem repair like: "Error: Commit_root already set when starting transaction". Such error will disappear after removing @trans finished. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
This new image misses one extent which leads lowmem mode to allocate new chunks in repair. Rename original image to no_extent_bad_dev.img. Because of its bad used bytes, it should let lowmem mode exclude blocks in repair. Due to problems of btrfs-image, choose xz as compression tool. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Signed-off-by: David Sterba <dsterba@suse.com>
Use visual markers that separate tests and individual commands run via the run_* helpers. Signed-off-by: David Sterba <dsterba@suse.com>
This parameter was introduced with the original implementation of the function but has never really been used, so just remove it. Signed-off-by: Nikolay Borisov <nborisov@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
Commit 17793e3 ("Btrfs-progs: extend the extent cache for the device extent") extended the cache extent APIs to support objectid to distinguish between phyisical extents with same dimensions but on different devices. However, it seems that this particular function is not used to allocate a device extent with accompanying objectid. Instead such extents as processed solely by insert_device_extent_record. Remove the unused code as a result. Signed-off-by: Nikolay Borisov <nborisov@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
This function is always called with objectid set to 0. So remove the parameter and statically set the ->objectid to 0 when allocating a new extent. Signed-off-by: Nikolay Borisov <nborisov@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
…inters Normally corrupted leaf should be caught by csum check, but sometimes corrupted item pointers (out of leaf range) can still pass csum check. In fact, our fsck/005 test case image has such corrupted item pointer and btrfs check can surprisingly fix it. Anyway, make print-tree to skip such item and remaining slots to avoid segfault. Please note that, in btrfs-progs we can't put such check into check_tree_block() nor do kernel level comprehensive check as under certain case, btrfs-progs can handle or even repair it. A strict check_tree_block() or backporting kernel btrfs_check_leaf() could break such test cases and reduce the utility of btrfs-progs. Issue: #128 Reported-by: Hubert Kario <hubert@kario.pl> Signed-off-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
kdave
force-pushed the
kdave:devel
branch
3 times, most recently
from
6eb37a2
to
220db00
kdave
force-pushed the
kdave:devel
branch
7 times, most recently
from
57d91fb
to
2635d7a
kdave
force-pushed the
kdave:devel
branch
3 times, most recently
from
e2dd5bf
to
8a416e8
kdave
force-pushed the
kdave:devel
branch
4 times, most recently
from
191d5f6
to
8fe2497
kdave
force-pushed the
kdave:devel
branch
5 times, most recently
from
7f2762e
to
07e6fa5
kdave
force-pushed the
kdave:devel
branch
2 times, most recently
from
33cb9f4
to
88ad76d
kdave
force-pushed the
kdave:devel
branch
3 times, most recently
from
a4c99d5
to
5bf1be5
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
jeffmahoney commentedMay 16, 2018
This is v3 of the qgroups usability patchset.
Changes since v2:
switch to libbtrfsutil.
unless -v is specified.
The most notable change is that this posting doesn't include the JSON
format patches since there was still some open discussion there. The
plumbing for passing global options around to commands is there, so we
really just need to decide on how we want to handle alternative formats.
For some commands, the formatting library that coreutils uses will
probably work but for qgroups show to represent nested groups, it's
unsuitable.