nvme: fix resv commands with nda device

[yuri_aetern.org]

Summary

• passing I/O commands through nda requires nsid field to be set (it was unused when going through nvme_ns_ioctl())
• ccb's status can be OR'ed with the flags, use CAM_STATUS_MASK

Test Plan

pre:

# nvmecontrol resv report nda24
nvmecontrol: report request failed: Input/output error

nvme24: RESERVATION REPORT sqid:3 cid:127 nsid:0
nvme24: INVALID NAMESPACE OR FORMAT (00/0b) crd:0 m:0 dnr:1 sqid:3 cid:127 cdw0:0

post:

# nvmecontrol resv report nda24
Generation:                       2152
Reservation Type:                 3
Number of Registered Controllers: 1
Persist Through Power Loss State: 0
Controller ID:                    0x0023
  Reservation Status:             1
  Host Identifier:                0x2818887a
  Reservation Key:                0xea35a678b846fea2

[imp]

With one exception, these changes look good to my eye.
But I've not chased things all the way through the protocol stack to make sure that the nsid isn't getting overridden in the passthrough cases (I don't see any, btw, but I didn't chase all possible paths down)

sys/cam/nvme/nvme_da.c

444–445

This is wrong. CAM_REQ_CMP is not a bitmask.

But it should be CAM_REQ_CMP always at this point in the code. What is it? None of the xtra bits should be set on a passthrough CCB, so this confuses me.

The proper thing to test for, though, is likely (ccb->ccb_h.status & CAM_STATUS_MASK) to account for those extra bits, all of which appear to be only in the error / recovery path, or for queueing commands which the passthrough interface doesn't do today.

[imp]

sys/cam/nvme/nvme_da.c

444–445

"should be CAM_REQ_CMP on success" I should have said. It can be any of the other errors too...

[yuri_aetern.org]

sys/cam/nvme/nvme_da.c

444–445

May be I'm phrasing it wrong, what I meant is that it got OR'ed in the following commit:

commit abea0c6b0ddc969d1f3b9cab1fd90df00de1f2e4
Author: Warner Losh <[e-mail]>
Date:   Sat Jul 17 16:10:46 2021 -0600

    cam: Mark the qos data is valid in xpd_done_direct() too.

    Sponsored by:           Netflix

diff --git a/sys/cam/cam_xpt.c b/sys/cam/cam_xpt.c
index b76d6f5adde..6b5a04b174e 100644
--- a/sys/cam/cam_xpt.c
+++ b/sys/cam/cam_xpt.c
@@ -4642,6 +4642,7 @@ xpt_done_direct(union ccb *done_ccb)

        /* Store the time the ccb was in the sim */
        done_ccb->ccb_h.qos.periph_data = cam_iosched_delta_t(done_ccb->ccb_h.qos.periph_data);
+       done_ccb->ccb_h.status |= CAM_QOS_VALID;
        xpt_done_process(&done_ccb->ccb_h);
 }

[yuri_aetern.org]

use CAM_STATUS_MASK, status is not really a bitmap

[imp]

This looks good to me now.... I must not have been doing passthrough commands that set QOS_VALID on completion somehow...

[imp]

Can I get a Signed-off-by: line or an email address with your full name so I can commit?

[yuri_aetern.org]

In D37696#857437, @imp wrote:

Can I get a Signed-off-by: line or an email address with your full name so I can commit?

Let's go with yuripv@.