Re: [PATCH 2/2] btrfs/163: replace sprout instead of seed

[Anand Jain <anand.jain@oracle.com>]

On 15/10/20 11:49 pm, Filipe Manana wrote:
> On Sat, Sep 5, 2020 at 12:25 AM Anand Jain <anand.jain@oracle.com> wrote:
>>
>> Make this test case inline with the kernel patch [1] changes
>> [1] btrfs: fix replace of seed device
> 
> Same comment as in the previous patch. Now that this is in Linus'
> tree, it would be good to mention the commit id too.

  Added in v3.

>>
>> So use the sprout device as the replace target instead of the seed device..
>> This change is compatible with the older kernels.
>>
>> While at this, this patch also fixes a typo fix as well.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   tests/btrfs/163     | 21 ++++++++++++++++-----
>>   tests/btrfs/163.out |  5 ++++-
>>   2 files changed, 20 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/btrfs/163 b/tests/btrfs/163
>> index 24c725afb6b9..354d88502d47 100755
>> --- a/tests/btrfs/163
>> +++ b/tests/btrfs/163
>> @@ -4,11 +4,15 @@
>>   #
>>   # FS QA Test 163
>>   #
>> -# Test case to verify that a seed device can be replaced
>> +# Test case to verify that a sprouted device can be replaced
>>   #  Create a seed device
>>   #  Create a sprout device
>>   #  Remount RW
>> -#  Run device replace on the seed device
>> +#  Run device replace on the sprout device
>> +#
>> +# Depends on the kernel patch
>> +#   btrfs: fail replace of seed device
>> +
>>   seq=`basename $0`
>>   seqres=$RESULT_DIR/$seq
>>   echo "QA output created by $seq"
>> @@ -39,6 +43,7 @@ _supported_fs btrfs
>>   _supported_os Linux
>>   _require_command "$BTRFS_TUNE_PROG" btrfstune
>>   _require_scratch_dev_pool 3
>> +_require_btrfs_forget_or_module_loadable
>>
>>   _scratch_dev_pool_get 3
>>
>> @@ -52,7 +57,7 @@ create_seed()
>>          run_check _mount $dev_seed $SCRATCH_MNT
>>          $XFS_IO_PROG -f -d -c "pwrite -S 0xab 0 4M" $SCRATCH_MNT/foobar >\
>>                  /dev/null
>> -       echo -- gloden --
>> +       echo -- golden --
>>          od -x $SCRATCH_MNT/foobar
>>          _run_btrfs_util_prog filesystem show -m $SCRATCH_MNT
>>          _scratch_unmount
>> @@ -64,22 +69,28 @@ add_sprout()
>>   {
>>          _run_btrfs_util_prog device add -f $dev_sprout $SCRATCH_MNT
>>          _run_btrfs_util_prog filesystem show -m $SCRATCH_MNT
>> +       _mount -o remount,rw $dev_sprout $SCRATCH_MNT
>> +       $XFS_IO_PROG -f -d -c "pwrite -S 0xcd 0 4M" $SCRATCH_MNT/foobar2 >\
>> +               /dev/null
> 
> Same comment as for the other patch.
> Why the direct IO? The test passes with buffered IO as well.
> If there's a reason for direct IO, it should be mentioned in a comment
> and _require_odirect added above.
> 

  Yes. No need of the directio here. Thanks.

>>   }
>>
>>   replace_seed()
>>   {
>> -       _run_btrfs_util_prog replace start -fB $dev_seed $dev_replace_tgt $SCRATCH_MNT
>> +       _run_btrfs_util_prog replace start -fB $dev_sprout $dev_replace_tgt $SCRATCH_MNT
> 
> So now the function should be renamed from replace_seed() to
> replace_sprout() as well. Shouldn't it?
> 

  Good catch will fix the function name.

> Other than that, it looks good and it works as expected.
> 

  All comments are fixed in v3.


Thanks, Anand

> Thanks.
> 
>>          _run_btrfs_util_prog filesystem show -m $SCRATCH_MNT
>>          _scratch_unmount
>> -       run_check _mount $dev_replace_tgt $SCRATCH_MNT
>> +       _btrfs_forget_or_module_reload
>> +       run_check _mount -o device=$dev_seed $dev_replace_tgt $SCRATCH_MNT
>>          echo -- sprout --
>>          od -x $SCRATCH_MNT/foobar
>> +       od -x $SCRATCH_MNT/foobar2
>>          _scratch_unmount
>>
>>   }
>>
>>   seed_is_mountable()
>>   {
>> +       _btrfs_forget_or_module_reload
>>          run_check _mount $dev_seed $SCRATCH_MNT
>>          _run_btrfs_util_prog filesystem show -m $SCRATCH_MNT
>>          _scratch_unmount
>> diff --git a/tests/btrfs/163.out b/tests/btrfs/163.out
>> index 91f6f5b6f48a..351ef7b040b2 100644
>> --- a/tests/btrfs/163.out
>> +++ b/tests/btrfs/163.out
>> @@ -1,5 +1,5 @@
>>   QA output created by 163
>> --- gloden --
>> +-- golden --
>>   0000000 abab abab abab abab abab abab abab abab
>>   *
>>   20000000
>> @@ -7,3 +7,6 @@ QA output created by 163
>>   0000000 abab abab abab abab abab abab abab abab
>>   *
>>   20000000
>> +0000000 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
>> +*
>> +20000000
>> --
>> 2.25.1
>>
> 
>