Re: [PATCH v2] btrfs-progs: test-misc: search the backup slot to use at runtime

[Josef Bacik <josef@toxicpanda.com>]

On Wed, Oct 13, 2021 at 09:12:33AM +0800, Qu Wenruo wrote:
> Test case misc/038 uses hardcoded backup slot number, this means if we
> change how many transactions we commit during mkfs, it will immediately
> break the tests.
> 
> Such hardcoded tests will be a big pain for later btrfs-progs updates.
> 
> Update it with runtime backup slot search.
> 
> Such search is done by using current filesystem generation as a search
> target and grab the slot number.
> 
> By this, no matter how many transactions we commit during mkfs, the test
> case should be able to handle it.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> Changelog:
> v2:
> - Use run_check() instead of manually redirect output to "$RESULT"
> - Quote "$main_root_ptr"
> ---
>  .../038-backup-root-corruption/test.sh        | 47 ++++++++++++-------
>  1 file changed, 29 insertions(+), 18 deletions(-)
> 
> diff --git a/tests/misc-tests/038-backup-root-corruption/test.sh b/tests/misc-tests/038-backup-root-corruption/test.sh
> index b6c3671f2c3a..bf41f1e0952b 100755
> --- a/tests/misc-tests/038-backup-root-corruption/test.sh
> +++ b/tests/misc-tests/038-backup-root-corruption/test.sh
> @@ -17,25 +17,35 @@ run_check $SUDO_HELPER touch "$TEST_MNT/file"
>  run_check_umount_test_dev
>  
>  dump_super() {
> -	run_check_stdout $SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super -f "$TEST_DEV"
> +	# In this test, we will dump super block multiple times, while the
> +	# existing run_check*() helpers will always dump all the output into
> +	# the log, flooding the log and hide real important info.
> +	# Thus here we call "btrfs" directly.
> +	$SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super -f "$TEST_DEV"
>  }
>  
> -# Ensure currently active backup slot is the expected one (slot 3)
> -backup2_root_ptr=$(dump_super | grep -A1 "backup 2" | grep backup_tree_root | awk '{print $2}')
> -
>  main_root_ptr=$(dump_super | awk '/^root\t/{print $2}')
> +# Grab current fs generation, and it will be used to determine which backup
> +# slot to use
> +cur_gen=$(dump_super | grep ^generation | awk '{print $2}')
> +backup_gen=$(($cur_gen - 1))
> +
> +# Grab the slot which matches @backup_gen
> +found=$(dump_super | grep backup_tree_root | grep -n "gen: $backup_gen")
>  
> -if [ "$backup2_root_ptr" -ne "$main_root_ptr" ]; then
> -	_log "Backup slot 2 not in use, trying slot 3"
> -	# Or use the next slot in case of free-space-tree
> -	backup3_root_ptr=$(dump_super | grep -A1 "backup 3" | grep backup_tree_root | awk '{print $2}')
> -	if [ "$backup3_root_ptr" -ne "$main_root_ptr" ]; then
> -		_fail "Neither backup slot 2 nor slot 3 are in use"
> -	fi
> -	_log "Backup slot 3 in use"
> +if [ -z "$found" ]; then
> +	_fail "Unable to find a backup slot with generation $backup_gen"
>  fi
>  
> -run_check "$INTERNAL_BIN/btrfs-corrupt-block" -m $main_root_ptr -f generation "$TEST_DEV"
> +slot_num=$(echo $found | cut -f1 -d:)
> +# To follow the dump-super output, where backup slot starts at 0.
> +slot_num=$(($slot_num - 1))

What happens if we're on $slot_num == 0?  Seems like this would mess up, right?
Thanks,

Josef