Re: [PATCH blktests v2] nbd: add a module load and device connect test

[Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>]

On Jul 07, 2022 / 11:56, Sun Ke wrote:
> This is a regression test for commit 06c4da89c24e
> nbd: call genl_unregister_family() first in nbd_cleanup()

Hello Sun, thanks for the patch.

I reverted the commit 06c4da89c24e from the kernel v5.19-rc5 and observed this
test case passes. I checked dmesg and saw kernel message below. Kernel did not
report the BUG which was noted in the commit 06c4da89c24e. Instead, nbd driver
printed out an error "couldn't allocate config".

Jul 07 20:47:01 redsun45q unknown: run blktests nbd/004 at 2022-07-07 20:47:01
Jul 07 20:47:01 redsun45q kernel: couldn't allocate config
Jul 07 20:47:01 redsun45q kernel: couldn't allocate config
Jul 07 20:47:01 redsun45q kernel: couldn't allocate config
Jul 07 20:47:01 redsun45q kernel: sysfs: cannot create duplicate filename '/devices/virtual/block/nbd0'
Jul 07 20:47:01 redsun45q kernel: CPU: 0 PID: 1043 Comm: modprobe Not tainted 5.19.0-rc5+ #2
Jul 07 20:47:01 redsun45q kernel: Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
Jul 07 20:47:01 redsun45q kernel: Call Trace:
Jul 07 20:47:01 redsun45q kernel:  <TASK>
Jul 07 20:47:01 redsun45q kernel:  dump_stack_lvl+0x5b/0x74
Jul 07 20:47:01 redsun45q kernel:  sysfs_warn_dup.cold+0x17/0x23
...

or sysfs printed out an warning:

[  366.098479] run blktests nbd/004 at 2022-07-07 21:17:22
[  366.747180] sysfs: cannot create duplicate filename '/devices/virtual/block/nbd0'
[  366.749653] CPU: 0 PID: 1508 Comm: modprobe Not tainted 5.19.0-rc5+ #2
[  366.751680] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebui
lt.qemu.org 04/01/2014
[  366.755169] Call Trace:
[  366.755991]  <TASK>
[  366.756722]  dump_stack_lvl+0x5b/0x74
[  366.757909]  sysfs_warn_dup.cold+0x17/0x23
...

It would be the better to catch these error and warning message in the test
case. For example, following hunk will catch it.

diff --git a/tests/nbd/004 b/tests/nbd/004
index 6b2c5ff..9277c10 100755
--- a/tests/nbd/004
+++ b/tests/nbd/004
@@ -47,6 +47,12 @@ test() {
        } 2>/dev/null

        _stop_nbd_server_netlink
+
+       if _dmesg_since_test_start | grep -q -e "couldn't allocate config" \
+          -e "cannot create duplicate filename"; then
+               echo "Fail"
+       fi
+
        echo "Test complete"
 }


> 
> Two concurrent processes，one load and unlock nbd module
> cyclically, the other one connect and disconnect nbd device cyclically.
> Last for 10 seconds.
> 
> Signed-off-by: Sun Ke <sunke32@huawei.com>
> ---
> v1->v2: 
> 1.change install/uninstall to load/unlock

My guess is that Christoph meant "load/unload".

> 2.use _have_modules instead
> 
>  tests/nbd/004     | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/nbd/004.out |  2 ++
>  tests/nbd/rc      | 18 ++++++++++++++++++
>  3 files changed, 72 insertions(+)
>  create mode 100755 tests/nbd/004
>  create mode 100644 tests/nbd/004.out
> 
> diff --git a/tests/nbd/004 b/tests/nbd/004
> new file mode 100755
> index 0000000..6b2c5ff
> --- /dev/null
> +++ b/tests/nbd/004
> @@ -0,0 +1,52 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2022 Sun Ke
> +#
> +# Regression test for commit 06c4da89c24e
> +# nbd: call genl_unregister_family() first in nbd_cleanup()
> +
> +. tests/nbd/rc
> +
> +DESCRIPTION="module load/unlock concurrently with connect/disconnect"
> +QUICK=1
> +
> +requires() {
> +	_have_modules

This helper function needs an argument as the module name to check, nbd.

-- 
Shin'ichiro Kawasaki