| Message ID | 20200930185422.11494-3-logang@deltatee.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | NVMe Target Passthru Block Tests | expand |
On 9/30/20 11:54, Logan Gunthorpe wrote: > > requires() { > _nvme_requires > - _have_program mkfs.xfs && _have_fio > + _have_xfs > + _have_fio Can you make _have_xfs return true false ? so it can be used with && ?
On 2020-10-06 5:44 p.m., Chaitanya Kulkarni wrote: > On 9/30/20 11:54, Logan Gunthorpe wrote: >> >> requires() { >> _nvme_requires >> - _have_program mkfs.xfs && _have_fio >> + _have_xfs >> + _have_fio > Can you make _have_xfs return true false ? so it can be used with && ? _have_xfs() does return true/false and can be used with && or in a conditional. Per [1], my opinion is that using && in the requires() function where the return value is ignored is confusing so I prefer not to do it in new code. If we want to reconsider this we, should add a check to ensure the return value of requires() matches the expectation of the global variable it uses. Logan [1] https://lore.kernel.org/linux-block/92478e6f-622a-a1ae-6189-4009f9a307bc@deltatee.com/
On 10/6/20 16:51, Logan Gunthorpe wrote: > _have_xfs() does return true/false and can be used with && or in a > conditional. > > Per [1], my opinion is that using && in the requires() function where > the return value is ignored is confusing so I prefer not to do it in new > code. > > If we want to reconsider this we, should add a check to ensure the > return value of requires() matches the expectation of the global > variable it uses. > > Logan > > [1] > https://lore.kernel.org/linux-block/92478e6f-622a-a1ae-6189-4009f9a307bc@deltatee.com/ Make sense to me, lets not change this, thanks for pointing that out.
diff --git a/common/rc b/common/rc index cdc0150ea5ea..44cb218c2fac 100644 --- a/common/rc +++ b/common/rc @@ -181,6 +181,14 @@ _have_tracepoint() { return 0 } +_have_fs() { + modprobe "$1" >/dev/null 2>&1 + if [[ ! -d "/sys/fs/$1" ]]; then + SKIP_REASON="kernel does not support filesystem $1" + return 1 + fi +} + _test_dev_is_rotational() { [[ $(cat "${TEST_DEV_SYSFS}/queue/rotational") -ne 0 ]] } diff --git a/common/xfs b/common/xfs new file mode 100644 index 000000000000..d1a603b8c7b5 --- /dev/null +++ b/common/xfs @@ -0,0 +1,11 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-3.0+ +# Copyright (C) 2017 Omar Sandoval +# +# fio helper functions. + +. common/shellcheck + +_have_xfs() { + _have_fs xfs && _have_program mkfs.xfs +} diff --git a/tests/nvme/012 b/tests/nvme/012 index 1dbd59804ed7..1d8d8e3cc271 100755 --- a/tests/nvme/012 +++ b/tests/nvme/012 @@ -5,14 +5,16 @@ # Test mkfs with data verification for block device backed ns. . tests/nvme/rc +. common/xfs DESCRIPTION="run mkfs and data verification fio job on NVMeOF block device-backed ns" TIMED=1 requires() { _nvme_requires - _have_program mkfs.xfs && _have_program fio && \ - _have_modules loop + _have_xfs + _have_program fio + _have_modules loop _require_nvme_trtype_is_fabrics } diff --git a/tests/nvme/013 b/tests/nvme/013 index df7f23e69252..3819a2730d9b 100755 --- a/tests/nvme/013 +++ b/tests/nvme/013 @@ -5,13 +5,15 @@ # Test mkfs with data verification for file backed ns. . tests/nvme/rc +. common/xfs DESCRIPTION="run mkfs and data verification fio job on NVMeOF file-backed ns" TIMED=1 requires() { _nvme_requires - _have_program mkfs.xfs && _have_fio + _have_xfs + _have_fio _require_nvme_trtype_is_fabrics }
Two nvme tests create and mount XFS filesystems and check for mkfs.xfs. They should also check for XFS support in the kernel so create a common helper for this. Signed-off-by: Logan Gunthorpe <logang@deltatee.com> --- common/rc | 8 ++++++++ common/xfs | 11 +++++++++++ tests/nvme/012 | 6 ++++-- tests/nvme/013 | 4 +++- 4 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 common/xfs