Re: [PATCH v2 2/2] btrfs/291: fix state transition logic and add size requirement

[Disha Goel <disgoel@linux.ibm.com>]

On 05/05/26 8:16 am, Anand Jain wrote:
> On 27/4/26 20:25, Disha Goel wrote:
>> This patch fixes two issues in btrfs/291:
>>
>> 1. Add minimum 9GB LOGWRITES_DEV size requirement
>>     The test creates LVM snapshots at each FUA point during replay,
>>     requiring significant space. Use _require_log_writes_sized to
>>     ensure sufficient space is available before running the test.
>>
> 
> It also depends on the SCRATCH_DEV size; the fix fails if SCRATCH_DEV is
>   10G.
> 
> [  173.478493] BTRFS error (device dm-3): device total_bytes should be
> at most 9663676416 but found 10737418240
> [  173.482763] BTRFS error (device dm-3): failed to read chunk tree: -22
> [  173.485457] BTRFS error (device dm-3): open_ctree failed: -22
> 
> Thanks.
> 

Thanks for the detailed review! I've addressed all the feedback in v3.

I've tested the fix with multiple SCRATCH_DEV sizes and confirmed it
works correctly.

> 
>> 2. Fix state transition logic for verity enablement
>>     The original test assumed orphan items would always be created
>>     during verity enablement (state 0->1 transition). However, in
>>     some cases verity completes without creating orphan items,
>>     causing the test to fail with "expected to reach verity done state".
>>
>>     Fix by transitioning to state 1 when either orphan items exist
>>     OR merkle items appear, handling both verity enablement paths.
>>     Also improve state 1 validation to only check for cleared merkle
>>     items when measurement actually fails.
>>
>> The test now correctly handles verity enablement with or without
>> orphan items while maintaining crash consistency validation.
>>
>> v2:
>> - Use _require_log_writes_sized instead of separate _require_log_writes
>>    and _require_logwrites_size calls
>>
>> Signed-off-by: Disha Goel <disgoel@linux.ibm.com>
>> ---
>>   tests/btrfs/291 | 17 ++++++++++++-----
>>   1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/tests/btrfs/291 b/tests/btrfs/291
>> index 122aeaa5..ac1d192d 100755
>> --- a/tests/btrfs/291
>> +++ b/tests/btrfs/291
>> @@ -36,7 +36,7 @@ _cleanup()
>>   _require_scratch
>>   _require_test
>>   _require_loop
>> -_require_log_writes
>> +_require_log_writes_sized $((9 * 1024 * 1024))
>>   _require_dm_target snapshot
>>   _require_command $LVM_PROG lvm
>>   _require_scratch_verity
>> @@ -129,9 +129,14 @@ do
>>   	_udev_wait /dev/mapper/$vgname-$snapname
>>   
>>   	orphan=$(count_item $snap_dev ORPHAN)
>> -	[ $state -eq 0 ] && [ $orphan -gt 0 ] && state=1
>> -
>>   	pre_mount=$(count_merkle_items $snap_dev)
>> +
>> +	if [ $state -eq 0 ]; then
>> +		if [ $orphan -gt 0 ] || [ $pre_mount -gt 0 ]; then
>> +			state=1
>> +		fi
>> +	fi
>> +
>>   	_mount $snap_dev $SCRATCH_MNT || _fail "mount failed at entry $cur"
>>   	fsverity measure $SCRATCH_MNT/fsv >>$seqres.full 2>&1
>>   	measured=$?
>> @@ -143,8 +148,10 @@ do
>>   	echo "entry: $cur, state: $state, orphan: $orphan, pre_mount: $pre_mount, post_mount: $post_mount" >> $seqres.full
>>   
>>   	if [ $state -eq 1 ]; then
>> -		[ $post_mount -eq 0 ] || \
>> -			_fail "mount failed to clear under-construction merkle items pre: $pre_mount, post: $post_mount at entry $cur";
>> +		if [ $measured -ne 0 ]; then
>> +			[ $post_mount -eq 0 ] || \
>> +				_fail "mount failed to clear under-construction merkle items pre: $pre_mount, post: $post_mount at entry $cur";
>> +		fi
>>   	fi
>>   	if [ $state -eq 2 ]; then
>>   		[ $pre_mount -gt 0 ] || \
> 

-- 
Regards,
Disha