On Wed, May 21, 2025 at 03:41:51PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> Make sure the lba size of the loop devices created for the metadump
> tests actually match that of the real SCRATCH_ devices or else the tests
> will fail.
>
> Signed-off-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
> ---
> common/metadump | 12 ++++++++++--
> common/rc | 7 +++++++
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
>
> diff --git a/common/metadump b/common/metadump
> index 61ba3cbb91647c..4ae03c605563fc 100644
> --- a/common/metadump
> +++ b/common/metadump
> @@ -76,6 +76,7 @@ _xfs_verify_metadump_v1()
>
> # Create loopdev for data device so we can mount the fs
> METADUMP_DATA_LOOP_DEV=$(_create_loop_device $data_img)
> + _force_loop_device_blocksize $METADUMP_DATA_LOOP_DEV $SCRATCH_DEV
That doesn't look right. You're passing the scratch device as a
block size parameter.
>
> # Mount fs, run an extra test, fsck, and unmount
> SCRATCH_DEV=$METADUMP_DATA_LOOP_DEV _scratch_mount
> @@ -123,12 +124,19 @@ _xfs_verify_metadump_v2()
>
> # Create loopdev for data device so we can mount the fs
> METADUMP_DATA_LOOP_DEV=$(_create_loop_device $data_img)
> + _force_loop_device_blocksize $METADUMP_DATA_LOOP_DEV $SCRATCH_DEV
>
> # Create loopdev for log device if we recovered anything
> - test -s "$log_img" && METADUMP_LOG_LOOP_DEV=$(_create_loop_device $log_img)
> + if [ -s "$log_img" ]; then
> + METADUMP_LOG_LOOP_DEV=$(_create_loop_device $log_img)
> + _force_loop_device_blocksize $METADUMP_LOG_LOOP_DEV $SCRATCH_LOGDEV
> + fi
>
> # Create loopdev for rt device if we recovered anything
> - test -s "$rt_img" && METADUMP_RT_LOOP_DEV=$(_create_loop_device $rt_img)
> + if [ -s "$rt_img" ]; then
> + METADUMP_RT_LOOP_DEV=$(_create_loop_device $rt_img)
> + _force_loop_device_blocksize $METADUMP_RT_LOOP_DEV $SCRATCH_RTDEV
> + fi
>
> # Mount fs, run an extra test, fsck, and unmount
> SCRATCH_DEV=$METADUMP_DATA_LOOP_DEV SCRATCH_LOGDEV=$METADUMP_LOG_LOOP_DEV SCRATCH_RTDEV=$METADUMP_RT_LOOP_DEV _scratch_mount
> diff --git a/common/rc b/common/rc
> index 4e3917a298e072..9e27f7a4afba44 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -4527,6 +4527,8 @@ _create_loop_device()
> }
>
> # Configure the loop device however needed to support the given block size.
> +# The first argument is the loop device; the second is either an integer block
> +# size, or a different block device whose blocksize we want to match.
> _force_loop_device_blocksize()
> {
> local loopdev="$1"
> @@ -4539,6 +4541,11 @@ _force_loop_device_blocksize()
> return 1
> fi
>
> + # second argument is really a bdev; copy its lba size
> + if [ -b "$blksize" ]; then
> + blksize="$(blockdev --getss "${blksize}")"
> + fi
Oh, you're overloading the second parameter with different types -
that's pretty nasty. It would be much cleaner to write a wrapper
function that extracts the block size from the device before calling
_force_loop_device_blocksize()....
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
