Re: [PATCH blktests 4/7] md/003: add NVMe atomic write tests for stacked devices

[John Garry <john.g.garry@oracle.com>]

On 18/09/2025 05:27, Shinichiro Kawasaki wrote:
> On Sep 12, 2025 / 09:57, John Garry wrote:
>> md/002 only tests SCSI via scsi_debug.
>>
>> It is also useful to test NVMe, so add a specific test for that.
>>
>> The results for 002 and 003 should be the same, so link them.
>>
>> _md_atomics_test requires 4x devices with atomics support, so check for
>> that.
>>
>> Signed-off-by: John Garry <john.g.garry@oracle.com>
> [...]
>> diff --git a/tests/md/003 b/tests/md/003
>> new file mode 100755
>> index 0000000..8128f8d
>> --- /dev/null
>> +++ b/tests/md/003
>> @@ -0,0 +1,51 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2025 Oracle and/or its affiliates
>> +#
>> +# Test NMVe Atomic Writes with MD devices
>> +
>> +. tests/nvme/rc
> 
> It is not recommended to introduce dependencies across tests/* groups. If you
> need some nvme related helper functions, they should be placed not in
> tests/nvme/rc but in common/nvme.
> 
> IIUC, tests/nvme/rc is required to call _nvme_requries() in requries(), but I
> think _nvme_requires() is too much for this test case. 

I thought that we would need _nvme_requries() for ensuring that we have 
the appropriate driver, i.e. the nvme core driver

> I gueess it is enough to
> call _require_test_dev_is_nvme() from device_requires() in md/003.
> To do that, I
> suggest to add another preparation patch which moves _require_test_dev_is_nvme()
> from tests/nvme/rc to common/nvme. (This comment assumes the test_device_array()
> support series.)

ok

> 
>> +. common/xfs
>> +
>> +DESCRIPTION="test md atomic writes for NVMe drives"
>> +QUICK=1
>> +
>> +requires() {
>> +	_nvme_requires
>> +}
>> +
>> +test() {
>> +	local ns
>> +	local testdev_count=0
>> +	declare -A NVME_TEST_DEVS
>> +	declare -A NVME_TEST_DEVS_NAME
>> +	declare -A NVME_TEST_DEVS_SYSFS
>> +
>> +	echo "Running md_atomics_test"
>> +
>> +	for i in "${!TEST_DEV_SYSFS_DIRS[@]}"; do
>> +		TEST_DEV=${TEST_DEV_SYSFS_DIRS[$i]}
>> +		if readlink -f "$TEST_DEV" | grep -q nvme; then
> 
> If _require_test_dev_is_nvme() is called from device_requires(), the check
> above will not be required.

ok, sure

thanks,
John