From 0bbf8b92a7c7ed134e9b918a046cdd1715978252 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Fri, 19 Apr 2024 23:41:01 +0200 Subject: [PATCH] btrfs-progs: check: add more transaction error handling Update error handling where a transaction start may not be paired with a commit (that actually checks errors). Add aborts where missing. Signed-off-by: David Sterba --- check/main.c | 162 +++++++++++++++++++++++++++++++++++--------- check/mode-common.c | 35 ++++++---- check/mode-lowmem.c | 137 ++++++++++++++++++++++++++++--------- 3 files changed, 256 insertions(+), 78 deletions(-) diff --git a/check/main.c b/check/main.c index 693f77c354..93af325f25 100644 --- a/check/main.c +++ b/check/main.c @@ -2187,7 +2187,11 @@ static int add_missing_dir_index(struct btrfs_root *root, key.type = BTRFS_DIR_INDEX_KEY; key.offset = backref->index; ret = btrfs_insert_empty_item(trans, root, &path, &key, data_size); - BUG_ON(ret); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_START_TRANS, "%m"); + return ret; + } leaf = path.nodes[0]; dir_item = btrfs_item_ptr(leaf, path.slots[0], struct btrfs_dir_item); @@ -2204,7 +2208,12 @@ static int add_missing_dir_index(struct btrfs_root *root, write_extent_buffer(leaf, backref->name, name_ptr, backref->namelen); btrfs_mark_buffer_dirty(leaf); btrfs_release_path(&path); - btrfs_commit_transaction(trans, root); + ret = btrfs_commit_transaction(trans, root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_START_TRANS, "%m"); + return ret; + } backref->found_dir_index = 1; dir_rec = get_inode_rec(inode_cache, backref->dir, 0); @@ -2258,7 +2267,11 @@ static int delete_dir_index(struct btrfs_root *root, ret = btrfs_delete_one_dir_name(trans, root, &path, di); BUG_ON(ret); btrfs_release_path(&path); - btrfs_commit_transaction(trans, root); + ret = btrfs_commit_transaction(trans, root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_START_TRANS, "%m"); + } return ret; } @@ -2295,8 +2308,18 @@ static int create_inode_item(struct btrfs_root *root, ret = insert_inode_item(trans, root, rec->ino, size, rec->nbytes, nlink, mode); - btrfs_commit_transaction(trans, root); - return 0; + if (ret < 0) { + btrfs_abort_transaction(trans, ret); + btrfs_commit_transaction(trans, root); + return ret; + } + + ret = btrfs_commit_transaction(trans, root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_START_TRANS, "%m"); + } + return ret; } static int repair_inode_backrefs(struct btrfs_root *root, @@ -2390,8 +2413,17 @@ static int repair_inode_backrefs(struct btrfs_root *root, backref->dir, &location, imode_to_type(rec->imode), backref->index); - BUG_ON(ret); - btrfs_commit_transaction(trans, root); + if (ret < 0) { + btrfs_abort_transaction(trans, ret); + btrfs_commit_transaction(trans, root); + break; + } + ret = btrfs_commit_transaction(trans, root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); + break; + } repaired++; } @@ -3074,8 +3106,12 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec) ret = repair_unaligned_extent_recs(trans, root, &path, rec); if (!ret && rec->errors & I_ERR_INVALID_GEN) ret = repair_inode_gen_original(trans, root, &path, rec); - btrfs_commit_transaction(trans, root); btrfs_release_path(&path); + ret = btrfs_commit_transaction(trans, root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); + } return ret; } @@ -3184,7 +3220,12 @@ static int check_inode_recs(struct btrfs_root *root, return ret; } - btrfs_commit_transaction(trans, root); + ret = btrfs_commit_transaction(trans, root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); + return ret; + } return -EAGAIN; } @@ -3662,8 +3703,12 @@ static int repair_btree(struct btrfs_root *root, cache = next_cache_extent(cache); } out: - btrfs_commit_transaction(trans, root); btrfs_release_path(&path); + ret = btrfs_commit_transaction(trans, root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); + } return ret; } @@ -4605,6 +4650,7 @@ static int try_to_fix_bad_block(struct btrfs_root *root, ret = btrfs_search_slot(trans, search_root, &key, &path, 0, 1); if (ret) { ret = -EIO; + btrfs_abort_transaction(trans, ret); btrfs_commit_transaction(trans, search_root); break; } @@ -4613,11 +4659,17 @@ static int try_to_fix_bad_block(struct btrfs_root *root, else if (status == BTRFS_TREE_BLOCK_INVALID_OFFSETS) ret = fix_item_offset(search_root, &path); if (ret) { + btrfs_abort_transaction(trans, ret); btrfs_commit_transaction(trans, search_root); break; } btrfs_release_path(&path); - btrfs_commit_transaction(trans, search_root); + ret = btrfs_commit_transaction(trans, search_root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); + break; + } } ulist_free(roots); btrfs_release_path(&path); @@ -7843,6 +7895,7 @@ static int fixup_extent_flags(struct extent_record *rec) ret = btrfs_search_slot(trans, root, &key, &path, 0, 1); if (ret < 0) { btrfs_release_path(&path); + btrfs_abort_transaction(trans, ret); btrfs_commit_transaction(trans, root); return ret; } else if (ret) { @@ -7852,7 +7905,12 @@ static int fixup_extent_flags(struct extent_record *rec) } fprintf(stderr, "Didn't find extent for %llu\n", rec->start); btrfs_release_path(&path); - btrfs_commit_transaction(trans, root); + ret = btrfs_commit_transaction(trans, root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); + return ret; + } return -ENOENT; } @@ -7870,8 +7928,12 @@ static int fixup_extent_flags(struct extent_record *rec) btrfs_mark_buffer_dirty(path.nodes[0]); btrfs_release_path(&path); ret = btrfs_commit_transaction(trans, root); - if (!ret) + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); + } else { fprintf(stderr, "Repaired extent flags for %llu\n", rec->start); + } return ret; } @@ -7950,7 +8012,7 @@ static int prune_corrupt_blocks(void) struct btrfs_trans_handle *trans = NULL; struct cache_extent *cache; struct btrfs_corrupt_block *corrupt; - int ret; + int ret = 0; while (1) { cache = search_cache_extent(gfs_info->corrupt_blocks, 0); @@ -7969,9 +8031,14 @@ static int prune_corrupt_blocks(void) prune_one_block(trans, corrupt); remove_cache_extent(gfs_info->corrupt_blocks, cache); } - if (trans) - return btrfs_commit_transaction(trans, gfs_info->tree_root); - return 0; + if (trans) { + ret = btrfs_commit_transaction(trans, gfs_info->tree_root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); + } + } + return ret; } static int record_unaligned_extent_rec(struct extent_record *rec) @@ -8096,13 +8163,19 @@ static int repair_extent_item_generation(struct extent_record *rec) /* Not possible */ if (ret == 0) ret = -EUCLEAN; - if (ret < 0) + if (ret < 0) { + btrfs_abort_transaction(trans, ret); + btrfs_commit_transaction(trans, extent_root); goto out; + } ret = btrfs_previous_extent_item(extent_root, &path, rec->start); if (ret > 0) ret = -ENOENT; - if (ret < 0) + if (ret < 0) { + btrfs_abort_transaction(trans, ret); + btrfs_commit_transaction(trans, extent_root); goto out; + } if (!new_gen) new_gen = trans->transid; @@ -8120,10 +8193,6 @@ static int repair_extent_item_generation(struct extent_record *rec) rec->generation = new_gen; out: btrfs_release_path(&path); - if (ret < 0) { - btrfs_abort_transaction(trans, ret); - btrfs_commit_transaction(trans, extent_root); - } return ret; } @@ -8343,8 +8412,11 @@ static int check_extent_refs(struct btrfs_root *root, goto repair_abort; } ret = btrfs_commit_transaction(trans, root); - if (ret) + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); goto repair_abort; + } } return ret; } @@ -8826,8 +8898,18 @@ static int check_block_groups(struct block_group_tree *bg_cache) } ret = btrfs_fix_block_accounting(trans); - btrfs_commit_transaction(trans, gfs_info->tree_root); - return ret ? ret : -EAGAIN; + if (ret < 0) { + btrfs_abort_transaction(trans, ret); + btrfs_commit_transaction(trans, gfs_info->tree_root); + return ret; + } + ret = btrfs_commit_transaction(trans, gfs_info->tree_root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); + return ret; + } + return -EAGAIN; } /** @@ -9646,14 +9728,24 @@ static int delete_bad_item(struct btrfs_root *root, struct bad_item *bad) } ret = btrfs_search_slot(trans, root, &bad->key, &path, -1, 1); - if (ret) { - if (ret > 0) - ret = 0; + if (ret < 0) { + btrfs_abort_transaction(trans, ret); + btrfs_commit_transaction(trans, root); goto out; } + if (ret > 0) { + ret = 0; + goto out_commit; + } ret = btrfs_del_item(trans, root, &path); -out: + if (ret < 0) { + btrfs_abort_transaction(trans, ret); + btrfs_commit_transaction(trans, root); + goto out; + } +out_commit: btrfs_commit_transaction(trans, root); +out: btrfs_release_path(&path); return ret; } @@ -9998,8 +10090,14 @@ static int repair_root_items(void) out: free_roots_info_cache(); btrfs_release_path(&path); - if (trans) - btrfs_commit_transaction(trans, gfs_info->tree_root); + if (trans) { + ret = btrfs_commit_transaction(trans, gfs_info->tree_root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); + return ret; + } + } if (ret < 0) return ret; diff --git a/check/mode-common.c b/check/mode-common.c index 6f3ea00144..8efa884be1 100644 --- a/check/mode-common.c +++ b/check/mode-common.c @@ -727,7 +727,7 @@ int reset_imode(struct btrfs_trans_handle *trans, struct btrfs_root *root, iitem = btrfs_item_ptr(leaf, slot, struct btrfs_inode_item); btrfs_set_inode_mode(leaf, iitem, mode); btrfs_mark_buffer_dirty(leaf); - return ret; + return 0; } static int find_file_type_dir_index(struct btrfs_root *root, u64 ino, u64 dirid, @@ -1006,16 +1006,20 @@ int repair_imode_common(struct btrfs_root *root, struct btrfs_path *path) btrfs_release_path(path); ret = reset_imode(trans, root, path, key.objectid, imode); - if (ret < 0) - goto abort; + if (ret < 0) { + btrfs_abort_transaction(trans, ret); + btrfs_commit_transaction(trans, root); + return ret; + } ret = btrfs_commit_transaction(trans, root); - if (!ret) - printf("reset mode for inode %llu root %llu\n", - key.objectid, root->root_key.objectid); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); + return ret; + } + printf("reset mode for inode %llu root %llu\n", + key.objectid, root->root_key.objectid); return ret; -abort: - btrfs_abort_transaction(trans, ret); - return ret; } /* @@ -1084,7 +1088,11 @@ int recow_extent_buffer(struct btrfs_root *root, struct extent_buffer *eb) btrfs_item_key_to_cpu(eb, &key, 0); ret = btrfs_search_slot(trans, root, &key, &path, 0, 1); - btrfs_commit_transaction(trans, root); + ret = btrfs_commit_transaction(trans, root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); + } btrfs_release_path(&path); return ret; } @@ -1183,7 +1191,9 @@ int repair_dev_item_bytes_used(struct btrfs_fs_info *fs_info, if (ret < 0) { errno = -ret; error("failed to update device item for devid %llu: %m", devid); - goto error; + btrfs_abort_transaction(trans, ret); + btrfs_commit_transaction(trans, fs_info->chunk_root); + return ret; } /* @@ -1199,9 +1209,6 @@ int repair_dev_item_bytes_used(struct btrfs_fs_info *fs_info, device->bytes_used); } return ret; -error: - btrfs_abort_transaction(trans, ret); - return ret; } static int populate_csum(struct btrfs_trans_handle *trans, diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c index a9908eaf62..fd9b975c4e 100644 --- a/check/mode-lowmem.c +++ b/check/mode-lowmem.c @@ -362,7 +362,9 @@ static int create_chunk_and_block_group(u64 flags, u64 *start, u64 *nbytes) if (ret) { errno = -ret; error("fail to allocate new chunk %m"); - goto out; + btrfs_abort_transaction(trans, ret); + btrfs_commit_transaction(trans, root); + return ret; } ret = btrfs_make_block_group(trans, gfs_info, 0, flags, *start, *nbytes); @@ -370,10 +372,16 @@ static int create_chunk_and_block_group(u64 flags, u64 *start, u64 *nbytes) errno = -ret; error("fail to make block group for chunk %llu %llu %m", *start, *nbytes); - goto out; + btrfs_abort_transaction(trans, ret); + btrfs_commit_transaction(trans, root); + return ret; + } + + ret = btrfs_commit_transaction(trans, root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); } -out: - btrfs_commit_transaction(trans, root); return ret; } @@ -577,26 +585,36 @@ static int delete_item(struct btrfs_root *root, struct btrfs_path *path) btrfs_release_path(path); ret = btrfs_search_slot(trans, root, &key, path, -1, 1); if (ret) { + error("failed to delete root %llu item[%llu, %u, %llu]", + root->objectid, key.objectid, key.type, key.offset); + btrfs_abort_transaction(trans, ret); + btrfs_commit_transaction(trans, root); ret = -ENOENT; goto out; } ret = btrfs_del_item(trans, root, path); - if (ret) + if (ret) { + error("failed to delete root %llu item[%llu, %u, %llu]", + root->objectid, key.objectid, key.type, key.offset); + btrfs_abort_transaction(trans, ret); + btrfs_commit_transaction(trans, root); goto out; + } if (path->slots[0] == 0) btrfs_prev_leaf(root, path); else path->slots[0]--; -out: - btrfs_commit_transaction(trans, root); - if (ret) - error("failed to delete root %llu item[%llu, %u, %llu]", - root->objectid, key.objectid, key.type, key.offset); - else + ret = btrfs_commit_transaction(trans, root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); + } else { printf("Deleted root %llu item[%llu, %u, %llu]\n", root->objectid, key.objectid, key.type, key.offset); + } +out: return ret; } @@ -620,7 +638,16 @@ static int repair_block_accounting(void) } ret = btrfs_fix_block_accounting(trans); - btrfs_commit_transaction(trans, gfs_info->tree_root); + if (ret < 0) { + btrfs_abort_transaction(trans, ret); + btrfs_commit_transaction(trans, gfs_info->tree_root); + return ret; + } + ret = btrfs_commit_transaction(trans, gfs_info->tree_root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); + } return ret; } @@ -1031,6 +1058,12 @@ static int repair_ternary_lowmem(struct btrfs_root *root, u64 dir_ino, u64 ino, UASSERT(stage < 3); trans = btrfs_start_transaction(root, 1); + if (IS_ERR(trans)) { + ret = PTR_ERR(trans); + errno = -ret; + error_msg(ERROR_MSG_START_TRANS, "%m"); + return ret; + } if (stage == 2) { ret = btrfs_unlink(trans, root, ino, dir_ino, index, name, name_len, 0); @@ -1502,23 +1535,37 @@ static int repair_inode_item_missing(struct btrfs_root *root, u64 ino, trans = btrfs_start_transaction(root, 1); if (IS_ERR(trans)) { - ret = -EIO; - goto out; + ret = PTR_ERR(trans); + errno = -ret; + error_msg(ERROR_MSG_START_TRANS, "%m"); + return ret; } ret = btrfs_search_slot(trans, root, &key, &path, 1, 1); - if (ret < 0 || !ret) - goto fail; + if (ret < 0 || !ret) { + error("failed to repair root %llu INODE ITEM[%llu] missing", + root->objectid, ino); + btrfs_abort_transaction(trans, ret); + btrfs_commit_transaction(trans, root); + goto out; + } /* insert inode item */ - create_inode_item_lowmem(trans, root, ino, filetype); - ret = 0; -fail: - btrfs_commit_transaction(trans, root); + ret = create_inode_item_lowmem(trans, root, ino, filetype); + if (ret < 0) { + error("failed to insert inode item %llu in root %llu", + ino, root->objectid); + btrfs_abort_transaction(trans, ret); + btrfs_commit_transaction(trans, root); + goto out; + } + + ret = btrfs_commit_transaction(trans, root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); + } out: - if (ret) - error("failed to repair root %llu INODE ITEM[%llu] missing", - root->objectid, ino); btrfs_release_path(&path); return ret; } @@ -1545,12 +1592,13 @@ static int lowmem_delete_corrupted_dir_item(struct btrfs_root *root, ret = delete_corrupted_dir_item(trans, root, di_key, namebuf, name_len); if (ret < 0) { btrfs_abort_transaction(trans, ret); - } else { - ret = btrfs_commit_transaction(trans, root); - if (ret < 0) { - errno = -ret; - error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); - } + btrfs_commit_transaction(trans, root); + return ret; + } + ret = btrfs_commit_transaction(trans, root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); } return ret; } @@ -1861,18 +1909,28 @@ static int punch_extent_hole(struct btrfs_root *root, struct btrfs_path *path, btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]); trans = btrfs_start_transaction(root, 1); - if (IS_ERR(trans)) + if (IS_ERR(trans)) { + ret = PTR_ERR(trans); + errno = -ret; + error_msg(ERROR_MSG_START_TRANS, "%m"); return PTR_ERR(trans); + } ret = btrfs_punch_hole(trans, root, ino, start, len); if (ret) { error("failed to add hole [%llu, %llu] in inode [%llu]", start, len, ino); btrfs_abort_transaction(trans, ret); + btrfs_commit_transaction(trans, root); return ret; } printf("Add a hole [%llu, %llu] in inode [%llu]\n", start, len, ino); - btrfs_commit_transaction(trans, root); + ret = btrfs_commit_transaction(trans, root); + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); + return ret; + } btrfs_release_path(path); ret = btrfs_search_slot(NULL, root, &key, path, 0, 0); @@ -1894,6 +1952,8 @@ static int repair_inline_ram_bytes(struct btrfs_root *root, trans = btrfs_start_transaction(root, 1); if (IS_ERR(trans)) { ret = PTR_ERR(trans); + errno = -ret; + error_msg(ERROR_MSG_START_TRANS, "%m"); return ret; } btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]); @@ -1922,7 +1982,10 @@ static int repair_inline_ram_bytes(struct btrfs_root *root, btrfs_mark_buffer_dirty(path->nodes[0]); ret = btrfs_commit_transaction(trans, root); - if (!ret) { + if (ret < 0) { + errno = -ret; + error_msg(ERROR_MSG_COMMIT_TRANS, "%m"); + } else { printf( "Successfully repaired inline ram_bytes for root %llu ino %llu\n", root->objectid, key.objectid); @@ -2286,6 +2349,8 @@ static int repair_inode_nbytes_lowmem(struct btrfs_root *root, if (IS_ERR(trans)) { ret = PTR_ERR(trans); err |= ret; + errno = -ret; + error_msg(ERROR_MSG_START_TRANS, "%m"); goto out; } @@ -2347,6 +2412,8 @@ static int repair_dir_isize_lowmem(struct btrfs_root *root, if (IS_ERR(trans)) { ret = PTR_ERR(trans); err |= ret; + errno = -ret; + error_msg(ERROR_MSG_START_TRANS, "%m"); goto out; } @@ -2400,6 +2467,8 @@ static int repair_inode_orphan_item_lowmem(struct btrfs_root *root, if (IS_ERR(trans)) { ret = PTR_ERR(trans); err |= ret; + errno = -ret; + error_msg(ERROR_MSG_START_TRANS, "%m"); goto out; } @@ -2458,6 +2527,8 @@ static int repair_inode_nlinks_lowmem(struct btrfs_root *root, trans = btrfs_start_transaction(root, 1); if (IS_ERR(trans)) { ret = PTR_ERR(trans); + errno = -ret; + error_msg(ERROR_MSG_START_TRANS, "%m"); goto out; } @@ -5159,6 +5230,8 @@ static int repair_fs_first_inode(struct btrfs_root *root, int err) trans = btrfs_start_transaction(root, 1); if (IS_ERR(trans)) { ret = PTR_ERR(trans); + errno = -ret; + error_msg(ERROR_MSG_START_TRANS, "%m"); goto out; }