* [PATCH v2] btrfs-progs: test-misc: search the backup slot to use at runtime
@ 2021-10-13 1:12 Qu Wenruo
2021-10-14 15:36 ` Josef Bacik
0 siblings, 1 reply; 4+ messages in thread
From: Qu Wenruo @ 2021-10-13 1:12 UTC (permalink / raw)
To: linux-btrfs
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))
+
+# Save the backup slot info into the log
+_log "Backup slot $slot_num will be utilized"
+dump_super | run_check grep -A9 "backup $slot_num:"
+
+run_check "$INTERNAL_BIN/btrfs-corrupt-block" -m "$main_root_ptr" -f generation "$TEST_DEV"
# Should fail because the root is corrupted
run_mustfail "Unexpected successful mount" \
@@ -45,9 +55,10 @@ run_mustfail "Unexpected successful mount" \
run_check_mount_test_dev -o usebackuproot
run_check_umount_test_dev
-# Since we've used backup 1 as the usable root, then backup 2 should have been
-# overwritten
-main_root_ptr=$(dump_super | grep root | head -n1 | awk '{print $2}')
-backup2_new_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}')
+
+# The next slot should be overwritten
+slot_num=$(( ($slot_num + 1) % 4 ))
+backup_new_root_ptr=$(dump_super | grep -A1 "backup $slot_num" | grep backup_tree_root | awk '{print $2}')
-[ "$backup2_root_ptr" -ne "$backup2_new_root_ptr" ] || _fail "Backup 2 not overwritten"
+[ "$main_root_ptr" -ne "$backup_new_root_ptr" ] || _fail "Backup 2 not overwritten"
--
2.33.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] btrfs-progs: test-misc: search the backup slot to use at runtime
2021-10-13 1:12 [PATCH v2] btrfs-progs: test-misc: search the backup slot to use at runtime Qu Wenruo
@ 2021-10-14 15:36 ` Josef Bacik
2021-10-14 22:36 ` Qu Wenruo
0 siblings, 1 reply; 4+ messages in thread
From: Josef Bacik @ 2021-10-14 15:36 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] btrfs-progs: test-misc: search the backup slot to use at runtime
2021-10-14 15:36 ` Josef Bacik
@ 2021-10-14 22:36 ` Qu Wenruo
2021-10-14 22:56 ` Josef Bacik
0 siblings, 1 reply; 4+ messages in thread
From: Qu Wenruo @ 2021-10-14 22:36 UTC (permalink / raw)
To: Josef Bacik, Qu Wenruo; +Cc: linux-btrfs
On 2021/10/14 23:36, Josef Bacik wrote:
> 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,
Note that, the $found is from "grep -n" which starts its line number at 1.
Thus $slot_num will always be >= 1, and nothing will be wrong.
Thanks,
Qu
>
> Josef
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] btrfs-progs: test-misc: search the backup slot to use at runtime
2021-10-14 22:36 ` Qu Wenruo
@ 2021-10-14 22:56 ` Josef Bacik
0 siblings, 0 replies; 4+ messages in thread
From: Josef Bacik @ 2021-10-14 22:56 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Qu Wenruo, linux-btrfs
On Fri, Oct 15, 2021 at 06:36:36AM +0800, Qu Wenruo wrote:
>
>
> On 2021/10/14 23:36, Josef Bacik wrote:
> > 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,
>
> Note that, the $found is from "grep -n" which starts its line number at 1.
>
> Thus $slot_num will always be >= 1, and nothing will be wrong.
>
I missed the -n part, sorry about that, you can add
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Thanks,
Josef
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-14 22:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-13 1:12 [PATCH v2] btrfs-progs: test-misc: search the backup slot to use at runtime Qu Wenruo
2021-10-14 15:36 ` Josef Bacik
2021-10-14 22:36 ` Qu Wenruo
2021-10-14 22:56 ` Josef Bacik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).