| Message ID | 20221116001234.581003-1-alan.adamson@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests/nvme: Remove test output for passthrough error logging | expand |
On 11/15/22 16:12, Alan Adamson wrote: > Commit d7ac8dca938c ("nvme: quiet user passthrough command errors") > disabled error logging for passthrough commands so the associated > error output in nvme/039.out should be removed. > > When an error logging opt-in mechanism for passthrough commands is > provided, the error output can be added back. > > Signed-off-by: Alan Adamson <alan.adamson@oracle.com> > --- Shinichiro, I think this should fix the issue. Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> -ck
On Nov 15, 2022 / 16:12, Alan Adamson wrote: > Commit d7ac8dca938c ("nvme: quiet user passthrough command errors") > disabled error logging for passthrough commands so the associated > error output in nvme/039.out should be removed. > > When an error logging opt-in mechanism for passthrough commands is > provided, the error output can be added back. Thanks for this quick action. This two-steps approach looks good for me. I confirmed the fix avoids the failure with v6.1-rc5 kernel. Also, I observe this fix makes the test case fail with v6.0 kernel. I suggest to skip the test case with kernel v6.0 or older, applying the hunk below. Could you repost v2 with this change? Or if you want, I can apply it together with v1. Please let me know your preference. diff --git a/tests/nvme/039 b/tests/nvme/039 index e175055..ea626e3 100755 --- a/tests/nvme/039 +++ b/tests/nvme/039 @@ -14,6 +14,7 @@ QUICK=1 requires() { _have_program nvme + _have_kver 6 1 _have_kernel_option FAULT_INJECTION && \ _have_kernel_option FAULT_INJECTION_DEBUG_FS }
On Wed, Nov 16, 2022 at 04:17:03AM +0000, Shinichiro Kawasaki wrote: > On Nov 15, 2022 / 16:12, Alan Adamson wrote: > > Commit d7ac8dca938c ("nvme: quiet user passthrough command errors") > > disabled error logging for passthrough commands so the associated > > error output in nvme/039.out should be removed. > > > > When an error logging opt-in mechanism for passthrough commands is > > provided, the error output can be added back. > > Thanks for this quick action. This two-steps approach looks good for me. > > I confirmed the fix avoids the failure with v6.1-rc5 kernel. Also, I observe > this fix makes the test case fail with v6.0 kernel. I suggest to skip the test > case with kernel v6.0 or older, applying the hunk below. Could you repost v2 > with this change? Or if you want, I can apply it together with v1. Please let > me know your preference. It sounds like some future case might allow these errors to log in some circumstances, so I'm not sure the test case should be so dependent on seeing or not seeing these messages. Is there a mechanism to say these are optional messages that may or may not show up?
> On Nov 16, 2022, at 8:48 AM, Keith Busch <kbusch@kernel.org> wrote: > > On Wed, Nov 16, 2022 at 04:17:03AM +0000, Shinichiro Kawasaki wrote: >> On Nov 15, 2022 / 16:12, Alan Adamson wrote: >>> Commit d7ac8dca938c ("nvme: quiet user passthrough command errors") >>> disabled error logging for passthrough commands so the associated >>> error output in nvme/039.out should be removed. >>> >>> When an error logging opt-in mechanism for passthrough commands is >>> provided, the error output can be added back. >> >> Thanks for this quick action. This two-steps approach looks good for me. >> >> I confirmed the fix avoids the failure with v6.1-rc5 kernel. Also, I observe >> this fix makes the test case fail with v6.0 kernel. I suggest to skip the test >> case with kernel v6.0 or older, applying the hunk below. Could you repost v2 >> with this change? Or if you want, I can apply it together with v1. Please let >> me know your preference. > > It sounds like some future case might allow these errors to log in some > circumstances, so I'm not sure the test case should be so dependent on > seeing or not seeing these messages. Is there a mechanism to say these > are optional messages that may or may not show up? We could just remove the tests for now which would also work for pre-6.1 and add them back when the future case shows up (Hopefully soon - I’m working on it). The test will be changed to test the opted-in and opted-out mechanism. Alan
> >>> I confirmed the fix avoids the failure with v6.1-rc5 kernel. Also, I observe >>> this fix makes the test case fail with v6.0 kernel. I suggest to skip the test >>> case with kernel v6.0 or older, applying the hunk below. Could you repost v2 >>> with this change? Or if you want, I can apply it together with v1. Please let >>> me know your preference. >> >> It sounds like some future case might allow these errors to log in some >> circumstances, so I'm not sure the test case should be so dependent on >> seeing or not seeing these messages. Is there a mechanism to say these >> are optional messages that may or may not show up? > > We could just remove the tests for now which would also work for pre-6.1 and add them back > when the future case shows up (Hopefully soon - I’m working on it). The test will be changed > to test the opted-in and opted-out mechanism. > This seems to be the right thing to add the testcase with right feature upstream than fixing the broken testcase creating compatibility issues. -ck
diff --git a/tests/nvme/039.out b/tests/nvme/039.out index 162935eb1d7b..139070d22240 100644 --- a/tests/nvme/039.out +++ b/tests/nvme/039.out @@ -2,6 +2,4 @@ Running nvme/039 Read(0x2) @ LBA 0, 1 blocks, Unrecovered Read Error (sct 0x2 / sc 0x81) DNR Read(0x2) @ LBA 0, 1 blocks, Unknown (sct 0x3 / sc 0x75) DNR Write(0x1) @ LBA 0, 1 blocks, Write Fault (sct 0x2 / sc 0x80) DNR - Identify(0x6), Access Denied (sct 0x2 / sc 0x86) DNR - Unknown(0x96), Invalid Command Opcode (sct 0x0 / sc 0x1) DNR Test complete
Commit d7ac8dca938c ("nvme: quiet user passthrough command errors") disabled error logging for passthrough commands so the associated error output in nvme/039.out should be removed. When an error logging opt-in mechanism for passthrough commands is provided, the error output can be added back. Signed-off-by: Alan Adamson <alan.adamson@oracle.com> --- tests/nvme/039.out | 2 -- 1 file changed, 2 deletions(-)