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

[Dave Chinner <david@fromorbit.com>]

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....

> 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...)

> 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.
-- 
Dave Chinner
david@fromorbit.com