On Apr 27, 2023 / 18:32, Ziyang Zhang wrote:
> It is very important to test ublk crash handling since the userspace
> part is not reliable. Especially we should test removing device, killing
> ublk daemons and user recovery feature.
Hello Ziyang, thanks for the patches. I think it is valuable to add the ublk
test group and the five test cases. Please find my comments in line. Also, I
saw some trailing spaces to remove.
I tried the two patches on the kernel v6.3, and observed ublk/004 and ublk/005
fail. Do I need some fixes on kernel side to make them pass?
>
> Add five new test for ublk to cover these cases.
nit: s/test/tests/
>
> Signed-off-by: Ziyang Zhang <ZiyangZhang@xxxxxxxxxxxxxxxxx>
> ---
> tests/ublk/001 | 39 +++++++++++++++++++++++++++
> tests/ublk/001.out | 2 ++
> tests/ublk/002 | 53 +++++++++++++++++++++++++++++++++++++
> tests/ublk/002.out | 2 ++
> tests/ublk/003 | 43 ++++++++++++++++++++++++++++++
> tests/ublk/003.out | 2 ++
> tests/ublk/004 | 63 +++++++++++++++++++++++++++++++++++++++++++
> tests/ublk/004.out | 2 ++
> tests/ublk/005 | 66 ++++++++++++++++++++++++++++++++++++++++++++++
> tests/ublk/005.out | 2 ++
> 10 files changed, 274 insertions(+)
> create mode 100644 tests/ublk/001
For historical reason, we add executable mode to the test script files. Please
add the file mode 755 to tests/ublk/001. Same for 002-005.
> create mode 100644 tests/ublk/001.out
> create mode 100644 tests/ublk/002
> create mode 100644 tests/ublk/002.out
> create mode 100644 tests/ublk/003
> create mode 100644 tests/ublk/003.out
> create mode 100644 tests/ublk/004
> create mode 100644 tests/ublk/004.out
> create mode 100644 tests/ublk/005
> create mode 100644 tests/ublk/005.out
>
> diff --git a/tests/ublk/001 b/tests/ublk/001
> new file mode 100644
> index 0000000..fe158ba
> --- /dev/null
> +++ b/tests/ublk/001
> @@ -0,0 +1,39 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2023 Ziyang Zhang
> +#
> +# Test ublk by deleting ublk device while running fio
> +
> +. tests/block/rc
> +. common/ublk
> +
> +DESCRIPTION="test ublk deletion"
> +QUICK=1
> +
> +requires() {
> + _have_ublk
> +}
I suggest to add group_requires() in tests/ublk/rc file, and call _have_ublk
in it. With this, the five test cases do not repeat the same requires().
> +
> +test() {
> + local ublk_prog="src/miniublk"
> +
> + echo "Running ${TEST_NAME}"
> +
> + if ! _init_ublk; then
> + return 1
> + fi
> +
> + ${ublk_prog} add -t null -n 0 > "$FULL"
> + udevadm settle
> + if ! ${ublk_prog} list -n 0 >> "$FULL"; then
> + echo "fail to list dev"
> + fi
> +
> + _run_fio_rand_io --filename=/dev/ublkb0 --time_based --runtime=30 > /dev/null 2>&1 &
> +
> + ${ublk_prog} del -n 0 >> "$FULL"
> +
> + _exit_ublk
> +
> + echo "Test complete"
> +}
> diff --git a/tests/ublk/001.out b/tests/ublk/001.out
> new file mode 100644
> index 0000000..0d070b3
> --- /dev/null
> +++ b/tests/ublk/001.out
> @@ -0,0 +1,2 @@
> +Running ublk/001
> +Test complete
> diff --git a/tests/ublk/002 b/tests/ublk/002
> new file mode 100644
> index 0000000..25cad13
> --- /dev/null
> +++ b/tests/ublk/002
> @@ -0,0 +1,53 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2023 Ziyang Zhang
> +#
> +# Test ublk by killing ublk daemon while running fio
> +# Delete the device after it is dead
> +
> +. tests/block/rc
> +. common/ublk
> +
> +DESCRIPTION="test ublk crash(1)"
This description and that of ublk/003 are not so informative. Could you try to
enrich them? Maybe "test ublk crash with delete after dead confirmation" and
"test ublk crash with delete just after daemon kill".
> +QUICK=1
> +
> +requires() {
> + _have_ublk
> +}
> +
> +test() {
> + local ublk_prog="src/miniublk"
> +
> + echo "Running ${TEST_NAME}"
> +
> + if ! _init_ublk; then
> + return 1
> + fi
> +
> + ${ublk_prog} add -t null -n 0 > "$FULL"
> + udevadm settle
> + if ! ${ublk_prog} list -n 0 >> "$FULL"; then
> + echo "fail to list dev"
> + fi
> +
> + _run_fio_rand_io --filename=/dev/ublkb0 --time_based --runtime=30 > /dev/null 2>&1 &
> +
> + pid=`${ublk_prog} list -n 0 | grep "pid" | awk '{print $7}'`
> + kill -9 $pid
> +
> + sleep 2
> + secs=0
> + while [ $secs -lt 10 ]; do
> + state=`${ublk_prog} list -n 0 | grep "state" | awk '{print $11}'`
> + [ "$state" == "DEAD" ] && break
> + sleep 1
> + let secs++
> + done
> + [ "$state" != "DEAD" ] && echo "device isn't dead after killing queue daemon"
> +
> + ${ublk_prog} del -n 0 >> "$FULL"
> +
> + _exit_ublk
> +
> + echo "Test complete"
> +}
> diff --git a/tests/ublk/002.out b/tests/ublk/002.out
> new file mode 100644
> index 0000000..93039b7
> --- /dev/null
> +++ b/tests/ublk/002.out
> @@ -0,0 +1,2 @@
> +Running ublk/002
> +Test complete
> diff --git a/tests/ublk/003 b/tests/ublk/003
> new file mode 100644
> index 0000000..34bce74
> --- /dev/null
> +++ b/tests/ublk/003
> @@ -0,0 +1,43 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2023 Ziyang Zhang
> +#
> +# Test ublk by killing ublk daemon while running fio
> +# Delete the device immediately
> +
> +. tests/block/rc
> +. common/ublk
> +
> +DESCRIPTION="test ublk crash(2)"
> +QUICK=1
> +
> +requires() {
> + _have_ublk
> +}
> +
> +test() {
> + local ublk_prog="src/miniublk"
> +
> + echo "Running ${TEST_NAME}"
> +
> + if ! _init_ublk; then
> + return 1
> + fi
> +
> + ${ublk_prog} add -t null -n 0 > "$FULL"
> + udevadm settle
> + if ! ${ublk_prog} list -n 0 >> "$FULL"; then
> + echo "fail to list dev"
> + fi
> +
> + _run_fio_rand_io --filename=/dev/ublkb0 --time_based --runtime=30 > /dev/null 2>&1 &
> +
> + pid=`${ublk_prog} list -n 0 | grep "pid" | awk '{print $7}'`
> + kill -9 $pid
> +
> + ${ublk_prog} del -n 0 >> "$FULL"
> +
> + _exit_ublk
> +
> + echo "Test complete"
> +}
> diff --git a/tests/ublk/003.out b/tests/ublk/003.out
> new file mode 100644
> index 0000000..90a3bfa
> --- /dev/null
> +++ b/tests/ublk/003.out
> @@ -0,0 +1,2 @@
> +Running ublk/003
> +Test complete
> diff --git a/tests/ublk/004 b/tests/ublk/004
> new file mode 100644
> index 0000000..c5d0694
> --- /dev/null
> +++ b/tests/ublk/004
> @@ -0,0 +1,63 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2023 Ziyang Zhang
> +#
> +# Test ublk user recovery by run fio with dev recovery:
> +# (1)kill all ubq_deamon, (2)recover with new ubq_daemon,
> +# (3)delete dev
> +
> +. tests/block/rc
> +. common/ublk
> +
> +DESCRIPTION="test ublk recovery(1)"
Same comment as DESCRIPTIONS for crash(1) and crash(2). The descriptions can be
"test ublk recovery with one time daemon kill" and "test ublk recovery with
two times daemon kill", or something like that.
> +QUICK=1
> +
> +requires() {
> + _have_ublk
> +}
> +
> +test() {
> + local ublk_prog="src/miniublk"
> +
> + echo "Running ${TEST_NAME}"
> +
> + if ! _init_ublk; then
> + return 1
> + fi
> +
> + ${ublk_prog} add -t null -n 0 -r 1 > "$FULL"
> + udevadm settle
> + if ! ${ublk_prog} list -n 0 >> "$FULL"; then
> + echo "fail to list dev"
> + fi
> +
> + _run_fio_rand_io --filename=/dev/ublkb0 --time_based --runtime=30 > /dev/null 2>&1 &
> + pid=`${ublk_prog} list -n 0 | grep "pid" | awk '{print $7}'`
> + kill -9 $pid
> +
> + sleep 2
> + secs=0
> + while [ $secs -lt 10 ]; do
> + state=`${ublk_prog} list -n 0 | grep "state" | awk '{print $11}'`
> + [ "$state" == "QUIESCED" ] && break
> + sleep 1
> + let secs++
> + done
> + [ "$state" != "QUIESCED" ] && echo "device isn't quiesced after killing queue daemon"
> +
> + secs=0
> + while [ $secs -lt 10 ]; do
> + ${ublk_prog} recover -t null -n 0 >> "$FULL"
> + [ $? -eq 0 ] && break
> + sleep 1
> + let secs++
> + done
> + state=`${ublk_prog} list -n 0 | grep "state" | awk '{print $11}'`
> + [ "$state" == "QUIESCED" ] && echo "failed to recover dev"
> +
> + ${ublk_prog} del -n 0 >> "$FULL"
> +
> + _exit_ublk
> +
> + echo "Test complete"
> +}
> diff --git a/tests/ublk/004.out b/tests/ublk/004.out
> new file mode 100644
> index 0000000..a92cd50
> --- /dev/null
> +++ b/tests/ublk/004.out
> @@ -0,0 +1,2 @@
> +Running ublk/004
> +Test complete
> diff --git a/tests/ublk/005 b/tests/ublk/005
> new file mode 100644
> index 0000000..23c0555
> --- /dev/null
> +++ b/tests/ublk/005
> @@ -0,0 +1,66 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2023 Ziyang Zhang
> +#
> +# Test ublk user recovery by run fio with dev recovery:
> +# (1)kill all ubq_deamon, (2)recover with new ubq_daemon,
> +# (3)kill all ubq_deamon, (4)delete dev
> +
> +. tests/block/rc
> +. common/ublk
> +
> +DESCRIPTION="test ublk recovery(2)"
> +QUICK=1
> +
> +requires() {
> + _have_ublk
> +}
> +
> +test() {
> + local ublk_prog="src/miniublk"
> +
> + echo "Running ${TEST_NAME}"
> +
> + if ! _init_ublk; then
> + return 1
> + fi
> +
> + ${ublk_prog} add -t null -n 0 -r 1 > "$FULL"
> + udevadm settle
> + if ! ${ublk_prog} list -n 0 >> "$FULL"; then
> + echo "fail to list dev"
> + fi
> +
> + _run_fio_rand_io --filename=/dev/ublkb0 --time_based --runtime=30 > /dev/null 2>&1 &
> + pid=`${ublk_prog} list -n 0 | grep "pid" | awk '{print $7}'`
> + kill -9 $pid
> +
> + sleep 2
> + secs=0
> + while [ $secs -lt 10 ]; do
> + state=`${ublk_prog} list -n 0 | grep "state" | awk '{print $11}'`
> + [ "$state" == "QUIESCED" ] && break
> + sleep 1
> + let secs++
> + done
> + [ "$state" != "QUIESCED" ] && echo "device isn't quiesced after killing queue daemon"
> +
> + secs=0
> + while [ $secs -lt 10 ]; do
> + ${ublk_prog} recover -t null -n 0 >> "$FULL"
> + [ $? -eq 0 ] && break
> + sleep 1
> + let secs++
> + done
> + state=`${ublk_prog} list -n 0 | grep "state" | awk '{print $11}'`
> + [ "$state" == "QUIESCED" ] && echo "failed to recover dev"
> +
> + pid=`${ublk_prog} list -n 0 | grep "pid" | awk '{print $7}'`
> + kill -9 $pid
> +
> + ${ublk_prog} del -n 0 >> "$FULL"
> +
> + _exit_ublk
> +
> + echo "Test complete"
> +}
> diff --git a/tests/ublk/005.out b/tests/ublk/005.out
> new file mode 100644
> index 0000000..20d7b38
> --- /dev/null
> +++ b/tests/ublk/005.out
> @@ -0,0 +1,2 @@
> +Running ublk/005
> +Test complete
> --
> 2.31.1
>
