Re: [PATCH blktests] nvme/{016,017}: use _check_genctr instead of _filter_discovery

[Chaitanya Kulkarni <chaitanyak@nvidia.com>]

On 6/6/2023 7:14 PM, Shinichiro Kawasaki wrote:
> On Jun 06, 2023 / 09:45, Sagi Grimberg wrote:
>>
>>>>> On May 31, 2023 / 09:07, Yang Xu wrote:
>>>>>> Since commit 328943e3 ("Update tests for discovery log page changes"),
>>>>>> blktests also include the discovery subsystem itself. But it
>>>>>> will lead these cases fails on older nvme-cli system.
>>>>>
>>>>> Thanks for this report. What is the nvme-cli version with the issue?
>>>>>
>>>>>>
>>>>>> To avoid this, like nvme/002, use _check_genctr to check instead of
>>>>>> comparing many discovery Log Entry output.
>>>>>>
>>>>>> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
>>>>>
>>>>> The change looks fine to me, but I'd wait for comments by nvme
>>>>> developers.
>>>>
>>>> I'm ok with this change, but IIRC Chaitanya wanted that we keep checking
>>>> the full log-page output...
>>>
>>> the original testcase was designed to validate the log page internals
>>> and  that correctness cannot be established without looking into the log
>>> page.
> 
> I think nvme/016 and 017 still have value even without the log-page output
> checks. They exercise namespace creations and deletions which other test
> cases don't.
> 
>>>
>>> but given that how much churn this is generating eveytime something
>>> changes in nvme-cli or in kernel implementation I'm really wondering is
>>> that worth everyone's time ?
>>>
>>> Sagi/Shinichiro any thoughts ?
>>
>> Also back then I thought it'd create churn because the log page output
>> is not an interface.
> 
> So, we should drop the log page output check, and it means that Yang's patch is
> the choice.
> 
> Chaitanya, is it ok for you?

I think it is reasonable to drop this check, also it will be great if
can we can drop any other such checks in the nvme test category to save
everyone's time.

-ck