Re: [PATCH blktests 0/7] Further stacked device atomic writes testing

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

On 16/09/2025 09:55, Shinichiro Kawasaki wrote:
> On Sep 12, 2025 / 09:57, John Garry wrote:
>> The testing of atomic writes support for stacked devices is limited.
>>
>> We only test scsi_debug and for a limited sets of personalities.
>>
>> Extend to test NVMe and also extend to the following stacked device
>> personalities:
>> - dm-linear
>> - dm-stripe
>> - dm-mirror
>>
>> Also add more strict atomic writes limits testing.
>>
>> John Garry (7):
>>    common/rc: add _min()
>>    md/rc: add _md_atomics_test
>>    md/002: convert to use _md_atomics_test
>>    md/003: add NVMe atomic write tests for stacked devices
> 
> Hello John, thanks for this series. Overall, this series looks valuable for me
> since it expands the test contents and target devices. Also it minimizes code
> duplication, which is good.

thanks for checking

> 
> 
> Having said that, I noticed a challenge in the series, especially in the 4th
> patch "md/003: add NVMe atomic write tests for stacked devices". This patch
> introduces a new test case md/003 that uses four NVME devices. Actually, this is
> the very first test case which runs test for multiple devices that users define
> in the TEST_DEVS variable.
> 
> Currently, blktests expects that each test case,
> 
>   a) implements test() which prepares test target device/s in it and test the
>      device/s, or,
>   b) implements test_device() which tests single TEST_DEV taken from
>      TEST_DEVS that the user prepared
> 
> The test case md/003 tests multiple devices. This is beyond the current blktests
> assumption. md/003 implements test(), and it refers to TEST_DEVS. It looks
> working, but it breaks the expectation above. 

Sure, I do think that the current infrastructure cannot handle what I 
want to do. I want to test multiple specified devices in tandem. md/002 
does not have such an problem, as it creates the devices itself (and so 
can specify test()).

> I concern this will confuse users.

understood

> For example, when user defines 4 NVME devices in TEST_DEVS, md/003 is run only
> once, but other test cases are run 4 times.

Yes

> It also will confuse blktests test
> case developers, since it is not guided to refer to TEST_DEVS from test(): e.g.,
> ./new script. So I think a different approach is required to meet your goal.

ok

JFYI, I had been using QEMU to test this with virtual NVMe devices. This 
allows me to manually set the atomic properties of the devices for good 
test coverage.

> 
> 
> I can think of two approaches. The first one is to follow the guide a) above.
> Assuming nvme loop devices can be used for the atomic test, 

I am not sure if they are, as I don't think that any such device will 
support atomics. I did already consider this.

> md/003 can prepare 4
> nvme loop devices and use it for test. This meets the expectation. This also
> will allow to run the test case where NVME devices are not available.
> 
>    Q: Can we use nvme loop devices for the atomic test?

As above, unfortunately I don't think so.

Indeed, the test really only tests NVMe device queue limits and not 
really the atomic behaviour itself. If there was a way to configure the 
atomics-related queue limits for nvmet, then it could work, but I don't 
think that there are. Indeed, it does not really make sense for these to 
be configured manually, as they are real HW device properties.

> 
> If nvme loop devices can not be used for the atomic test, or if you prefer to
> run the test for the real NVME devices, I think it's the better to improve the
> blktests framework to support using multiple devices for a single test case. I
> think new variables and functions should be introduced to support it, to avoid
> the confusions that I noted above. For example, the test case should implement
> the test in test_device_array() instead of test(), and it should refer to
> TEST_DEV_ARRAY that users define instead of TEST_DEVS.

sounds reasonable

 > > Based on the second approach, I quickly prototyped the blktests 
change [1]. I
> also modified md/003 to adapt to the change [2].
> 
> [1] https://urldefense.com/v3/__https://github.com/kawasaki/blktests/commit/7db35a16d7410cae728da8d6b9b2483e33e9c99b__;!!ACWV5N9M2RV99hQ!Lm9AlQ3T9qSGDEjCR0nGmjCGC_2wfuAbkP6zN9EfZD7L2Y7mgpKPah_fYZh6L_OPkH9IIxP4f9n1Q9dRRRJxbZQeqRtr$
> [2] https://urldefense.com/v3/__https://github.com/kawasaki/blktests/commit/278e6c74deba68e3044abf0e6c3ec350c0bc4a40__;!!ACWV5N9M2RV99hQ!Lm9AlQ3T9qSGDEjCR0nGmjCGC_2wfuAbkP6zN9EfZD7L2Y7mgpKPah_fYZh6L_OPkH9IIxP4f9n1Q9dRRRJxbSlcsw0E$
> 
> Please let me know your thoughts on the two approaches.

Let me check it, thanks!

> 
> 
> P.S. I will have some more comments on the details of the series, but before
>       making those comments, I would like to clarify how to resolve the challenge
>       above.

ok, good.

Cheers,
John