enable bs > ps device testing
v3 cover enable bs > ps device testing
This v3 series addresses the feedback from the v2 series [0] and makes some minor new changes, the change are:
- Fixes all shellcheck complaints
- Addresses spacing / tabs fixes
- Adds _test_dev_suits_xfs() as suggested by Shinichiro and makes tests which require this depend on it
- Clamps _min_io() to 4k as well for backward compatibility
- Few minor enhancements to help capture up error messages from mkfs from block/032
This goes tested against a 64k sector size NVMe drive, and also using ./check so regular loopback devices are used. This helps test 64k sector devices, patches for which have been posted [1].
[0] https://lkml.kernel.org/r/[email protected]
[1] https://lkml.kernel.org/r/[email protected]
Luis Chamberlain (6):
common/xfs: ignore first umount error on _xfs_mkfs_and_mount() block/032: make error messages clearer if mkfs or mount fails
common: add and use min io for fio
common/xfs: use min io for fs blocksize
tests: use test device min io to support bs > ps common/xfs: add _test_dev_suits_xfs() to verify logical block size
will work
common/fio | 26 ++++++++++++++++++++++++-- common/rc | 24 ++++++++++++++++++++++++ common/xfs | 23 +++++++++++++++++++++-- tests/block/003 | 6 +++++- tests/block/007 | 5 ++++- tests/block/032 | 7 ++++--- tests/nvme/012 | 1 + tests/nvme/035 | 1 + tests/nvme/049 | 15 +++++++++++---- 9 files changed, 95 insertions(+), 13 deletions(-)
Signature · 1 line
2.45.2
PATCH v3 1/6 common/xfs: ignore first umount error on _xfs_mkfs_and_mount()
We want to help capture error messages with _xfs_mkfs_and_mount() on $FULL, to do that we should avoid spamming error messages for things which we know are not fatal. Such is the case of when we try to mkfs a filesystem but before that try to umount the target path. The first umount is just for sanity, so ignore the error messages from it.
--- common/xfs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
common/xfs +1 -1
@@ -15,7 +15,7 @@ _xfs_mkfs_and_mount() { local mount_dir=$2 mkdir -p "${mount_dir}" - umount "${mount_dir}" + umount "${mount_dir}" >/dev/null 2>&1 mkfs.xfs -l size=64m -f "${bdev}" || return $? mount "${bdev}" "${mount_dir}" }
Signature · 1 line
2.45.2
Re: [PATCH blktests v3 1/6] common/xfs: ignore first umount error on _xfs_mkfs_and_mount()
Quoted 17 lines · Luis Chamberlain
We want to help capture error messages with
_xfs_mkfs_and_mount()on $FULL, to do that we should avoid spamming error messages for things which we know are not fatal. Such is the case of when we try to mkfs a filesystem but before that try to umount the target path. The first umount is just for sanity, so ignore the error messages from it.Signed-off-by Luis Chamberlain <[email protected]>--- common/xfs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)common/xfs +0
-0
common/xfs _xfs_mkfs_and_mount() { · line 21
Quoted 8 lines
common/xfs +1
-1@@ -15,7 +15,7 @@ _xfs_mkfs_and_mount() { local mount_dir=$2 mkdir -p "${mount_dir}" - umount "${mount_dir}" + umount "${mount_dir}" >/dev/null 2>&1 mkfs.xfs -l size=64m -f "${bdev}" || return $? mount "${bdev}" "${mount_dir}" }
Shouldn't ">/dev/null 2>&1" be added to the _xfs_mkfs_and_mount() caller rather than inside the _xfs_mkfs_and_mount() implementation? The above patch makes it impossible for any caller to capture the stdout output of umount.
Thanks,
Bart.
Re: [PATCH blktests v3 1/6] common/xfs: ignore first umount error on _xfs_mkfs_and_mount()
Quoted 22 lines · Bart Van Assche
Quoted 21 lines · Luis Chamberlain
We want to help capture error messages with
_xfs_mkfs_and_mount()on $FULL, to do that we should avoid spamming error messages for things which we know are not fatal. Such is the case of when we try to mkfs a filesystem but before that try to umount the target path. The first umount is just for sanity, so ignore the error messages from it.Signed-off-by Luis Chamberlain <[email protected]>--- common/xfs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)common/xfs +1
-1@@ -15,7 +15,7 @@ _xfs_mkfs_and_mount() { local mount_dir=$2 mkdir -p "${mount_dir}" - umount "${mount_dir}" + umount "${mount_dir}" >/dev/null 2>&1
Quoted 8 lines
common/xfs _xfs_mkfs_and_mount() { · line 21
Quoted 3 lines
common/xfs +0
-0mkfs.xfs -l size=64m -f "${bdev}" || return $? mount "${bdev}" "${mount_dir}" }Shouldn't ">/dev/null 2>&1" be added to the
_xfs_mkfs_and_mount()caller rather than inside the_xfs_mkfs_and_mount()implementation? The above patch makes it impossible for any caller to capture the stdout output of umount.
That is the point. In this case the umount *can* fail, we do it for sanity purposes. We don't care if umount failed.
Luis
Re: [PATCH blktests v3 1/6] common/xfs: ignore first umount error on _xfs_mkfs_and_mount()
Quoted 32 lines · Luis Chamberlain
Quoted 22 lines · Bart Van Assche
Quoted 21 lines · Luis Chamberlain
We want to help capture error messages with
_xfs_mkfs_and_mount()on $FULL, to do that we should avoid spamming error messages for things which we know are not fatal. Such is the case of when we try to mkfs a filesystem but before that try to umount the target path. The first umount is just for sanity, so ignore the error messages from it.Signed-off-by Luis Chamberlain <[email protected]>--- common/xfs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)common/xfs +1
-1@@ -15,7 +15,7 @@ _xfs_mkfs_and_mount() { local mount_dir=$2 mkdir -p "${mount_dir}" - umount "${mount_dir}" + umount "${mount_dir}" >/dev/null 2>&1Quoted 8 lines
common/xfs _xfs_mkfs_and_mount() { · line 21
Quoted 3 lines
common/xfs +0
-0mkfs.xfs -l size=64m -f "${bdev}" || return $? mount "${bdev}" "${mount_dir}" }Shouldn't ">/dev/null 2>&1" be added to the
_xfs_mkfs_and_mount()caller rather than inside the_xfs_mkfs_and_mount()implementation? The above patch makes it impossible for any caller to capture the stdout output of umount.
Quoted 4 lines
That is the point. In this case the umount *can* fail, we do it for sanity purposes. We don't care if umount failed.
Luis
IMO, umount --quiet option will help instead of redirect to /dev/null.
In most cases, the umount command just ensures that nothing is mounted at ${mount_dir}, and prints the "not mounted" error message. The --quiet option will suppress it, and I guess it will address the problem Luis faces. With this, still we can keep other error messages printed when anything unexpected happens.
PATCH v3 2/6 block/032: make error messages clearer if mkfs or mount fails
In reply to Luis ChamberlainIf block/032 fails at mkfs we want to know why, so propagate error messages. While at it, enhance the test to also propagate the error return from mount and remove the odd sleep for a udevadm settle as that's the only thing I can think of we need to wait for here.
--- tests/block/032 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
tests/block/032 +3 -3
@@ -25,10 +25,10 @@ test() { fi mkdir -p "${TMPDIR}/mnt" - _xfs_mkfs_and_mount "/dev/${SCSI_DEBUG_DEVICES[0]}" "${TMPDIR}/mnt" > /dev/null 2>&1 + _xfs_mkfs_and_mount "/dev/${SCSI_DEBUG_DEVICES[0]}" "${TMPDIR}/mnt" >> $FULL || return $? echo 1 > "/sys/block/${SCSI_DEBUG_DEVICES[0]}/device/delete" - sleep 2 - umount "${TMPDIR}/mnt" + udevadm settle + umount "${TMPDIR}/mnt" || return $? _exit_scsi_debug
Signature · 1 line
2.45.2
Re: [PATCH blktests v3 2/6] block/032: make error messages clearer if mkfs or mount fails
Quoted 16 lines · Luis Chamberlain
If block/032 fails at mkfs we want to know why, so propagate error messages. While at it, enhance the test to also propagate the error return from mount and remove the odd sleep for a udevadm settle as that's the only thing I can think of we need to wait for here.
Signed-off-by Luis Chamberlain <[email protected]>--- tests/block/032 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)tests/block/032 +0
-0
tests/block/032 test() { · line 28
Quoted 5 lines
tests/block/032 +1
-1@@ -25,10 +25,10 @@ test() { fi mkdir -p "${TMPDIR}/mnt" - _xfs_mkfs_and_mount "/dev/${SCSI_DEBUG_DEVICES[0]}" "${TMPDIR}/mnt" > /dev/null 2>&1 + _xfs_mkfs_and_mount "/dev/${SCSI_DEBUG_DEVICES[0]}" "${TMPDIR}/mnt" >> $FULL || return $?
The $FULL needs double quotations to avoid a shellcheck warning. That change was in the 6th patch, but let's fix it here.
When _xfs_mkfs_and_mount() fails, I think it is the better to call _exit_scsi_debug() before returning from test(). It will clean up the scsi_debug device, then will reduce the impact on following test cases.
Quoted 11 lines
tests/block/032 +2
-2echo 1 > "/sys/block/${SCSI_DEBUG_DEVICES[0]}/device/delete" - sleep 2 - umount "${TMPDIR}/mnt" + udevadm settle + umount "${TMPDIR}/mnt" || return $? _exit_scsi_debugSignature · 1 line
2.45.2
PATCH v3 3/6 common: add and use min io for fio
In reply to Luis ChamberlainWhen using fio we should not issue IOs smaller than the device supports. Today a lot of places have in place 4k, but soon we will have devices which support bs > ps. For those devices we should check the minimum supported IO.
However, since we also have a min optimal IO, we might as well use that as well. By using this we can also leverage the same lookup with stat whether or not the target file is a block device or a file.
--- common/fio | 26 ++++++++++++++++++++++++-- common/rc | 24 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-)
common/fio +24 -2
@@ -189,15 +189,37 @@ _run_fio() { return $rc } +_fio_opts_to_min_io() { + local arg path + local -i min_io=4096 + + for arg in "$@"; do + [[ "$arg" =~ ^--filename= || "$arg" =~ --directory= ]] || continue + path="${arg##*=}" + min_io=$(_min_io "$path") + # Keep 4K minimum IO size for historical consistency + ((min_io < 4096)) && min_io=4096 + break + done + + echo "$min_io" +} + # Wrapper around _run_fio used if you need some I/O but don't really care much # about the details _run_fio_rand_io() { - _run_fio --bs=4k --rw=randread --norandommap --numjobs="$(nproc)" \ + local bs + + bs=$(_fio_opts_to_min_io "$@") || return 1 + _run_fio --bs="$bs" --rw=randread --norandommap --numjobs="$(nproc)" \ --name=reads --direct=1 "$@" } _run_fio_verify_io() { - _run_fio --name=verify --rw=randwrite --direct=1 --ioengine=libaio --bs=4k \ + local bs + + bs=$(_fio_opts_to_min_io "$@") || return 1 + _run_fio --name=verify --rw=randwrite --direct=1 --ioengine=libaio --bs="$bs" \ --iodepth=16 --verify=crc32c --verify_state_save=0 "$@" }
common/rc +24 -0
@@ -400,6 +400,30 @@ _test_dev_is_partition() { [[ -n ${TEST_DEV_PART_SYSFS} ]] } +_min_io() { + local path_or_dev=$1 + local min_io=4096 + if [ -z "$path_or_dev" ]; then + echo "path for min_io does not exist" + return 1 + fi + + if [ -c "$path_or_dev" ]; then + if [[ "$path_or_dev" == /dev/ng* ]]; then + path_or_dev="${path_or_dev/ng/nvme}" + fi + fi + + if [ -e "$path_or_dev" ]; then + min_io=$(stat --printf=%o "$path_or_dev") + ((min_io < 4096)) && min_io=4096 + echo "$min_io" + else + echo "Error: '$path_or_dev' does not exist or is not accessible" + return 1 + fi +} + # Return max open zones or max active zones of the test target device. # If the device has both, return smaller value. _test_dev_max_open_active_zones() {
Signature · 1 line
2.45.2
Re: [PATCH blktests v3 3/6] common: add and use min io for fio
Quoted 20 lines · Luis Chamberlain
When using fio we should not issue IOs smaller than the device supports. Today a lot of places have in place 4k, but soon we will have devices which support bs > ps. For those devices we should check the minimum supported IO.
However, since we also have a min optimal IO, we might as well use that as well. By using this we can also leverage the same lookup with stat whether or not the target file is a block device or a file.
Signed-off-by Luis Chamberlain <[email protected]>--- common/fio | 26 ++++++++++++++++++++++++-- common/rc | 24 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-)common/fio +0
-0
common/fio _run_fio() { · line 196
Quoted 8 lines
common/fio +5
-0@@ -189,15 +189,37 @@ _run_fio() { return $rc } +_fio_opts_to_min_io() { + local arg path + local -i min_io=4096 + + for arg in "$@"; do
Still spaces are left in the linve above.
common/fio _run_fio() { · line 201
Quoted 5 lines
common/fio +5
-0+ [[ "$arg" =~ ^--filename= || "$arg" =~ --directory= ]] || continue + path="${arg##*=}" + min_io=$(_min_io "$path") + # Keep 4K minimum IO size for historical consistency + ((min_io < 4096)) && min_io=4096
I think the 4k comparison above is no longer required, since you added it in _min_io().
Quoted 42 lines
common/fio +14
-2+ break + done + + echo "$min_io" +} + # Wrapper around _run_fio used if you need some I/O but don't really care much # about the details _run_fio_rand_io() { - _run_fio --bs=4k --rw=randread --norandommap --numjobs="$(nproc)" \ + local bs + + bs=$(_fio_opts_to_min_io "$@") || return 1 + _run_fio --bs="$bs" --rw=randread --norandommap --numjobs="$(nproc)" \ --name=reads --direct=1 "$@" } _run_fio_verify_io() { - _run_fio --name=verify --rw=randwrite --direct=1 --ioengine=libaio --bs=4k \ + local bs + + bs=$(_fio_opts_to_min_io "$@") || return 1 + _run_fio --name=verify --rw=randwrite --direct=1 --ioengine=libaio --bs="$bs" \ --iodepth=16 --verify=crc32c --verify_state_save=0 "$@" }common/rc +8
-0@@ -400,6 +400,30 @@ _test_dev_is_partition() { [[ -n ${TEST_DEV_PART_SYSFS} ]] } +_min_io() { + local path_or_dev=$1 + local min_io=4096 + if [ -z "$path_or_dev" ]; then + echo "path for min_io does not exist" + return 1 + fi +
common/rc _test_dev_is_partition() { · line 420
Quoted 10 lines
common/rc +10
-0+ if [ -c "$path_or_dev" ]; then + if [[ "$path_or_dev" == /dev/ng* ]]; then + path_or_dev="${path_or_dev/ng/nvme}" + fi + fi + + if [ -e "$path_or_dev" ]; then + min_io=$(stat --printf=%o "$path_or_dev") + ((min_io < 4096)) && min_io=4096 + echo "$min_io"
Here's the added check, so _min_io() return value is always 4k or larger, isn't it?
Quoted 12 lines
common/rc +6
-0+ else + echo "Error: '$path_or_dev' does not exist or is not accessible" + return 1 + fi +} + # Return max open zones or max active zones of the test target device. # If the device has both, return smaller value. _test_dev_max_open_active_zones() {Signature · 1 line
2.45.2
PATCH v3 4/6 common/xfs: use min io for fs blocksize
In reply to Luis ChamberlainUse the min io for the target block size. Likewise we need to increase the log size if using a bs > 4096.
--- common/xfs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
common/xfs +9 -1
@@ -13,10 +13,18 @@ _have_xfs() { _xfs_mkfs_and_mount() { local bdev=$1 local mount_dir=$2 + local bs + local xfs_logsize="64m" + + bs=$(_min_io "$bdev") + + if [[ $bs -gt 4096 ]]; then + xfs_logsize="128m" + fi mkdir -p "${mount_dir}" umount "${mount_dir}" >/dev/null 2>&1 - mkfs.xfs -l size=64m -f "${bdev}" || return $? + mkfs.xfs -l size=$xfs_logsize -f "${bdev}" -b size="$bs" || return $? mount "${bdev}" "${mount_dir}" }
Signature · 1 line
2.45.2
PATCH v3 5/6 tests: use test device min io to support bs > ps
In reply to Luis ChamberlainWhen a block device supports a minimum block size > ps we must ensure we don't issue IOs below what is supported. Just leverage the min optimal IO to also ensure we use the optimal IO as well.
--- tests/block/003 | 6 +++++- tests/block/007 | 5 ++++- tests/nvme/049 | 15 +++++++++++---- 3 files changed, 20 insertions(+), 6 deletions(-)
tests/block/003 +5 -1
@@ -18,10 +18,14 @@ device_requires() { } test_device() { + local test_dev_bs + + test_dev_bs=$(_min_io "$TEST_DEV") + echo "Running ${TEST_NAME}" FIO_PERF_FIELDS=("trim iops") - _fio_perf --bsrange=4k-4g --rw=randtrim --norandommap --name=discards \ + _fio_perf --bsrange="${test_dev_bs}"-4g --rw=randtrim --norandommap --name=discards \ --filename="$TEST_DEV" --number_ios=200k echo "Test complete"
tests/block/007 +4 -1
@@ -31,13 +31,16 @@ cleanup_fallback_device() { } run_fio_job() { + local test_dev_bs + + test_dev_bs=$(_min_io "$TEST_DEV") if _test_dev_is_rotational; then size="32m" else size="1g" fi - _fio_perf --bs=4k --rw=randread --norandommap --name=reads \ + _fio_perf --bs="$test_dev_bs" --rw=randread --norandommap --name=reads \ --filename="$TEST_DEV" --size="$size" --direct=1 \ --ioengine=pvsync2 --hipri="$1" }
tests/nvme/049 +11 -4
@@ -19,10 +19,16 @@ test_device() { echo "Running ${TEST_NAME}" local ngdev=${TEST_DEV/nvme/ng} - local common_args=( + local test_dev_bs + local target_size=4096 + local common_args=() + local fio_output + + test_dev_bs=$(_min_io "$ngdev") + common_args=( --size=1M --filename="$ngdev" - --bs=4k + --bs="$test_dev_bs" --rw=randread --numjobs=1 --iodepth=16
@@ -32,10 +38,11 @@ test_device() { --time_based --runtime=2 ) - local fio_output + + ((test_dev_bs > target_size)) && target_size=$test_dev_bs # check security permission - if ! fio_output=$(fio --name=check --size=4k --filename="$ngdev" \ + if ! fio_output=$(fio --name=check --bs="$test_dev_bs" --size="$target_size" --filename="$ngdev" \ --rw=read --ioengine=io_uring_cmd 2>&1) && grep -q -e "Operation not permitted" \ -e "Permission denied" <<< "$fio_output"; then
Signature · 1 line
2.45.2
PATCH v3 6/6 common/xfs: add _test_dev_suits_xfs() to verify logical block size will work
In reply to Luis Chamberlainmkfs.xfs will use the sector size exposed by the device, if this is larger than 32k this will fail as the largest sector size on XFS is 32k. Provide a sanity check to ensure we skip creating a filesystem if the sector size is larger than what XFS supports.
--- common/xfs | 11 +++++++++++ tests/block/032 | 3 ++- tests/nvme/012 | 1 + tests/nvme/035 | 1 + 4 files changed, 15 insertions(+), 1 deletion(-)
common/xfs +11 -0
@@ -10,6 +10,17 @@ _have_xfs() { _have_fs xfs && _have_program mkfs.xfs } +_test_dev_suits_xfs() { + local logical_block_size + + logical_block_size=$(_test_dev_queue_get logical_block_size) + if ((logical_block_size > 32768 )); then + SKIP_REASONS+=("sector size ${logical_block_size} is larger than max XFS sector size 32768") + return 1 + fi + return 0 +} + _xfs_mkfs_and_mount() { local bdev=$1 local mount_dir=$2
tests/block/032 +2 -1
@@ -15,6 +15,7 @@ QUICK=1 requires() { _have_xfs _have_module scsi_debug + _test_dev_suits_xfs } test() {
@@ -25,7 +26,7 @@ test() { fi mkdir -p "${TMPDIR}/mnt" - _xfs_mkfs_and_mount "/dev/${SCSI_DEBUG_DEVICES[0]}" "${TMPDIR}/mnt" >> $FULL || return $? + _xfs_mkfs_and_mount "/dev/${SCSI_DEBUG_DEVICES[0]}" "${TMPDIR}/mnt" >> "$FULL" || return $? echo 1 > "/sys/block/${SCSI_DEBUG_DEVICES[0]}/device/delete" udevadm settle umount "${TMPDIR}/mnt" || return $?
tests/nvme/012 +1 -0
@@ -17,6 +17,7 @@ requires() { _have_loop _require_nvme_trtype_is_fabrics _require_nvme_test_img_size 350m + _test_dev_suits_xfs } set_conditions() {
tests/nvme/035 +1 -0
@@ -14,6 +14,7 @@ requires() { _have_kernel_option NVME_TARGET_PASSTHRU _have_xfs _have_fio + _test_dev_suits_xfs } device_requires() {
Signature · 1 line
2.45.2
Re: [PATCH blktests v3 6/6] common/xfs: add _test_dev_suits_xfs() to verify logical block size will work
Quoted 41 lines · Luis Chamberlain
mkfs.xfs will use the sector size exposed by the device, if this is larger than 32k this will fail as the largest sector size on XFS is 32k. Provide a sanity check to ensure we skip creating a filesystem if the sector size is larger than what XFS supports.
Suggested-by Shinichiro Kawasaki <[email protected]>Signed-off-by Luis Chamberlain <[email protected]>--- common/xfs | 11 +++++++++++ tests/block/032 | 3 ++- tests/nvme/012 | 1 + tests/nvme/035 | 1 + 4 files changed, 15 insertions(+), 1 deletion(-)common/xfs +11
-0@@ -10,6 +10,17 @@ _have_xfs() { _have_fs xfs && _have_program mkfs.xfs } +_test_dev_suits_xfs() { + local logical_block_size + + logical_block_size=$(_test_dev_queue_get logical_block_size) + if ((logical_block_size > 32768 )); then + SKIP_REASONS+=("sector size ${logical_block_size} is larger than max XFS sector size 32768") + return 1 + fi + return 0 +} + _xfs_mkfs_and_mount() { local bdev=$1 local mount_dir=$2tests/block/032 +0
-0
tests/block/032 QUICK=1 · line 18
Quoted 4 lines
tests/block/032 +1
-0@@ -15,6 +15,7 @@ QUICK=1 requires() { _have_xfs _have_module scsi_debug + _test_dev_suits_xfs
I don't think this check is needed.
_test_dev_suits_xfs() calls _test_dev_queue_get(), which works only for test cases with test_device(). Then, it works for nvme/035. But does not work for either block/032 or nvme/012, which prepares test target device in test(), so they do not need the check.
Quoted 17 lines
tests/block/032 +1
-1} test() {@@ -25,7 +26,7 @@ test() { fi mkdir -p "${TMPDIR}/mnt" - _xfs_mkfs_and_mount "/dev/${SCSI_DEBUG_DEVICES[0]}" "${TMPDIR}/mnt" >> $FULL || return $? + _xfs_mkfs_and_mount "/dev/${SCSI_DEBUG_DEVICES[0]}" "${TMPDIR}/mnt" >> "$FULL" || return $? echo 1 > "/sys/block/${SCSI_DEBUG_DEVICES[0]}/device/delete" udevadm settle umount "${TMPDIR}/mnt" || return $?tests/nvme/012 +0
-0
tests/nvme/012 requires() { · line 20
Quoted 4 lines
tests/nvme/012 +1
-0@@ -17,6 +17,7 @@ requires() { _have_loop _require_nvme_trtype_is_fabrics _require_nvme_test_img_size 350m + _test_dev_suits_xfs
Same here.
tests/nvme/035 requires() { · line 17
Quoted 12 lines
tests/nvme/012 +0
-0} set_conditions() {tests/nvme/035 +1
-0@@ -14,6 +14,7 @@ requires() { _have_kernel_option NVME_TARGET_PASSTHRU _have_xfs _have_fio + _test_dev_suits_xfs
This is the requirement check for TEST_DEV, so I suggest to move it from requires() to device_requires().
Quoted 6 lines
tests/nvme/035 +0
-0} device_requires() {Signature · 1 line
2.45.2
Quoted 11 lines · Luis Chamberlain
This v3 series addresses the feedback from the v2 series [0] and makes some minor new changes, the change are:
- Fixes all shellcheck complaints
- Addresses spacing / tabs fixes
- Adds_test_dev_suits_xfs()as suggested by Shinichiro and makes tests which require this depend on it
- Clamps_min_io()to 4k as well for backward compatibility
- Few minor enhancements to help capture up error messages from mkfs from block/032
Quoted 6 lines
This goes tested against a 64k sector size NVMe drive, and also using ./check so regular loopback devices are used. This helps test 64k sector devices, patches for which have been posted [1].
[0] https://lkml.kernel.org/r/[email protected]
[1] https://lkml.kernel.org/r/[email protected]
Luis, thank you very much for the improvements. I made comments on some of the patches. FYI, I reflected my comments on your patches, and pushed them to a github repo branch [2]. Please take a look in them and see if my comments make sense or not.
Quoted 19 lines · Shinichiro Kawasaki
Quoted 11 lines · Luis Chamberlain
This v3 series addresses the feedback from the v2 series [0] and makes some minor new changes, the change are:
- Fixes all shellcheck complaints
- Addresses spacing / tabs fixes
- Adds_test_dev_suits_xfs()as suggested by Shinichiro and makes tests which require this depend on it
- Clamps_min_io()to 4k as well for backward compatibility
- Few minor enhancements to help capture up error messages from mkfs from block/032Quoted 6 lines
This goes tested against a 64k sector size NVMe drive, and also using ./check so regular loopback devices are used. This helps test 64k sector devices, patches for which have been posted [1].
[0] https://lkml.kernel.org/r/[email protected]
[1] https://lkml.kernel.org/r/[email protected]
Quoted 6 lines
Luis, thank you very much for the improvements. I made comments on some of the patches. FYI, I reflected my comments on your patches, and pushed them to a github repo branch [2]. Please take a look in them and see if my comments make sense or not.
Looks good, thanks for doing this, passes all my tests too, please feel free to merge :)
Luis
Quoted 25 lines · Luis Chamberlain
Quoted 24 lines · Shinichiro Kawasaki
Quoted 11 lines · Luis Chamberlain
This v3 series addresses the feedback from the v2 series [0] and makes some minor new changes, the change are:
- Fixes all shellcheck complaints
- Addresses spacing / tabs fixes
- Adds_test_dev_suits_xfs()as suggested by Shinichiro and makes tests which require this depend on it
- Clamps_min_io()to 4k as well for backward compatibility
- Few minor enhancements to help capture up error messages from mkfs from block/032Quoted 6 lines
This goes tested against a 64k sector size NVMe drive, and also using ./check so regular loopback devices are used. This helps test 64k sector devices, patches for which have been posted [1].
[0] https://lkml.kernel.org/r/[email protected]
[1] https://lkml.kernel.org/r/[email protected]Luis, thank you very much for the improvements. I made comments on some of the patches. FYI, I reflected my comments on your patches, and pushed them to a github repo branch [2]. Please take a look in them and see if my comments make sense or not.
Quoted 4 lines
Quoted 1 line
Looks good, thanks for doing this, passes all my tests too, please feel free to merge :)
Thanks for the confirmation.
FYI, I applied the series with the modifications.