Re: [PATCH v6 01/11] fstests: add _loop_image_create_clone() helper

[Anand Suveer Jain <asj@kernel.org>]

On 29/5/26 12:27, Darrick J. Wong wrote:
> On Thu, May 28, 2026 at 12:05:32PM +0800, Anand Jain wrote:
>> Introduce _loop_image_create_clone() and _loop_image_destroy() to mkfs an
>> image file and clone it to another image file, and attach a loop device to
>> them. And its destroy part.
>>
>> Signed-off-by: Anand Jain <asj@kernel.org>
>> ---
>>  common/rc | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 63 insertions(+)
>>
>> diff --git a/common/rc b/common/rc
>> index 79189e7e6e94..d7e3e0bdfb1e 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -1520,6 +1520,69 @@ _scratch_resvblks()
>>  	esac
>>  }
>>  
>> +# Create a small loop image, run an optional tuning function ($2) on it,
>> +# clone it, and attach both to loop devices, returned in ($1).
>> +# Args:
>> +#   $1: Nameref to return the array of allocated loop devices [base, clone].
>> +#   $2: Optional callback function to tune the base filesystem before cloning.
>> +_loop_image_create_clone()
>> +{
>> +	local -n _ret=$1
> 
> That switch   ^^ is very clever.  I always wondered how one did indirect
> variables in bash.
> 
>> +	local pre_clone_tune_func="$2"
>> +	local img_file=$TEST_DIR/${seq}.img
>> +	local img_file_clone=$TEST_DIR/${seq}_clone.img
>> +	local size=$(_small_fs_size_mb 128) # Smallest possible
>> +	local loop_devs
>> +
>> +	# Since we copy the block device image, we keep its size small.
>> +	_require_fs_space $TEST_DIR $((size * 1024))
>> +
>> +	_create_file_sized $((size * 1024 * 1024)) $img_file ||
>> +				_fail "Failed: Create $img_file $size"
>> +
>> +	loop_devs=$(_create_loop_device $img_file)
>> +	_ret=($loop_devs)
> 
> Should this check that a loopdev actually got created?
> 

Hmm, in the function _create_loop_device(), we are
calling _fail if create fails, so no need to duplicate, right?

>> +	case $FSTYP in
>> +	xfs)
>> +		_mkfs_dev "-s size=4096" ${loop_devs[0]}
>> +		;;
>> +	btrfs)
>> +		_mkfs_dev ${loop_devs[0]}
>> +		;;
>> +	*)
>> +		_mkfs_dev ${loop_devs[0]}
>> +		;;
>> +	esac
>> +
>> +	# Only execute if the function argument is not empty
>> +	if [ -n "$pre_clone_tune_func" ]; then
>> +		$pre_clone_tune_func ${loop_devs[0]}
>> +	fi
>> +
>> +	sync ${loop_devs[0]}
>> +	cp $img_file $img_file_clone
>> +


>> +	loop_devs="$loop_devs $(_create_loop_device $img_file_clone)"
> 
> 	local lodev="$(_create_loop_device ...)"
> 
> 	test -z "$lodev" && _fail "second loopdev not created"
> 	_ret+=("$lodev")
> 
> ?

If the second `_create_loop_device()` happens to fail, it will
already have called `_fail`, so "second loopdev..." won't be
used at all.


Thanks, Anand



>> +
>> +	_ret=($loop_devs)
>> +}
>> +
>> +# Teardown loop devices and delete their underlying backing image files.
>> +# Accepts a list of loop device paths (e.g., /dev/loop0 /dev/loop1).
>> +_loop_image_destroy()
>> +{
>> +	for d in "$@"; do
>> +		# Retrieve the path of the backing file
>> +		local f=$(losetup --noheadings --output BACK-FILE $d)
>> +
>> +		# Detach the loop device from the backing file
>> +		_destroy_loop_device "$d"
>> +
>> +		# Clean up the backing disk image file
>> +		[ -n "$f" ] && rm -f "$f"
>> +	done
>> +}
>>  
>>  # Repair scratch filesystem.  Returns 0 if the FS is good to go (either no
>>  # errors found or errors were fixed) and nonzero otherwise; also spits out
>> -- 
>> 2.43.0
>>
>>