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

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

On 09/09/2025 07:44, Ojaswin Mujoo wrote:
>>>>> +create_mixed_mappings() {
>>>> Is this same as patch 08/12?
>>> I believe you mean the [D]SYNC tests, yes it is the same.
>> then maybe factor out the test, if possible. I assume that this sort of
>> approach is taken for xfstests.
>>
> I'm not sure what you mean by factor out the*test*. We are testing
> different things there and the only thing common in the tests is
> creation of mixed mapping files and the check to ensure we didn't tear
> data.
> 
> In case you mean to factor out the create_mixed_mappings() helper into
> common/rc, sure I can do that but I'm unsure if at this point it would
> be very useful for other tests.

above it was mentioned that the code in create_mixed_mappings was 
common, so that is why I suggested to factor it out. If it does not make 
sense, then fine (and don't).

> 
>>>>> +	local file=$1
>>>>> +	local size_bytes=$2
>>>>> +
>>>>> +	echo "# Filling file $file with alternate mappings till size $size_bytes" >> $seqres.full
>>>>> +	#Fill the file with alternate written and unwritten blocks
>>>>> +	local off=0
>>>>> +	local operations=("W" "U")
>>>>> +
>>>>> +	for ((i=0; i<$((size_bytes / blksz )); i++)); do
>>>>> +		index=$(($i % ${#operations[@]}))
>>>>> +		map="${operations[$index]}"
>>>>> +
> <...>


>>>>> +	echo >> $seqres.full
>>>>> +	echo "# Starting filesize integrity test for atomic writes" >> $seqres.full
>>>> what does "Starting filesize integrity test" mean?
>>> Basically other tests already truncate the file to a higher value and
>>> then perform the shut down test. Here we actually do append atomic
>>> writes since we want to also stress the i_size update paths during
>>> shutdown to ensure that doesn't cause any tearing with atomic writes.
>>>
>>> I can maybe rename it to:
>>>
>>>
>>> echo "# Starting data integrity test for atomic append writes" >> $seqres.full
>>>
>>> Thanks for the review!
>>>
>> It's just the name "integrity" that throws me a bit..
> So I mean integrity as in writes are not tearing after the shutdown.
> That's how we have worded the other sub-tests above.

you could mention "shutdown" also in the print.

Thanks!