| Message ID | 20231106003523.1923694-1-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [blktests] ublk/rc: prefer to rublk over miniublk | expand |
On Nov 06, 2023 / 08:35, Ming Lei wrote: > Add one wrapper script for using rublk to run ublk tests, and prefer > to rublk because it is well implemented and more reliable. > > This way has been run for months in rublk's github CI test. > > https://github.com/ming1/rublk > > Signed-off-by: Ming Lei <ming.lei@redhat.com> Hi Ming, it sounds good to shift from miniublk to rublk to reduce maintenance work of src/miniublk. I tried the patch, and found a couple of points to improve. 1) The issue that Akinobu addressed with the commit 880fb6afff2e is observed with rublk. I did the command lines below which were noted in his commit: $ modprobe -r scsi_debug $ modprobe scsi_debug sector_size=4096 dev_size_mb=2048 $ mkfs.ext4 /dev/sdX $ mount /dev/sdX results/ $ ./check ublk/003 Then I observed the failure: ublk/003 (test mounting block device exported by ublk) [failed] runtime 0.529s ... 0.465s --- tests/ublk/003.out 2023-09-05 10:05:11.292889193 +0900 +++ /home/shin/Blktests/blktests/results/nodev/ublk/003.out.bad 2023-11-07 14:18:44.966654288 +0900 @@ -1,2 +1,3 @@ Running ublk/003 +got , should be ext4 Test complete So I guess rublk needs a similar fix as Akinobu did for miniublk. 2) I ran shellcheck for src/rublk_wrapper.sh and observed two meesages: src/rublk_wrapper.sh:10:12: error: Double quote array expansions to avoid re-splitting elements. [SC2068] src/rublk_wrapper.sh:32:7: note: Double quote to prevent globbing and word splitting. [SC2086] I suggest to apply changes below to make the script a bit more robust. diff --git a/Makefile b/Makefile index 4bed1da..43f2ab0 100644 --- a/Makefile +++ b/Makefile @@ -19,7 +19,7 @@ SHELLCHECK_EXCLUDE := SC2119 check: shellcheck -x -e $(SHELLCHECK_EXCLUDE) -f gcc check new common/* \ - tests/*/rc tests/*/[0-9]*[0-9] + tests/*/rc tests/*/[0-9]*[0-9] src/*.sh ! grep TODO tests/*/rc tests/*/[0-9]*[0-9] ! find -name '*.out' -perm /u=x+g=x+o=x -printf '%p is executable\n' | grep . diff --git a/src/rublk_wrapper.sh b/src/rublk_wrapper.sh index 803743e..2e79a01 100755 --- a/src/rublk_wrapper.sh +++ b/src/rublk_wrapper.sh @@ -4,9 +4,9 @@ # # rublk wrapper for adapting miniublk's command line -PARA="" +PARA=() ACT=$1 -for arg in $@; do +for arg in "$@"; do if [ "$arg" = "-t" ]; then continue fi @@ -23,9 +23,9 @@ for arg in $@; do if [ "$arg" = "-f" ]; then continue fi - PARA+=" $arg" + PARA+=("$arg") else - PARA+=" $arg" + PARA+=("$arg") fi done -rublk $PARA +rublk "${PARA[@]}"
Hi Shinichiro, On Tue, Nov 07, 2023 at 05:28:59AM +0000, Shinichiro Kawasaki wrote: > On Nov 06, 2023 / 08:35, Ming Lei wrote: > > Add one wrapper script for using rublk to run ublk tests, and prefer > > to rublk because it is well implemented and more reliable. > > > > This way has been run for months in rublk's github CI test. > > > > https://github.com/ming1/rublk > > > > Signed-off-by: Ming Lei <ming.lei@redhat.com> > > Hi Ming, it sounds good to shift from miniublk to rublk to reduce maintenance > work of src/miniublk. I tried the patch, and found a couple of points to > improve. > > 1) The issue that Akinobu addressed with the commit 880fb6afff2e is observed > with rublk. I did the command lines below which were noted in his commit: > > $ modprobe -r scsi_debug > $ modprobe scsi_debug sector_size=4096 dev_size_mb=2048 > $ mkfs.ext4 /dev/sdX > $ mount /dev/sdX results/ > $ ./check ublk/003 > > Then I observed the failure: > > ublk/003 (test mounting block device exported by ublk) [failed] > runtime 0.529s ... 0.465s > --- tests/ublk/003.out 2023-09-05 10:05:11.292889193 +0900 > +++ /home/shin/Blktests/blktests/results/nodev/ublk/003.out.bad 2023-11-07 14:18:44.966654288 +0900 > @@ -1,2 +1,3 @@ > Running ublk/003 > +got , should be ext4 > Test complete > > So I guess rublk needs a similar fix as Akinobu did for miniublk. Indeed, rublk-loop just takes default 512B as block size, and will fix it in v0.1.3, which also supports to specify logical/physical block size from command line, and I will add one test case in blktests later. > > > 2) I ran shellcheck for src/rublk_wrapper.sh and observed two meesages: > > src/rublk_wrapper.sh:10:12: error: Double quote array expansions to avoid re-splitting elements. [SC2068] > src/rublk_wrapper.sh:32:7: note: Double quote to prevent globbing and word splitting. [SC2086] > > I suggest to apply changes below to make the script a bit more robust. Good catch, thanks for the improvement, and I will integrate it into next version. Thanks, Ming
diff --git a/README.md b/README.md index b6445d6..313a3cc 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,7 @@ Some tests require the following: - multipath-tools (Debian, openSUSE, Arch Linux) or device-mapper-multipath (Fedora) - dmsetup (Debian) or device-mapper (Fedora, openSUSE, Arch Linux) +- rublk (`cargo install --version=0.1.2 rublk`) for ublk test Build blktests with `make`. Optionally, install it to a known location with `make install` (`/usr/local/blktests` by default, but this can be changed by diff --git a/src/rublk_wrapper.sh b/src/rublk_wrapper.sh new file mode 100755 index 0000000..803743e --- /dev/null +++ b/src/rublk_wrapper.sh @@ -0,0 +1,31 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-3.0+ +# Copyright (C) 2023 Ming Lei <ming.lei@redhat.com> +# +# rublk wrapper for adapting miniublk's command line + +PARA="" +ACT=$1 +for arg in $@; do + if [ "$arg" = "-t" ]; then + continue + fi + + if [ "$ACT" = "recover" ]; then + if [ "$arg" = "loop" ] || [ "$arg" = "null" ]; then + continue; + fi + + if [ -f "$arg" ]; then + continue + fi + + if [ "$arg" = "-f" ]; then + continue + fi + PARA+=" $arg" + else + PARA+=" $arg" + fi +done +rublk $PARA diff --git a/tests/ublk/rc b/tests/ublk/rc index c553296..5fbf861 100644 --- a/tests/ublk/rc +++ b/tests/ublk/rc @@ -14,4 +14,8 @@ group_requires() { _have_fio } -export UBLK_PROG="src/miniublk" +if which rublk > /dev/null 2>&1; then + export UBLK_PROG="src/rublk_wrapper.sh" +else + export UBLK_PROG="src/miniublk" +fi
Add one wrapper script for using rublk to run ublk tests, and prefer to rublk because it is well implemented and more reliable. This way has been run for months in rublk's github CI test. https://github.com/ming1/rublk Signed-off-by: Ming Lei <ming.lei@redhat.com> --- README.md | 1 + src/rublk_wrapper.sh | 31 +++++++++++++++++++++++++++++++ tests/ublk/rc | 6 +++++- 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100755 src/rublk_wrapper.sh