On Jun 07, 2023 / 03:35, Chaitanya Kulkarni wrote:
> 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@xxxxxxxxxxx>
> >>>>>
> >>>>> 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,
Okay, I have applied Yang's patch (Thanks Yang!). And I also added a follow-up
commit to remove the helper fucntion _filter_discovery, which is no longer used.
> also it will be great if
> can we can drop any other such checks in the nvme test category to save
> everyone's time.
Another commit 93ae757d4ca2("nvme/{002,030}: Move discovery generation counter
code to rc") removed the check from nvme/002. So, there is no check for
Discovery Log Page outputs in the nvme group at this moment. As far as I see,
there seems no other Log Page checks.
