On 07/29/2013 04:13 PM, Alexey Kardashevskiy wrote:
> On 07/29/2013 04:08 PM, Paolo Bonzini wrote:
>> Il 29/07/2013 06:49, Alexey Kardashevskiy ha scritto:
>>> At the moment the guest kernel issues two types of task management
>>> requests to the hypervisor - task about and lun reset. This adds
>>> handling for these tasks. As spapr-vscsi starts calling scsi_req_cancel(),
>>> free_request callback was implemented.
>>>
>>> As virtio-vscsi, spapr-vscsi does not handle CLEAR_ACA either as CDB
>>> control byte does not seem to be used at all so NACA bit is not
>>> set to the guest so the guest has no good reason to call CLEAR_ACA task.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
>>> ---
>>> Changes:
>>> 2013/07/29 (v5):
>>> * sending response on task management was fixed not to use vscsi_send_rsp()
>>> as it is unable to send response data
>>>
>>> 2013/07/26:
>>> * fixed error handling
>>>
>>> 2013/07/23:
>>> * remove unnecessary free_request callback
>>>
>>> 2013/07/22:
>>> * fixed LUN_RESET (it used to clear requests while it should reset a device)
>>> * added handling of ABORT_TASK_SET/CLEAR_TASK_SET
>>>
>>> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
>>> ---
>>> hw/scsi/spapr_vscsi.c | 117
>>> ++++++++++++++++++++++++++++++++++++++------------
>>> hw/scsi/srp.h | 7 +++
>>> 2 files changed, 96 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
>>> index 541ffcc..8e49d0d 100644
>>> --- a/hw/scsi/spapr_vscsi.c
>>> +++ b/hw/scsi/spapr_vscsi.c
>>> @@ -118,6 +118,20 @@ static struct vscsi_req *vscsi_get_req(VSCSIState *s)
>>> return NULL;
>>> }
>>>
>>> +static struct vscsi_req *vscsi_find_req(VSCSIState *s, uint64_t srp_tag)
>>> +{
>>> + vscsi_req *req;
>>> + int i;
>>> +
>>> + for (i = 0; i < VSCSI_REQ_LIMIT; i++) {
>>> + req = &s->reqs[i];
>>> + if (req->iu.srp.cmd.tag == srp_tag) {
>>> + return req;
>>> + }
>>> + }
>>> + return NULL;
>>> +}
>>> +
>>> static void vscsi_put_req(vscsi_req *req)
>>> {
>>> if (req->sreq != NULL) {
>>> @@ -654,40 +668,87 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req
>>> *req)
>>> static int vscsi_process_tsk_mgmt(VSCSIState *s, vscsi_req *req)
>>> {
>>> union viosrp_iu *iu = &req->iu;
>>> - int fn;
>>> + vscsi_req *tmpreq;
>>> + int i, lun = 0, resp = SRP_TSK_MGMT_COMPLETE;
>>> + SCSIDevice *d;
>>> + uint64_t tag = iu->srp.rsp.tag;
>>> + uint8_t sol_not = iu->srp.cmd.sol_not;
>>>
>>> fprintf(stderr, "vscsi_process_tsk_mgmt %02x\n",
>>> iu->srp.tsk_mgmt.tsk_mgmt_func);
>>>
>>> - switch (iu->srp.tsk_mgmt.tsk_mgmt_func) {
>>> -#if 0 /* We really don't deal with these for now */
>>> - case SRP_TSK_ABORT_TASK:
>>> - fn = ABORT_TASK;
>>> - break;
>>> - case SRP_TSK_ABORT_TASK_SET:
>>> - fn = ABORT_TASK_SET;
>>> - break;
>>> - case SRP_TSK_CLEAR_TASK_SET:
>>> - fn = CLEAR_TASK_SET;
>>> - break;
>>> - case SRP_TSK_LUN_RESET:
>>> - fn = LOGICAL_UNIT_RESET;
>>> - break;
>>> - case SRP_TSK_CLEAR_ACA:
>>> - fn = CLEAR_ACA;
>>> - break;
>>> -#endif
>>> - default:
>>> - fn = 0;
>>> - }
>>> - if (fn) {
>>> - /* XXX Send/Handle target task management */
>>> - ;
>>> + d = vscsi_device_find(&s->bus, be64_to_cpu(req->iu.srp.tsk_mgmt.lun),
>>> &lun);
>>> + if (!d) {
>>> + resp = SRP_TSK_MGMT_FIELDS_INVALID;
>>> } else {
>>> - vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x20, 0);
>>> - vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
>>> + switch (iu->srp.tsk_mgmt.tsk_mgmt_func) {
>>> + case SRP_TSK_ABORT_TASK:
>>> + if (d->lun != lun) {
>>> + resp = SRP_TSK_MGMT_FIELDS_INVALID;
>>> + break;
>>> + }
>>> +
>>> + tmpreq = vscsi_find_req(s, req->iu.srp.tsk_mgmt.task_tag);
>>> + if (tmpreq && tmpreq->sreq) {
>>> + assert(tmpreq->sreq->hba_private);
>>> + scsi_req_cancel(tmpreq->sreq);
>>> + }
>>> + break;
>>> +
>>> + case SRP_TSK_LUN_RESET:
>>> + if (d->lun != lun) {
>>> + resp = SRP_TSK_MGMT_FIELDS_INVALID;
>>> + break;
>>> + }
>>> +
>>> + qdev_reset_all(&d->qdev);
>>> + break;
>>> +
>>> + case SRP_TSK_ABORT_TASK_SET:
>>> + case SRP_TSK_CLEAR_TASK_SET:
>>> + if (d->lun != lun) {
>>> + resp = SRP_TSK_MGMT_FIELDS_INVALID;
>>> + break;
>>> + }
>>> +
>>> + for (i = 0; i < VSCSI_REQ_LIMIT; i++) {
>>> + tmpreq = &s->reqs[i];
>>> + if (tmpreq->iu.srp.cmd.lun != req->iu.srp.tsk_mgmt.lun) {
>>> + continue;
>>> + }
>>> + if (!tmpreq->active || !tmpreq->sreq) {
>>> + continue;
>>> + }
>>> + assert(tmpreq->sreq->hba_private);
>>> + scsi_req_cancel(tmpreq->sreq);
>>> + }
>>> + break;
>>> +
>>> + case SRP_TSK_CLEAR_ACA:
>>> + resp = SRP_TSK_MGMT_NOT_SUPPORTED;
>>> + break;
>>> +
>>> + default:
>>> + resp = SRP_TSK_MGMT_FIELDS_INVALID;
>>> + break;
>>> + }
>>> }
>>> - return !fn;
>>> +
>>> + /* Compose the response here as */
>>> + memset(iu, 0, sizeof(struct srp_rsp) + 4);
>>> + iu->srp.rsp.opcode = SRP_RSP;
>>> + iu->srp.rsp.req_lim_delta = cpu_to_be32(1);
>>> + iu->srp.rsp.tag = tag;
>>> + iu->srp.rsp.flags |= SRP_RSP_FLAG_RSPVALID;
>>> + iu->srp.rsp.resp_data_len = cpu_to_be32(4);
>>> + /* Copy UCSOLNT bit as SRP_RSP_FLAG_RSPVALID is set */
>>> + iu->srp.rsp.sol_not = (sol_not & 0x04) >> 1;
>>
>> This is ">> 2" and also needs to look at SCSOLNT with the same rules as
>> normal responses, but I can fix this when applying.
>
>
> Ouch, overlooked :( Thanks!
Did this patch go anywhere? I could not find it in
git://github.com/bonzini/qemu.git scsi-next or anywhere else.
Should I repost it as qemu 1.6 was released and we started new cycle?
I am asking because I have 2 more patches about vscsi which depend on this one.
Thank you.
--
Alexey
