diff mbox series

btrfs-progs: Check for metadata uuid feature in misc-tests/034-metadata-uuid

Message ID 20190805114522.12151-1-nborisov@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: Check for metadata uuid feature in misc-tests/034-metadata-uuid | expand

Commit Message

Nikolay Borisov Aug. 5, 2019, 11:45 a.m. UTC
Instead of checking the kernel version, explicitly check for the
presence of metadata_uuid file in sysfs. This allows the test to be run
on older kernels that might have this feature backported.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 tests/misc-tests/034-metadata-uuid/test.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Qu Wenruo Aug. 5, 2019, 12:16 p.m. UTC | #1
On 2019/8/5 下午7:45, Nikolay Borisov wrote:
> Instead of checking the kernel version, explicitly check for the
> presence of metadata_uuid file in sysfs. This allows the test to be run
> on older kernels that might have this feature backported.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

The idea is pretty good, as sysfs is way more accurate.

But /sys/fs/btrfs/features is not ensured to exist, e.g btrfs module not
loaded yet.

Can we fallback to regular kernel version check if
/sys/fs/btrfs/features not exist?

Thanks,
Qu

> ---
>  tests/misc-tests/034-metadata-uuid/test.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/misc-tests/034-metadata-uuid/test.sh b/tests/misc-tests/034-metadata-uuid/test.sh
> index 3ef110cda823..6ac55b1cacfa 100755
> --- a/tests/misc-tests/034-metadata-uuid/test.sh
> +++ b/tests/misc-tests/034-metadata-uuid/test.sh
> @@ -10,8 +10,8 @@ check_prereq btrfs-image
>  setup_root_helper
>  prepare_test_dev
>
> -if ! check_min_kernel_version 5.0; then
> -	_not_run "kernel too old, METADATA_UUID support needed"
> +if [ ! -f /sys/fs/btrfs/features/metadata_uuid ] ; then
> +	_not_run "METADATA_UUID feature not supported"
>  fi
>
>  read_fsid() {
>
Nikolay Borisov Aug. 5, 2019, 12:20 p.m. UTC | #2
On 5.08.19 г. 15:16 ч., Qu Wenruo wrote:
> 
> 
> On 2019/8/5 下午7:45, Nikolay Borisov wrote:
>> Instead of checking the kernel version, explicitly check for the
>> presence of metadata_uuid file in sysfs. This allows the test to be run
>> on older kernels that might have this feature backported.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> 
> The idea is pretty good, as sysfs is way more accurate.
> 
> But /sys/fs/btrfs/features is not ensured to exist, e.g btrfs module not
> loaded yet.
> 
> Can we fallback to regular kernel version check if
> /sys/fs/btrfs/features not exist?

The top-level test runned (misc-tests.sh in this case) already calls
check_kernel_support which ensures the module is loaded. So such
fallback is unnecessary.

> 
> Thanks,
> Qu
> 
>> ---
>>  tests/misc-tests/034-metadata-uuid/test.sh | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/misc-tests/034-metadata-uuid/test.sh b/tests/misc-tests/034-metadata-uuid/test.sh
>> index 3ef110cda823..6ac55b1cacfa 100755
>> --- a/tests/misc-tests/034-metadata-uuid/test.sh
>> +++ b/tests/misc-tests/034-metadata-uuid/test.sh
>> @@ -10,8 +10,8 @@ check_prereq btrfs-image
>>  setup_root_helper
>>  prepare_test_dev
>>
>> -if ! check_min_kernel_version 5.0; then
>> -	_not_run "kernel too old, METADATA_UUID support needed"
>> +if [ ! -f /sys/fs/btrfs/features/metadata_uuid ] ; then
>> +	_not_run "METADATA_UUID feature not supported"
>>  fi
>>
>>  read_fsid() {
>>
>
Qu Wenruo Aug. 5, 2019, 12:26 p.m. UTC | #3
On 2019/8/5 下午8:20, Nikolay Borisov wrote:
>
>
> On 5.08.19 г. 15:16 ч., Qu Wenruo wrote:
>>
>>
>> On 2019/8/5 下午7:45, Nikolay Borisov wrote:
>>> Instead of checking the kernel version, explicitly check for the
>>> presence of metadata_uuid file in sysfs. This allows the test to be run
>>> on older kernels that might have this feature backported.
>>>
>>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>>
>> The idea is pretty good, as sysfs is way more accurate.
>>
>> But /sys/fs/btrfs/features is not ensured to exist, e.g btrfs module not
>> loaded yet.
>>
>> Can we fallback to regular kernel version check if
>> /sys/fs/btrfs/features not exist?
>
> The top-level test runned (misc-tests.sh in this case) already calls
> check_kernel_support which ensures the module is loaded. So such
> fallback is unnecessary.

Oh, forgot that.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

>
>>
>> Thanks,
>> Qu
>>
>>> ---
>>>  tests/misc-tests/034-metadata-uuid/test.sh | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/misc-tests/034-metadata-uuid/test.sh b/tests/misc-tests/034-metadata-uuid/test.sh
>>> index 3ef110cda823..6ac55b1cacfa 100755
>>> --- a/tests/misc-tests/034-metadata-uuid/test.sh
>>> +++ b/tests/misc-tests/034-metadata-uuid/test.sh
>>> @@ -10,8 +10,8 @@ check_prereq btrfs-image
>>>  setup_root_helper
>>>  prepare_test_dev
>>>
>>> -if ! check_min_kernel_version 5.0; then
>>> -	_not_run "kernel too old, METADATA_UUID support needed"
>>> +if [ ! -f /sys/fs/btrfs/features/metadata_uuid ] ; then
>>> +	_not_run "METADATA_UUID feature not supported"
>>>  fi
>>>
>>>  read_fsid() {
>>>
>>
David Sterba Aug. 5, 2019, 6:55 p.m. UTC | #4
On Mon, Aug 05, 2019 at 02:45:22PM +0300, Nikolay Borisov wrote:
> Instead of checking the kernel version, explicitly check for the
> presence of metadata_uuid file in sysfs. This allows the test to be run
> on older kernels that might have this feature backported.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Applied, thanks.
diff mbox series

Patch

diff --git a/tests/misc-tests/034-metadata-uuid/test.sh b/tests/misc-tests/034-metadata-uuid/test.sh
index 3ef110cda823..6ac55b1cacfa 100755
--- a/tests/misc-tests/034-metadata-uuid/test.sh
+++ b/tests/misc-tests/034-metadata-uuid/test.sh
@@ -10,8 +10,8 @@  check_prereq btrfs-image
 setup_root_helper
 prepare_test_dev
 
-if ! check_min_kernel_version 5.0; then
-	_not_run "kernel too old, METADATA_UUID support needed"
+if [ ! -f /sys/fs/btrfs/features/metadata_uuid ] ; then
+	_not_run "METADATA_UUID feature not supported"
 fi
 
 read_fsid() {