[PATCH blktests v3 1/3] nvme/rc: introduce remote target support
Shinichiro Kawasaki
shinichiro.kawasaki at wdc.com
Tue Jul 2 02:31:31 PDT 2024
On Jul 02, 2024 / 17:51, Shin'ichiro Kawasaki wrote:
> CC+: Ofir
Sorry, I forgot to add Ofir to the CC list. Let me resend this note.
>
> 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@volumez.com/
>
>
> 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 at suse.de>
>
> 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
> >
More information about the Linux-nvme
mailing list