Re: [PATCH 2/2] fstests: add configuration option for executing post mkfs commands

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2023/9/28 15:04, Anand Jain wrote:
>
>
> On 28/09/2023 12:26, Qu Wenruo wrote:
>>
>>
>> On 2023/9/28 13:53, Anand Jain wrote:
>>> This patch introduces new configuration file parameters,
>>> POST_SCRATCH_MKFS_CMD and POST_SCRATCH_POOL_MKFS_CMD.
>>>
>>> Usage example:
>>>
>>>          POST_SCRATCH_MKFS_CMD="btrfstune -m"
>>>          POST_SCRATCH_POOL_MKFS_CMD="btrfstune -m"
>>
>> Can't we add extra options for mkfs.btrfs to support metadata uuid at
>> mkfs time?
>>
>> We already support quota and all other features, I think it would be
>> much easier to implement metadata_uuid inside mkfs.
>>
>> If this feature is only for metadata_uuid, then I really prefer to do it
>> inside mkfs.btrfs.
>
> Thanks for the comments.
>
> The use of btrfstune -m is just an example; any other command,
> function, or script can be assigned to the variable POST_SCRATCH_xx.

The last time I tried something like this, I got strong objection from
some guy in the XFS community.

Just good luck if you can have a better chance.

>
> Now, regarding updating mkfs.btrfs with the btrfstune -m feature,
> why not? It simplifies testing. However, can we identify a use case
> other than testing?

For consistency, I believe all (at the ones we can enable) features
should have a mkfs equivalent at least.

(And can get around the the test limitations for sure)

Thanks,
Qu
>
> Thanks, Anand
>
>>
>> Thanks,
>> Qu
>>>
>>> With this configuration option, test cases using _scratch_mkfs(),
>>> scratch_pool_mkfs(), or _scratch_mkfs_sized() will run the above
>>> set value after the mkfs operation.
>>>
>>> Other mkfs functions, such as _mkfs_dev(), are not connected to the
>>> POST_SCRATCH_MKFS_CMD.
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> ---
>>>   common/btrfs | 35 +++++++++++++++++++++++++++++++++++
>>>   1 file changed, 35 insertions(+)
>>>
>>> diff --git a/common/btrfs b/common/btrfs
>>> index 798c899f6233..b0972e882c7c 100644
>>> --- a/common/btrfs
>>> +++ b/common/btrfs
>>> @@ -690,17 +690,48 @@ _require_btrfs_scratch_logical_resolve_v2()
>>>       _scratch_unmount
>>>   }
>>>
>>> +post_scratch_mkfs_cmd()
>>> +{
>>> +    if [[ -v POST_SCRATCH_MKFS_CMD ]]; then
>>> +        echo "$POST_SCRATCH_MKFS_CMD $SCRATCH_DEV"
>>> +        $POST_SCRATCH_MKFS_CMD $SCRATCH_DEV
>>> +        return $?
>>> +    fi
>>> +
>>> +    return 0
>>> +}
>>> +
>>> +post_scratch_pool_mkfs_cmd()
>>> +{
>>> +    if [[ -v POST_SCRATCH_POOL_MKFS_CMD ]]; then
>>> +        echo "$POST_SCRATCH_POOL_MKFS_CMD $SCRATCH_DEV_POOL"
>>> +        $POST_SCRATCH_POOL_MKFS_CMD $SCRATCH_DEV_POOL
>>> +        return $?
>>> +    fi
>>> +
>>> +    return 0
>>> +}
>>> +
>>>   _scratch_mkfs_retry_btrfs()
>>>   {
>>>       # _scratch_do_mkfs() may retry mkfs without $MKFS_OPTIONS
>>>       _scratch_do_mkfs "$MKFS_BTRFS_PROG" "cat" $*
>>>
>>> +    if [[ $? == 0 ]]; then
>>> +        post_scratch_mkfs_cmd
>>> +    fi
>>> +
>>>       return $?
>>>   }
>>>
>>>   _scratch_mkfs_btrfs()
>>>   {
>>>       $MKFS_BTRFS_PROG $MKFS_OPTIONS $mixed_opt -b $fssize $SCRATCH_DEV
>>> +
>>> +    if [[ $? == 0 ]]; then
>>> +        post_scratch_mkfs_cmd
>>> +    fi
>>> +
>>>       return $?
>>>   }
>>>
>>> @@ -708,5 +739,9 @@ _scratch_pool_mkfs_btrfs()
>>>   {
>>>       $MKFS_BTRFS_PROG $MKFS_OPTIONS $* $SCRATCH_DEV_POOL
>>>
>>> +    if [[ $? == 0 ]]; then
>>> +        post_scratch_pool_mkfs_cmd
>>> +    fi
>>> +
>>>       return $?
>>>   }