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

[Ming Lei <ming.lei@redhat.com>]

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