Re: [PATCH] btrfs: abort the transaction if we fail to replay log trees
From: Johannes Thumshirn <hidden>
Date: 2021-05-19 16:22:28
On 19/05/2021 17:29, Josef Bacik wrote:
quoted hunk ↗ jump to hunk
During inspection of the return path for replay I noticed that we don't actually abort the transaction if we get a failure during replay. This isn't a problem necessarily, as we properly return the error and will fail to mount. However we still leave this dangling transaction that could conceivably be committed without thinking there was an error. Handle this by making sure we abort the transaction on error to safeguard us from any problems in the future. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/tree-log.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c index 4dc74949040d..18009095908b 100644 --- a/fs/btrfs/tree-log.c +++ b/fs/btrfs/tree-log.c@@ -6352,8 +6352,10 @@ int btrfs_recover_log_trees(struct btrfs_root *log_root_tree) return 0; error: - if (wc.trans) + if (wc.trans) { + btrfs_abort_transaction(wc.trans, ret); btrfs_end_transaction(wc.trans); + } btrfs_free_path(path); return ret; }
Hmm our Wiki's Development notes page says: Please keep all transaction abort exactly at the place where they happen and do not merge them to one. This pattern should be used everwhere and is important when debugging because we can pinpoint the line in the code from the syslog message and do not have to guess which way it got to the merged call. But there are 6 'goto error;' lines in btrfs_recover_log_trees() and changing all of them might be a bit too much. Anyways, Reviewed-by: Johannes Thumshirn <redacted>