CC+: Ofir A heads up: this patch causes conflict with Ofir's patch to move tests/nvme/rc helpers to common/nvme [*]. Depending on which comes first, Daniel or Ofir will need patch rework. [*] https://lore.kernel.org/linux-block/20240624104620.2156041-2-ofir.gal@xxxxxxxxxxx/ On Jun 27, 2024 / 11:10, Daniel Wagner wrote: > Most of the NVMEeoF tests are exercising the host code of the nvme > subsystem. There is no real reason not to run these against a real > target. We just have to skip the soft target setup and make it possible > to setup a remote target. > > Because all tests use now the common setup/cleanup helpers we just need > to intercept this call and forward it to an external component. > > As we already have various nvme variables to setup the target which we > should allow to overwrite. Also introduce a NVME_TARGET_CONTROL variable > which points to a script which gets executed whenever a targets needs to > be created/destroyed. > > Signed-off-by: Daniel Wagner <dwagner@xxxxxxx> Daniel, thanks for this v3 patch. Please find some comments in line. I ran "make check" and observed some shellcheck warnings: $ make check shellcheck -x -e SC2119 -f gcc check common/* \ tests/*/rc tests/*/[0-9]*[0-9] src/*.sh tests/nvme/rc:962:5: warning: eval negates the benefit of arrays. Drop eval to preserve whitespace/symbols (or eval as string). [SC2294] tests/nvme/041:34:36: note: Double quote to prevent globbing and word splitting. [SC2086] tests/nvme/042:40:52: note: Double quote to prevent globbing and word splitting. [SC2086] tests/nvme/042:54:61: note: Double quote to prevent globbing and word splitting. [SC2086] tests/nvme/043:37:36: note: Double quote to prevent globbing and word splitting. [SC2086] tests/nvme/044:36:36: note: Double quote to prevent globbing and word splitting. [SC2086] tests/nvme/044:42:36: note: Double quote to prevent globbing and word splitting. [SC2086] tests/nvme/045:41:36: note: Double quote to prevent globbing and word splitting. [SC2086] tests/nvme/045:47:36: note: Double quote to prevent globbing and word splitting. [SC2086] tests/nvme/045:72:43: note: Double quote to prevent globbing and word splitting. [SC2086] tests/nvme/045:82:43: note: Double quote to prevent globbing and word splitting. [SC2086] tests/nvme/051:40:25: note: Double quote to prevent globbing and word splitting. [SC2086] tests/nvme/051:41:25: note: Double quote to prevent globbing and word splitting. [SC2086] > --- > Documentation/running-tests.md | 33 ++++++++++++++++++++ > check | 4 +++ > tests/nvme/rc | 57 ++++++++++++++++++++++++++++++++-- > 3 files changed, 92 insertions(+), 2 deletions(-) > > diff --git a/Documentation/running-tests.md b/Documentation/running-tests.md > index 968702e76bb5..fe4f729bd037 100644 > --- a/Documentation/running-tests.md > +++ b/Documentation/running-tests.md > @@ -120,6 +120,9 @@ The NVMe tests can be additionally parameterized via environment variables. > - NVME_NUM_ITER: 1000 (default) > The number of iterations a test should do. This parameter had an old name > 'nvme_num_iter'. The old name is still usable, but not recommended. > +- NVME_TARGET_CONTROL: When defined, the generic target setup/cleanup code will > + be skipped and this script gets called. This makes it possible to run > + the fabric nvme tests against a real target. > > ### Running nvme-rdma and SRP tests > > @@ -167,3 +170,33 @@ if ! findmnt -t configfs /sys/kernel/config > /dev/null; then > mount -t configfs configfs /sys/kernel/config > fi > ``` > +### NVME_TARGET_CONTROL > + > +When NVME_TARGET_CONTROL is set, blktests will call the script which the > +environment variable points to, to fetch the configuration values to be used for > +the runs, e.g subsysnqn or hostnqn. This allows the blktest to be run against > +external configured/setup targets. > + > +The blktests expects that the script interface implements following > +commands: > + > +config: > + --show-blkdev-type > + --show-trtype > + --show-hostnqn > + --show-hostid > + --show-host-traddr > + --show-traddr > + --show-trsvid > + --show-subsys-uuid > + --show-subsysnqn > + > +setup: > + --subsysnqn SUBSYSNQN > + --subsys-uuid SUBSYS_UUID > + --hostnqn HOSTNQN > + --ctrlkey CTRLKEY > + --hostkey HOSTKEY > + > +cleanup: > + --subsysnqn SUBSYSNQN Thanks. I think the NVME_TARGET_CONTROL script command line interface is well documented. > diff --git a/check b/check > index 3ed4510f3f40..d0475629773d 100755 > --- a/check > +++ b/check > @@ -603,6 +603,10 @@ _run_group() { > # shellcheck disable=SC1090 > . "tests/${group}/rc" > > + if declare -fF group_setup >/dev/null; then > + group_setup > + fi This new hook adds some complexity, but I can not think of better way. So, I agree to add this hook. > + > if declare -fF group_requires >/dev/null; then > group_requires > if [[ -v SKIP_REASONS ]]; then > diff --git a/tests/nvme/rc b/tests/nvme/rc > index c1ddf412033b..4465dea0370b 100644 > --- a/tests/nvme/rc > +++ b/tests/nvme/rc > @@ -23,6 +23,7 @@ _check_conflict_and_set_default NVME_IMG_SIZE nvme_img_size 1G > _check_conflict_and_set_default NVME_NUM_ITER nvme_num_iter 1000 > nvmet_blkdev_type=${nvmet_blkdev_type:-"device"} > NVMET_BLKDEV_TYPES=${NVMET_BLKDEV_TYPES:-"device file"} > +nvme_target_control="${NVME_TARGET_CONTROL:-}" > > _NVMET_TRTYPES_is_valid() { > local type > @@ -135,6 +136,13 @@ _nvme_requires() { > return 0 > } > > +group_setup() { > + if [[ -n "${nvme_target_control}" ]]; then > + NVMET_TRTYPES="$(${nvme_target_control} config --show-trtype)" > + NVMET_BLKDEV_TYPES="$(${nvme_target_control} config --show-blkdev-type)" > + fi > +} > + > group_requires() { > _have_root > _NVMET_TRTYPES_is_valid > @@ -359,6 +367,10 @@ _cleanup_nvmet() { > fi > done > > + if [[ -n "${nvme_target_control}" ]]; then > + return > + fi > + > for port in "${NVMET_CFS}"/ports/*; do > name=$(basename "${port}") > echo "WARNING: Test did not clean up port: ${name}" > @@ -403,11 +415,26 @@ _cleanup_nvmet() { > > _setup_nvmet() { > _register_test_cleanup _cleanup_nvmet > + > + if [[ -n "${nvme_target_control}" ]]; then > + def_hostnqn="$(${nvme_target_control} config --show-hostnqn)" > + def_hostid="$(${nvme_target_control} config --show-hostid)" > + def_host_traddr="$(${nvme_target_control} config --show-host-traddr)" > + def_traddr="$(${nvme_target_control} config --show-traddr)" > + def_trsvcid="$(${nvme_target_control} config --show-trsvid)" > + def_subsys_uuid="$(${nvme_target_control} config --show-subsys-uuid)" > + def_subsysnqn="$(${nvme_target_control} config --show-subsysnqn)" I guess that the lines above caused unpredictable values in $def_*, then caused many of the shellcheck warnings in tests/nvme/*. I'm afraid that another commit will be required to modify tests/nvme/* and address the warnings. > + return > + fi > + > modprobe -q nvmet > + > if [[ "${nvme_trtype}" != "loop" ]]; then > modprobe -q nvmet-"${nvme_trtype}" > fi > + > modprobe -q nvme-"${nvme_trtype}" > + > if [[ "${nvme_trtype}" == "rdma" ]]; then > start_soft_rdma > for i in $(rdma_network_interfaces) > @@ -425,6 +452,7 @@ _setup_nvmet() { > fi > done > fi > + > if [[ "${nvme_trtype}" = "fc" ]]; then > modprobe -q nvme-fcloop > _setup_fcloop "${def_local_wwnn}" "${def_local_wwpn}" \ > @@ -873,11 +901,13 @@ _find_nvme_passthru_loop_dev() { > > _nvmet_target_setup() { > local blkdev_type="${nvmet_blkdev_type}" > + local subsys_uuid="${def_subsys_uuid}" > + local subsysnqn="${def_subsysnqn}" > local blkdev > + local ARGS=() > local ctrlkey="" > local hostkey="" > - local subsysnqn="${def_subsysnqn}" > - local subsys_uuid="${def_subsys_uuid}" > + local blkdev > local port > > while [[ $# -gt 0 ]]; do > @@ -909,6 +939,22 @@ _nvmet_target_setup() { > esac > done > > + if [[ -n "${hostkey}" ]]; then > + ARGS+=(--hostkey "${hostkey}") > + fi > + if [[ -n "${ctrlkey}" ]]; then > + ARGS+=(--ctrkey "${ctrlkey}") > + fi > + > + if [[ -n "${nvme_target_control}" ]]; then > + eval "${nvme_target_control}" setup \ > + --subsysnqn "${subsysnqn}" \ > + --subsys-uuid "${subsys_uuid}" \ > + --hostnqn "${def_hostnqn}" \ > + "${ARGS[@]}" &> /dev/null I suggest to replaces ${ARGS[@]} with ${ARGS[*]}. It avoids one of the shellcheck warnings, hopefully. > + return > + fi > + > truncate -s "${NVME_IMG_SIZE}" "$(_nvme_def_file_path)" > if [[ "${blkdev_type}" == "device" ]]; then > blkdev="$(losetup -f --show "$(_nvme_def_file_path)")" > @@ -948,6 +994,13 @@ _nvmet_target_cleanup() { > esac > done > > + if [[ -n "${nvme_target_control}" ]]; then > + eval "${nvme_target_control}" cleanup \ > + --subsysnqn "${subsysnqn}" \ > + > /dev/null > + return > + fi > + > _get_nvmet_ports "${subsysnqn}" ports > > for port in "${ports[@]}"; do > -- > 2.45.2 >
