Re: [PATCH v6 09/12] generic: Add sudden shutdown tests for multi block atomic writes

[John Garry <john.g.garry@oracle.com>]

On 17/09/2025 09:09, Ojaswin Mujoo wrote:
>> nit: I think that I mentioned this the last time - I would not use the word
>> "expected". We have old data, new data, and actual data. The only thing
>> which we expect is that actual data will be either all old or all new.
>   
> Hey John so I mentioned here [1] that the wording "expected new",
> "expected old", "actual" looked more clear to me than "new", "old" and
> "actual" and you replied with sure so I though we were good there 🙂
> 
> But no worries I can make this change. I'll keep the wording as
> new, old and actual.

great, thanks

> 
>>> +			echo "$expected_data_old"
>>> +			echo
>>> +			echo "Expected new: "
>>> +			echo "$expected_data_new"
>>> +			echo
>>> +			echo "Actual contents: "
>>> +			echo "$actual_data"
>>> +
>>> +			_fail
>>> +		fi
>>> +		echo -n "Check at offset $off succeeded! " >> $seqres.full
>>> +		if [[ "$actual_data" == "$expected_data_new" ]]
>>> +		then
>>> +			echo "matched new" >> $seqres.full
>>> +		elif [[ "$actual_data" == "$expected_data_old" ]]
>>> +		then
>>> +			echo "matched old" >> $seqres.full
>>> +		fi
>>> +		off=$(( off + awu_max ))
>>> +	done
>>> +}
>>> +
>>> +# test data integrity for file by shutting down in between atomic writes
>>> +test_data_integrity() {
>>> +	echo >> $seqres.full
>>> +	echo "# Writing atomically to file in background" >> $seqres.full
>>> +
>>> +	start_atomic_write_and_shutdown
>>> +
>>> +	last_offset=$(tail -n 1 $tmp.aw | cut -d" " -f4)
>>> +	if [[ -z $last_offset ]]
>>> +	then
>>> +		last_offset=0
>>> +	fi
>>> +
>>> +	echo >> $seqres.full
>>> +	echo "# Last offset of atomic write: $last_offset" >> $seqres.full
>>> +
>>> +	rm $tmp.aw
>>> +	sleep 0.5
>>> +
>>> +	_scratch_cycle_mount
>>> +
>>> +	# we want to verify all blocks around which the shutdown happened
>>> +	verify_start=$(( last_offset - (awu_max * 5)))
>>> +	if [[ $verify_start < 0 ]]
>>> +	then
>>> +		verify_start=0
>>> +	fi
>>> +
>>> +	verify_end=$(( last_offset + (awu_max * 5)))
>>> +	if [[ "$verify_end" -gt "$filesize" ]]
>>> +	then
>>> +		verify_end=$filesize
>>> +	fi
>>> +}
>>> +
>>> +# test data integrity for file with written and unwritten mappings
>>> +test_data_integrity_mixed() {
>>> +	$XFS_IO_PROG -fc "truncate 0" $testfile >> $seqres.full
>>> +
>>> +	echo >> $seqres.full
>>> +	echo "# Creating testfile with mixed mappings" >> $seqres.full
>>> +	create_mixed_mappings $testfile $filesize
>>> +
>>> +	test_data_integrity
>>> +
>>> +	verify_data_blocks $verify_start $verify_end "$expected_data_old_mixed" "$expected_data_new"
>>> +}
>>> +
>>> +# test data integrity for file with completely written mappings
>>> +test_data_integrity_written() {
>> nit: again, I am not so keen on using the word "integrity" at all.
>> "integrity" in storage world relates to T10 PI support in Linux. I know that
>> last time I mentioned it's ok to use "integrity" when close to words "atomic
>> write", but I still fear some doubt on whether we are talking about T10 PI
>> when we mention integrity.
> Okay got it, fine then how about using phrases like "test for torn
> data for file with completely written mapping" and such?

sure, it's ok

Thanks very much