On 5/29/24 10:15, Gulam Mohamed wrote:
> When one process opens a loop device partition and another process detaches
> it, there will be a race condition due to which stale loop partitions are
> created causing IO errors. This test will detect the race
>
> Signed-off-by: Gulam Mohamed <gulam.mohamed@xxxxxxxxxx>
thanks for the test, there seems to be an issue with the formatting
of the patch ?
> ---
> tests/loop/010 | 75 ++++++++++++++++++++++++++++++++++++++++++++++
> tests/loop/010.out | 2 ++
> 2 files changed, 77 insertions(+)
> create mode 100755 tests/loop/010
> create mode 100644 tests/loop/010.out
>
> diff --git a/tests/loop/010 b/tests/loop/010
> new file mode 100755
> index 000000000000..aa9c1d33bdb9
> --- /dev/null
> +++ b/tests/loop/010
> @@ -0,0 +1,75 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2024, Oracle and/or its affiliates.
> +#
> +# Test to detect a race between loop detach and loop open which creates
> +# stale loop partitions when one process opens the loop partition and
> +# another process detaches the loop device
> +#
> +. tests/loop/rc
> +DESCRIPTION="check stale loop partition"
> +TIMED=1
> +
> +requires() {
> + _have_program parted
> + _have_program mkfs.xfs
nit:-
do we need to add blkid/udevadm ? if not ignore this comment ..
> +}
> +
> +image_file="$TMPDIR/loopImg"
> +
> +create_loop()
> +{
> + while true
> + do
> + loop_device="$(losetup -P -f --show "${image_file}")"
> + blkid /dev/loop0p1 >> /dev/null 2>&1
> + done
> +}
> +
> +detach_loop()
> +{
> + while true
> + do
> + if [ -e /dev/loop0 ]; then
> + losetup -d /dev/loop0 > /dev/null 2>&1
nit:-
do we want to record this somewhere for debugging purpose instead of
/dev/null ?
> + fi
> + done
> +}
> +
> +test() {
> + echo "Running ${TEST_NAME}"
> + local loop_device
> +
> + truncate -s 1G "${image_file}"
> + parted -a none -s "${image_file}" mklabel gpt
> + loop_device="$(losetup -P -f --show "${image_file}")"
> + parted -a none -s "${loop_device}" mkpart primary 64s 109051s
> +
> + udevadm settle
> + if [ ! -e "${loop_device}" ]; then
> + return 1
> + fi
> +
> + mkfs.xfs -f "${loop_device}p1" > /dev/null 2>&1
> +
same here ...
> + losetup -d "${loop_device}" > /dev/null 2>&1
> +
same here ...
> + create_loop &
> + create_pid=$!
> + detach_loop &
> + detach_pid=$!
> +
> + sleep "${TIMEOUT:-90}"
> + {
> + kill -9 $create_pid
> + kill -9 $detach_pid
> + wait
> + sleep 1
> + } 2>/dev/null
> +
> + losetup -D > /dev/null 2>&1
same here ...
> + if _dmesg_since_test_start | grep -q "partition scan of loop0 failed (rc=-16)"; then
do we want to keep the error message short and achieve same result ?
just a thought feel free to ignore this comment ..
> + echo "Fail"
> + fi
> + echo "Test complete"
> +}
> diff --git a/tests/loop/010.out b/tests/loop/010.out
> new file mode 100644
> index 000000000000..64a6aee00b8a
> --- /dev/null
> +++ b/tests/loop/010.out
> @@ -0,0 +1,2 @@
> +Running loop/010
> +Test complete
apart from formatting and nit comments it looks good ...
Reviewed-by: Chaitanya Kulkarni <kch@xxxxxxxxxx>
-ck
