Re: [PATCH 1/5] xfstest: add ubifs support

[Dongsheng Yang <yangds.fnst@cn.fujitsu.com>]

On 08/03/2015 12:12 PM, Dave Chinner wrote:
> On Thu, Jul 30, 2015 at 02:06:53PM +0800, Dongsheng Yang wrote:
>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>> ---
>>   check         | 2 ++
>>   common/config | 9 +++++----
>>   common/rc     | 8 ++++++--
>>   3 files changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/check b/check
>> index a300130..20d45f8 100755
>> --- a/check
>> +++ b/check
>> @@ -69,6 +69,7 @@ check options
>>       -nfs                test NFS
>>       -cifs               test CIFS
>>       -tmpfs              test TMPFS
>> +    -ubifs		test ubifs
>>       -l			line mode diff
>>       -udiff		show unified diff (default)
>>       -n			show me, do not run tests
>> @@ -207,6 +208,7 @@ while [ $# -gt 0 ]; do
>>   	-nfs)	FSTYP=nfs ;;
>>   	-cifs)	FSTYP=cifs ;;
>>   	-tmpfs)	FSTYP=tmpfs ;;
>> +	-ubifs)	FSTYP=ubifs ;;
>
> It's a local device - we should be able to probe it to find FSTYP
> like we do with all other local devices. Probing is much preferable
> to adding more FS specific CLI options....

That's true, it's not necessary, I agree probing is better.
I will remove this line in next version.
>
>> diff --git a/common/config b/common/config
>> index 8e21c28..2c809b6 100644
>> --- a/common/config
>> +++ b/common/config
>> @@ -220,6 +220,7 @@ case "$HOSTOS" in
>>           export MKFS_EXT4_PROG="`set_prog_path mkfs.ext4`"
>>           export MKFS_UDF_PROG="`set_prog_path mkudffs`"
>>           export MKFS_BTRFS_PROG="`set_btrfs_mkfs_prog_path_with_opts`"
>> +        export MKFS_UBIFS_PROG="`set_prog_path mkfs.ubifs`"
>>           export MKFS_F2FS_PROG="`set_prog_path mkfs.f2fs`"
>>           export DUMP_F2FS_PROG="`set_prog_path dump.f2fs`"
>>           export BTRFS_UTIL_PROG="`set_prog_path btrfs`"
>> @@ -465,9 +466,9 @@ get_next_config() {
>>   		exit 1
>>   	fi
>>
>> -	echo $TEST_DEV | grep -qE ":|//" > /dev/null 2>&1
>> +	echo $TEST_DEV | grep -qE ":|//|ubi" > /dev/null 2>&1
>>   	if [ ! -b "$TEST_DEV" -a "$?" != "0" ]; then
>> -		echo "common/config: Error: \$TEST_DEV ($TEST_DEV) is not a block device or a network filesystem"
>> +		echo "common/config: Error: \$TEST_DEV ($TEST_DEV) is not a block device or a network filesystem or ubi device"
>>   		exit 1
>>   	fi
>>
>> @@ -488,9 +489,9 @@ get_next_config() {
>>   		export SCRATCH_DEV_NOT_SET=true
>>   	fi
>>
>> -	echo $SCRATCH_DEV | grep -qE ":|//" > /dev/null 2>&1
>> +	echo $SCRATCH_DEV | grep -qE ":|//|ubi" > /dev/null 2>&1
>>   	if [ ! -z "$SCRATCH_DEV" -a ! -b "$SCRATCH_DEV" -a "$?" != "0" ]; then
>> -		echo "common/config: Error: \$SCRATCH_DEV ($SCRATCH_DEV) is not a block device or a network filesystem"
>> +		echo "common/config: Error: \$SCRATCH_DEV ($SCRATCH_DEV) is not a block device or a network filesystem or ubi device"
>>   		exit 1
>>   	fi
>
> Hmmm - how do we validate that we were passed a valid ubi device in
> TEST_DEV/SCRATCH_DEV? (goes back to the probing question above...)

I am using the following config for my testing. So if we want to
test ubifs, we have to pass a ubi volume to it. the name of it
looks as ubiX_X.

# cat local.config
# Ideally define at least these 4 to match your environment
# The first 2 are required.
# See README for other variables which can be set.
#
# Note: SCRATCH_DEV >will< get overwritten!

export TEST_DEV=/dev/ubi0_0
export TEST_DIR=/mnt/test
export SCRATCH_DEV=/dev/ubi0_1
export SCRATCH_MNT=/mnt/scratch

>
>> diff --git a/common/rc b/common/rc
>> index 610045e..b206827 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -1106,7 +1106,7 @@ _require_scratch_nocheck()
>>   		     _notrun "this test requires a valid \$SCRATCH_MNT"
>>   		fi
>>   		;;
>> -	tmpfs)
>> +	tmpfs|ubifs)
>>   		if [ -z "$SCRATCH_DEV" -o ! -d "$SCRATCH_MNT" ];
>>   		then
>>   		    _notrun "this test requires a valid \$SCRATCH_MNT and unique $SCRATCH_DEV"
>> @@ -1180,7 +1180,7 @@ _require_test()
>>   		     _notrun "this test requires a valid \$TEST_DIR"
>>   		fi
>>   		;;
>> -	tmpfs)
>> +	tmpfs|ubifs)
>>   		if [ -z "$TEST_DEV" -o ! -d "$TEST_DIR" ];
>>   		then
>>   		    _notrun "this test requires a valid \$TEST_DIR and unique $TEST_DEV"
>> @@ -2054,6 +2054,8 @@ _check_test_fs()
>>       tmpfs)
>>   	# no way to check consistency for tmpfs
>>   	;;
>> +    ubifs)
>> +	;;
>>       *)
>>   	_check_generic_filesystem $TEST_DEV
>>   	;;
>> @@ -2092,6 +2094,8 @@ _check_scratch_fs()
>>       tmpfs)
>>   	# no way to check consistency for tmpfs
>>   	;;
>> +    ubifs)
>> +	;;
>
> There is no fsck.ubifs? How do you validate the filesystem is
> consistent?
>
> Keep in minds that the people you are asking to review/commit this
> code have no idea what UBIFS is or how it works, so you need to
> explain why xfstests requires non-standard configuration and
> bypassing of the config sanity testing...
>
> Cheers,
>
> Dave.
>