| Message ID | 20220420092503.11123-3-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/pv-scsi: update header and harden frontend | expand |
On 4/20/22 5:25 AM, Juergen Gross wrote: > @@ -569,7 +645,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req, > wait_for_completion(&pending_req->tmr_done); > > err = (se_cmd->se_tmr_req->response == TMR_FUNCTION_COMPLETE) ? > - SUCCESS : FAILED; > + XEN_VSCSIIF_RSLT_RESET_SUCCESS : XEN_VSCSIIF_RSLT_RESET_FAILED; > > scsiback_do_resp_with_sense(NULL, err, 0, pending_req); > transport_generic_free_cmd(&pending_req->se_cmd, 0); You also want to initialize err to XEN_VSCSIIF_RSLT_RESET_FAILED. And also looking at invocations of scsiback_do_resp_with_sense() I think those may need to be adjusted as well. -boris
On 20.04.22 18:12, Boris Ostrovsky wrote: > > On 4/20/22 5:25 AM, Juergen Gross wrote: >> @@ -569,7 +645,7 @@ static void scsiback_device_action(struct vscsibk_pend >> *pending_req, >> wait_for_completion(&pending_req->tmr_done); >> err = (se_cmd->se_tmr_req->response == TMR_FUNCTION_COMPLETE) ? >> - SUCCESS : FAILED; >> + XEN_VSCSIIF_RSLT_RESET_SUCCESS : XEN_VSCSIIF_RSLT_RESET_FAILED; >> scsiback_do_resp_with_sense(NULL, err, 0, pending_req); >> transport_generic_free_cmd(&pending_req->se_cmd, 0); > > > You also want to initialize err to XEN_VSCSIIF_RSLT_RESET_FAILED. I did that. > And also looking at invocations of scsiback_do_resp_with_sense() I think those > may need to be adjusted as well. No, the invocations are fine, but scsiback_result() needs to pass through the lowest 16 bits instead of only the lowest 8 bits of the result value. Juergen
On 4/21/22 4:40 AM, Juergen Gross wrote: > On 20.04.22 18:12, Boris Ostrovsky wrote: >> >> On 4/20/22 5:25 AM, Juergen Gross wrote: >>> @@ -569,7 +645,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req, >>> wait_for_completion(&pending_req->tmr_done); >>> err = (se_cmd->se_tmr_req->response == TMR_FUNCTION_COMPLETE) ? >>> - SUCCESS : FAILED; >>> + XEN_VSCSIIF_RSLT_RESET_SUCCESS : XEN_VSCSIIF_RSLT_RESET_FAILED; >>> scsiback_do_resp_with_sense(NULL, err, 0, pending_req); >>> transport_generic_free_cmd(&pending_req->se_cmd, 0); >> >> >> You also want to initialize err to XEN_VSCSIIF_RSLT_RESET_FAILED. > > I did that. Yes you did. I don't know what I was was looking at. > >> And also looking at invocations of scsiback_do_resp_with_sense() I think those may need to be adjusted as well. > > No, the invocations are fine, but scsiback_result() needs to pass through > the lowest 16 bits instead of only the lowest 8 bits of the result value. > What I was thinking was that this could use the reverse of XEN_VSCSIIF_RSLT_HOST(), i.e. something like #define RSLT_HOST_TO_XEN_VSCSIIF(x) ((x)<<16) to be explicit about namespaces. BTW, should scsiback_result() use XEN_VSCSIIF_RSLT_HOST() at the top? -boris
On 21.04.22 22:56, Boris Ostrovsky wrote: > > On 4/21/22 4:40 AM, Juergen Gross wrote: >> On 20.04.22 18:12, Boris Ostrovsky wrote: >>> And also looking at invocations of scsiback_do_resp_with_sense() I think >>> those may need to be adjusted as well. >> >> No, the invocations are fine, but scsiback_result() needs to pass through >> the lowest 16 bits instead of only the lowest 8 bits of the result value. >> > > What I was thinking was that this could use the reverse of > XEN_VSCSIIF_RSLT_HOST(), i.e. something like > > #define RSLT_HOST_TO_XEN_VSCSIIF(x) ((x)<<16) > > to be explicit about namespaces. I don't think this is needed. > BTW, should scsiback_result() use XEN_VSCSIIF_RSLT_HOST() at the top? Yes, I'll do that. Juergen
diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c index 0c5e565aa8cf..673dd73844ff 100644 --- a/drivers/xen/xen-scsiback.c +++ b/drivers/xen/xen-scsiback.c @@ -280,6 +280,82 @@ static void scsiback_free_translation_entry(struct kref *kref) kfree(entry); } +static int32_t scsiback_result(int32_t result) +{ + int32_t host_status; + + switch (result >> 16) { + case DID_OK: + host_status = XEN_VSCSIIF_RSLT_HOST_OK; + break; + case DID_NO_CONNECT: + host_status = XEN_VSCSIIF_RSLT_HOST_NO_CONNECT; + break; + case DID_BUS_BUSY: + host_status = XEN_VSCSIIF_RSLT_HOST_BUS_BUSY; + break; + case DID_TIME_OUT: + host_status = XEN_VSCSIIF_RSLT_HOST_TIME_OUT; + break; + case DID_BAD_TARGET: + host_status = XEN_VSCSIIF_RSLT_HOST_BAD_TARGET; + break; + case DID_ABORT: + host_status = XEN_VSCSIIF_RSLT_HOST_ABORT; + break; + case DID_PARITY: + host_status = XEN_VSCSIIF_RSLT_HOST_PARITY; + break; + case DID_ERROR: + host_status = XEN_VSCSIIF_RSLT_HOST_ERROR; + break; + case DID_RESET: + host_status = XEN_VSCSIIF_RSLT_HOST_RESET; + break; + case DID_BAD_INTR: + host_status = XEN_VSCSIIF_RSLT_HOST_BAD_INTR; + break; + case DID_PASSTHROUGH: + host_status = XEN_VSCSIIF_RSLT_HOST_PASSTHROUGH; + break; + case DID_SOFT_ERROR: + host_status = XEN_VSCSIIF_RSLT_HOST_SOFT_ERROR; + break; + case DID_IMM_RETRY: + host_status = XEN_VSCSIIF_RSLT_HOST_IMM_RETRY; + break; + case DID_REQUEUE: + host_status = XEN_VSCSIIF_RSLT_HOST_REQUEUE; + break; + case DID_TRANSPORT_DISRUPTED: + host_status = XEN_VSCSIIF_RSLT_HOST_TRANSPORT_DISRUPTED; + break; + case DID_TRANSPORT_FAILFAST: + host_status = XEN_VSCSIIF_RSLT_HOST_TRANSPORT_FAILFAST; + break; + case DID_TARGET_FAILURE: + host_status = XEN_VSCSIIF_RSLT_HOST_TARGET_FAILURE; + break; + case DID_NEXUS_FAILURE: + host_status = XEN_VSCSIIF_RSLT_HOST_NEXUS_FAILURE; + break; + case DID_ALLOC_FAILURE: + host_status = XEN_VSCSIIF_RSLT_HOST_ALLOC_FAILURE; + break; + case DID_MEDIUM_ERROR: + host_status = XEN_VSCSIIF_RSLT_HOST_MEDIUM_ERROR; + break; + case DID_TRANSPORT_MARGINAL: + host_status = XEN_VSCSIIF_RSLT_HOST_TRANSPORT_MARGINAL; + break; + default: + host_status = XEN_VSCSIIF_RSLT_HOST_ERROR; + break; + } + + return (host_status << 16) | (result & 0x00ff); +} + static void scsiback_send_response(struct vscsibk_info *info, char *sense_buffer, int32_t result, uint32_t resid, uint16_t rqid) @@ -295,7 +371,7 @@ static void scsiback_send_response(struct vscsibk_info *info, ring_res = RING_GET_RESPONSE(&info->ring, info->ring.rsp_prod_pvt); info->ring.rsp_prod_pvt++; - ring_res->rslt = result; + ring_res->rslt = scsiback_result(result); ring_res->rqid = rqid; if (sense_buffer != NULL && @@ -555,7 +631,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req, struct scsiback_nexus *nexus = tpg->tpg_nexus; struct se_cmd *se_cmd = &pending_req->se_cmd; u64 unpacked_lun = pending_req->v2p->lun; - int rc, err = FAILED; + int rc, err = XEN_VSCSIIF_RSLT_RESET_FAILED; init_completion(&pending_req->tmr_done); @@ -569,7 +645,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req, wait_for_completion(&pending_req->tmr_done); err = (se_cmd->se_tmr_req->response == TMR_FUNCTION_COMPLETE) ? - SUCCESS : FAILED; + XEN_VSCSIIF_RSLT_RESET_SUCCESS : XEN_VSCSIIF_RSLT_RESET_FAILED; scsiback_do_resp_with_sense(NULL, err, 0, pending_req); transport_generic_free_cmd(&pending_req->se_cmd, 0);
Instead of using the kernel's values for the result of PV scsi operations use the values of the interface definition. Signed-off-by: Juergen Gross <jgross@suse.com> --- drivers/xen/xen-scsiback.c | 82 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 79 insertions(+), 3 deletions(-)