linux-ext4 - Re: [PATCH RESEND 5/10] xfstest: shared/001: Standard collapse range tests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAKYAXd_ags=K+OAcJoRHdTFLuU7vUbR+afdmpR2sj3jgTeQnPA@mail.gmail.com>
Date:	Tue, 4 Feb 2014 12:49:33 +0900
From:	Namjae Jeon <linkinjeon@...il.com>
To:	Dave Chinner <david@...morbit.com>
Cc:	viro@...iv.linux.org.uk, bpm@....com, tytso@....edu,
	adilger.kernel@...ger.ca, jack@...e.cz, mtk.manpages@...il.com,
	linux-fsdevel@...r.kernel.org, xfs@....sgi.com,
	linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org,
	Namjae Jeon <namjae.jeon@...sung.com>,
	Ashish Sangwan <a.sangwan@...sung.com>
Subject: Re: [PATCH RESEND 5/10] xfstest: shared/001: Standard collapse range tests

2014-02-04, Dave Chinner <david@...morbit.com>:
> On Sun, Feb 02, 2014 at 02:45:58PM +0900, Namjae Jeon wrote:
>> From: Namjae Jeon <namjae.jeon@...sung.com>
>>
>> This testcase(001) tries to test various corner cases
>> for fcollapse range functionality over different type of extents.
>>
>> Signed-off-by: Namjae Jeon <namjae.jeon@...sung.com>
>> Signed-off-by: Ashish Sangwan <a.sangwan@...sung.com>
>
> Couple of things:
>
>>  			-c "$map_cmd -v" $testfile | $filter_cmd
>>  		[ $? -ne 0 ] && die_now
>>  		_md5_checksum $testfile
>> @@ -415,10 +425,10 @@ _test_generic_punch()
>>  		if [ "$remove_testfile" ]; then
>>  			rm -f $testfile
>>  		fi
>> -		$XFS_IO_PROG -f -c "truncate 20k" \
>> -			-c "$alloc_cmd 0 8k" \
>> -			-c "pwrite 8k 8k" $sync_cmd \
>> -			-c "$zero_cmd 4k 8k" \
>> +		$XFS_IO_PROG -f -c "truncate $(($multiple * 20))k" \
>> +			-c "$alloc_cmd 0 $(($multiple * 8))k" \
>> +			-c "pwrite $(($multiple * 8))k $(($multiple * 8))k" $sync_cmd \
>> +			-c "$zero_cmd $(($multiple * 4))k $(($multiple * 8))k" \
>>  			-c "$map_cmd -v" $testfile | $filter_cmd
>
Hi. Dave.
> This is unreadable, and therefore I'd consider that these changes
> render _test_generic_punch unmaintainable.
>
> Either it needs tobe factored to be more readable, or we need a more
> readable way of representing the offsets and sizes if we want them
> to be variable. For example:
>
> _4k="$((multiple * 4))k"
> _8k="$((multiple * 8))k"
> _20k="$((multiple * 20))k"
>
> leads to:
>
> 		$XFS_IO_PROG -f -c "truncate $_20k" \
> 			-c "$alloc_cmd 0 $_8k" \
> 			-c "pwrite $_8k $_8k" $sync_cmd \
> 			-c "$zero_cmd $_4k $_8k" \
> 			-c "$map_cmd -v" $testfile | $filter_cmd
>
> which is still readable and allows us to arbitrarily scale the
> parameters. It even allows us to handle different filesystem block
> sizes if we really want to....
Okay, I will change it as you suggest.
>
>>  		-c "$map_cmd -v" $testfile | $filter_cmd
>>  	[ $? -ne 0 ] && die_now
>>  	_md5_checksum $testfile
>>
>> +	# If zero_cmd is fcollpase, don't check unaligned offsets
>> +	if [ "$zero_cmd" == "fcollapse" ]; then
>> +		if [ "$remove_testfile" ]; then
>> +			rm -f $testfile
>> +			rm -f $testfile.2
>> +		fi
>> +		return
>> +	fi
>
> No need to remove the test files here - we remove them at
> test startup to ensure we have a known initial state....
Okay.
>
>> +0: [0..63]: extent
>> +bb7df04e1b0a2570657527a7e108ae23
>> +	13. data -> unwritten -> data
>> +0: [0..63]: extent
>> +0f0151cbed83e4bf6e5bde26e82ab115
>> +	14. data -> hole @ EOF
>> +fallocate: Invalid argument
>> +0: [0..159]: extent
>
> This error appears in all the golden outputs. If it's correct, then
> perhaps it should be filtered out or commented somewhere to explain
> why it is expected.
Okay, I will add the comments to explain about this.

Thanks for your review :)
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@...morbit.com
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html