On Jun 06, 2023 / 03:20, Chaitanya Kulkarni wrote:
> On 6/5/2023 4:18 PM, 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.
>
> 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 ?
I don't have future view about the stability of the log page. I would like to
hear call by Sagi and/or nvme developers about it. If we expect more log page
changes, Yang's change in blktests sounds reasonable.
If we expect no more log page changes in the future, we can think of another
solution: skip the test cases on older kernel (or nvme-cli). I think the
blktests commit 328943e3 ("Update tests for discovery log page changes")
corresponds to the kernel commit e5ea42faa773 ("nvme: display correct subsystem
NQN"), applied in the kernel version 5.16. So "_have_kver 5 16" for the test
cases will avoid the failure Yang observes.
Yang, may I confirm the kernel version you use? If you use RHEL 8 based OS, I
think it is v4.18.
