[PATCH blktests] ublk/rc: prefer to rublk over miniublk

[PATCH blktests] ublk/rc: prefer to rublk over miniublk

[Ming Lei]

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

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
-- 
2.41.0

Re: [PATCH blktests] ublk/rc: prefer to rublk over miniublk

[Shinichiro Kawasaki]

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[@]}"

Re: [PATCH blktests] ublk/rc: prefer to rublk over miniublk

[Ming Lei]

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