Re: [PATCH blktests v2 3/3] Add tests for the SRP initiator and target drivers

[Omar Sandoval <osandov@osandov.com>]

On Wed, Jun 27, 2018 at 02:49:08PM -0700, Bart Van Assche wrote:
> This patch adds the following tests:
> 001: Create and remove LUNs
> 002: File I/O on top of multipath concurrently with logout and login (mq)
> 003: File I/O on top of multipath concurrently with logout and login (sq)
> 004: File I/O on top of multipath concurrently with logout and login (sq-on-mq)
> 005: Direct I/O with large transfer sizes, cmd_sg_entries=255 and bs=4M
> 006: Direct I/O with large transfer sizes, cmd_sg_entries=255 and bs=8M
> 007: Direct I/O with large transfer sizes, cmd_sg_entries=1 and bs=4M
> 008: Direct I/O with large transfer sizes, cmd_sg_entries=1 and bs=8M
> 009: Buffered I/O with large transfer sizes, cmd_sg_entries=255 and bs=4M
> 010: Buffered I/O with large transfer sizes, cmd_sg_entries=255 and bs=8M
> 011: Block I/O on top of multipath concurrently with logout and login
> 012: dm-mpath on top of multiple I/O schedulers
> 013: Direct I/O using a discontiguous buffer
> 
> Other changes in this patch are:
> - Document required kernel config options, user space packages and
>   multipath.conf in README.md.
> - Add file tests/srp/functions with shell functions that are used by
>   multiple SRP tests.
> - Add file tests/srp/rc.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> ---

Thanks, Bart, happy to see this making progress :)

The tests currently fail like this for me:

srp/001 (Create and remove LUNs)                             [failed]
runtime  18.238s  ...  17.925s
    --- tests/srp/001.out       2018-06-28 15:18:36.533835920 -0700
    +++ results/nodev/srp/001.out.bad   2018-06-28 16:21:53.187213618 -0700
    @@ -1,6 +1,6 @@
    +Unloaded the ib_srp kernel module
     Unloaded the ib_srpt kernel module
     Unloaded the rdma_rxe kernel module
    -
     Configured SRP target driver
     Unloaded the ib_srp kernel module
     count_luns(): 3 <> 3
srp/002 (File I/O on top of multipath concurrently with logout and login (mq)) [failed]
runtime  6.680s  ...  6.640s
    --- tests/srp/002.out       2018-06-28 15:18:36.537169282 -0700
    +++ results/nodev/srp/002.out.bad   2018-06-28 16:21:59.930603931 -0700
    @@ -3,5 +3,4 @@
     Unloaded the rdma_rxe kernel module
     Configured SRP target driver
     Unloaded the ib_srp kernel module
    -Unloaded the ib_srpt kernel module
    -Unloaded the rdma_rxe kernel module
    +/dev/disk/by-id/dm-uuid-mpath-3600140572616d6469736b31000000000: not found

And everything else fails like srp/002.

Some more comments below.

>  README.md           |   71 +++
>  tests/srp/001       |   71 +++
>  tests/srp/001.out   |    8 +
>  tests/srp/002       |   49 ++
>  tests/srp/002.out   |    7 +
>  tests/srp/003       |   50 ++
>  tests/srp/003.out   |    7 +
>  tests/srp/004       |   50 ++
>  tests/srp/004.out   |    7 +
>  tests/srp/005       |   40 ++
>  tests/srp/005.out   |    7 +
>  tests/srp/006       |   40 ++
>  tests/srp/006.out   |    7 +
>  tests/srp/007       |   40 ++
>  tests/srp/007.out   |    7 +
>  tests/srp/008       |   39 ++
>  tests/srp/008.out   |    7 +
>  tests/srp/009       |   40 ++
>  tests/srp/009.out   |    7 +
>  tests/srp/010       |   40 ++
>  tests/srp/010.out   |    7 +
>  tests/srp/011       |   44 ++
>  tests/srp/011.out   |    7 +
>  tests/srp/012       |   52 ++
>  tests/srp/012.out   |    8 +
>  tests/srp/013       |   48 ++
>  tests/srp/013.out   |    8 +
>  tests/srp/functions | 1288 +++++++++++++++++++++++++++++++++++++++++++
>  tests/srp/rc        |   54 ++
>  29 files changed, 2110 insertions(+)
>  create mode 100755 tests/srp/001
>  create mode 100644 tests/srp/001.out
>  create mode 100755 tests/srp/002
>  create mode 100644 tests/srp/002.out
>  create mode 100755 tests/srp/003
>  create mode 100644 tests/srp/003.out
>  create mode 100755 tests/srp/004
>  create mode 100644 tests/srp/004.out
>  create mode 100755 tests/srp/005
>  create mode 100644 tests/srp/005.out
>  create mode 100755 tests/srp/006
>  create mode 100644 tests/srp/006.out
>  create mode 100755 tests/srp/007
>  create mode 100644 tests/srp/007.out
>  create mode 100755 tests/srp/008
>  create mode 100644 tests/srp/008.out
>  create mode 100755 tests/srp/009
>  create mode 100644 tests/srp/009.out
>  create mode 100755 tests/srp/010
>  create mode 100644 tests/srp/010.out
>  create mode 100755 tests/srp/011
>  create mode 100644 tests/srp/011.out
>  create mode 100755 tests/srp/012
>  create mode 100644 tests/srp/012.out
>  create mode 100755 tests/srp/013
>  create mode 100644 tests/srp/013.out
>  create mode 100755 tests/srp/functions
>  create mode 100755 tests/srp/rc
> 
> diff --git a/README.md b/README.md
> index 78a41567416e..d8df938700cd 100644
> --- a/README.md
> +++ b/README.md
> @@ -6,6 +6,38 @@ filesystem testing framework.
>  
>  ## Getting Started
>  
> +Make sure that at least the following symbols are set in the kernel config:
> +
> +* CONFIG_BLK_DEV_DM
> +* CONFIG_BLK_DEV_NULL_BLK
> +* CONFIG_BLK_DEV_RAM
> +* CONFIG_BLK_DEV_SD
> +* CONFIG_CHR_DEV_SG
> +* CONFIG_DM_MULTIPATH
> +* CONFIG_DM_MULTIPATH_QL
> +* CONFIG_DM_MULTIPATH_ST
> +* CONFIG_INFINIBAND
> +* CONFIG_INFINIBAND_ADDR_TRANS
> +* CONFIG_INFINIBAND_IPOIB
> +* CONFIG_INFINIBAND_SRP
> +* CONFIG_INFINIBAND_SRPT
> +* CONFIG_INFINIBAND_USER_ACCESS
> +* CONFIG_INFINIBAND_USER_MAD
> +* CONFIG_INFINIBAND_USER_MEM
> +* CONFIG_NVME_CORE
> +* CONFIG_NVME_RDMA
> +* CONFIG_NVME_TARGET
> +* CONFIG_NVME_TARGET_RDMA
> +* CONFIG_RDMA_RXE
> +* CONFIG_SCSI_DEBUG
> +* CONFIG_SCSI_DH_ALUA
> +* CONFIG_SCSI_DH_EMC
> +* CONFIG_SCSI_DH_RDAC
> +* CONFIG_SCSI_SRP_ATTRS
> +* CONFIG_TARGET_CORE
> +* CONFIG_TCM_FILEIO
> +* CONFIG_TCM_IBLOCK

Why don't all of these have _have_module checks in
group_requires()/requires()?

>  The dependencies are minimal, but make sure you have them installed:
>  
>  - bash 4
> @@ -13,6 +45,10 @@ The dependencies are minimal, but make sure you have them installed:
>  - GNU awk
>  - util-linux
>  - fio
> +- gcc
> +- make
> +- multipath-tools or device-mapper-multipath
> +- e2fsprogs and xfsprogs
>  
>  Add the list of block devices you want to test on in a file named `config`:
>  
> @@ -20,6 +56,41 @@ Add the list of block devices you want to test on in a file named `config`:
>  TEST_DEVS=(/dev/nvme0n1 /dev/sdb)
>  ```
>  
> +For the SRP tests, merge or copy the following into /etc/multipathd.conf and
> +restart multipathd:
> +
> +<span></span>
> +
> +    defaults {
> +        user_friendly_names     yes
> +        queue_without_daemon    no
> +    }
> +    devices {
> +        device {
> +            vendor       "LIO-ORG|SCST_BIO|FUSIONIO"
> +            product      ".*"
> +            features     "1 queue_if_no_path"
> +            path_checker tur
> +        }
> +    }
> +    blacklist {
> +        device {
> +            vendor  "ATA"
> +        }
> +        device {
> +            vendor  "QEMU"
> +        }
> +        device {
> +            vendor  "Linux"
> +            product "scsi_debug"
> +        }
> +        devnode     "^nullb.*"
> +    }
> +    blacklist_exceptions {
> +        property        ".*"
> +        devnode         "^nvme"
> +    }
> +

Does multipathd have any way to run with a custom config file, so we can
just start it on demand instead of having to do this?

[snip]

> diff --git a/tests/srp/functions b/tests/srp/functions

This stuff should just go in tests/srp/rc.

> new file mode 100755
> index 000000000000..8fe654711613
> --- /dev/null
> +++ b/tests/srp/functions
> @@ -0,0 +1,1288 @@
> +#!/bin/bash
> +#
> +# Copyright (c) 2016-2018 Western Digital Corporation or its affiliates
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License
> +# as published by the Free Software Foundation; either version 2
> +# of the License, or (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc.
> +
> +have_brd() {
> +	modinfo brd >/dev/null 2>&1
> +}
> +
> +scsi_debug_dev_path() {
> +	local bd d
> +
> +	for d in /sys/bus/pseudo/drivers/scsi_debug/adapter*/host*/target*/*/block/*; do
> +		[ -e "$d" ] || continue
> +		bd=${d/*\//}
> +	done
> +	[ -n "$bd" ] || return 1
> +	echo "/dev/$bd"
> +}
> +
> +vdev_path=(/dev/ram0 /dev/ram1 "$(scsi_debug_dev_path)")
> +scsi_serial=(ramdisk1 ramdisk2 scsidbg)
> +memtotal=$(sed -n 's/^MemTotal:[[:blank:]]*\([0-9]*\)[[:blank:]]*kB$/\1/p' /proc/meminfo)
> +max_ramdisk_size=$((1<<25))
> +if have_brd; then
> +    ramdisk_size=$((memtotal*(1024/16)))  # in bytes
> +    if [ $ramdisk_size -gt $max_ramdisk_size ]; then
> +	ramdisk_size=$max_ramdisk_size
> +    fi
> +elif [ -e /sys/class/block/ram0 ]; then
> +    # in bytes
> +    ramdisk_size=$(($(</sys/class/block/ram0/size) * 512))
> +else
> +	echo "Error: could not find /dev/ram0"
> +	exit 1
> +fi

If brd isn't enabled, I get hilarity:

$ sudo ./check srp
./check: line 451: tests/Error: could not find /dev/rc: No such file or directory
./check: line 410: tests/Error: could not find /dev/ram0
: No such file or directory

At the very least, this check should be happening in group_requires(),
and brd should just be mandatory. Or even better, is there a reason we
can't use the persistent mode of null_blk instead of brd?

[snip]

> +# Load the SRP initiator driver with kernel module parameters $1..$n.
> +start_srp() {
> +	modprobe scsi_transport_srp || return $?
> +	modprobe ib_srp "$@" dyndbg=+pmf || return $?
> +}

These modprobes (and all of the other ones in this file) should have
matching _have_module checks in group_requires() or requires().

[snip]

> +	# Load the I/O scheduler kernel modules
> +	(
> +		cd "/lib/modules/$(uname -r)/kernel/block" &&
> +		    for m in *.ko; do
> +			    modprobe "${m%.ko}"
> +		    done
> +	)

All of my schedulers are built in, so I get:

+modprobe: FATAL: Module * not found in directory /lib/modules/4.18.0-rc2

This should handle that case and go in check instead of here.

> +	if [ -d /sys/kernel/debug/dynamic_debug ]; then
> +		for m in ; do
> +			echo "module $m +pmf" >/sys/kernel/debug/dynamic_debug/control
> +		done
> +	fi
> +
> +	start_target
> +}
> diff --git a/tests/srp/rc b/tests/srp/rc
> new file mode 100755
> index 000000000000..f5de9610dc2b
> --- /dev/null
> +++ b/tests/srp/rc
> @@ -0,0 +1,54 @@
> +#!/bin/bash
> +#
> +# Copyright (c) 2018 Western Digital Corporation or its affiliates
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License
> +# as published by the Free Software Foundation; either version 2
> +# of the License, or (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc.
> +
> +. common/rc
> +
> +is_lio_configured() {
> +	(
> +		cd /sys/kernel/config/target >&/dev/null || return 1
> +		for e in target/* core/fileio* core/iblock* core/pscsi*; do
> +			[ -d "$e" ] && [ "$e" != core ] && return 0
> +		done
> +	)
> +
> +	return 1
> +}
> +
> +group_requires() {
> +	_have_configfs || return $?
> +	if is_lio_configured; then
> +		echo "Error: LIO must be unloaded before the SRP tests are run"

This should set SKIP_REASON instead of echoing.

> +		return 1
> +	fi
> +	_have_module dm_multipath || return $?
> +	_have_module ib_srp || return $?
> +	_have_module ib_srpt || return $?
> +	_have_module sd_mod || return $?
> +	_have_program mkfs.ext4 || return $?
> +	_have_program mkfs.xfs || return $?
> +	_have_program multipath || return $?
> +	_have_program multipathd || return $?
> +	_have_program pidof || return $?
> +	_have_program sg_reset || return $?
> +	_have_root || return $?
> +
> +	if ! pidof multipathd >/dev/null; then
> +		echo "Error: multipathd is not running"

Same, set SKIP_REASON.

> +		return 1
> +	fi
> +}
> -- 
> 2.17.1
>