On Sun, 2013-07-21 at 20:04 +1000, Alexey Kardashevskiy wrote:
> 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.
My worry is that the specification calls for all of them, and we don't
have ways to advertize that we support only a subset, so we might end
up with backward compatibility problems with future clients.
Paolo, how did you generally test the task mgmnt implementation you did
in virtio-scsi ? The set of tasks looks fairly similar (and similarly,
itl looks like the Linux client will only use a couple of them).
> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
> ---
>
> I still do not have really good test to test the task management, any working
> ideas? :)
>
> This is made on top of "pseries: rework PAPR virtual SCSI" which some day may
> make it to upstream.
>
> ---
> hw/scsi/spapr_vscsi.c | 84
> +++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 64 insertions(+), 20 deletions(-)
>
> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
> index a86199b..6b90c9c 100644
> --- a/hw/scsi/spapr_vscsi.c
> +++ b/hw/scsi/spapr_vscsi.c
> @@ -117,6 +117,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) {
> @@ -641,6 +655,13 @@ static void *vscsi_load_request(QEMUFile *f, SCSIRequest
> *sreq)
> return req;
> }
>
> +static void vscsi_free_request(SCSIBus *bus, void *priv)
> +{
> + vscsi_req *req = priv;
> +
> + vscsi_put_req(req);
> +}
> +
> static void vscsi_process_login(VSCSIState *s, vscsi_req *req)
> {
> union viosrp_iu *iu = &req->iu;
> @@ -750,43 +771,65 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req
> *req)
> return 0;
> }
>
> +/*
> + * The SRP services request the SRP initiator port issue an SRP_TSK_MGMT
> + * request (see 6.7) with a TASK MANAGEMENT FLAGS field set to indicate
> + * an ABORT TASK function to be sent to the selected SCSI device.
> + */
> +static int vscsi_tsk_mgmt_abort_task(VSCSIState *s, vscsi_req *req)
> +{
> + vscsi_req *task_req = vscsi_find_req(s, req->iu.srp.tsk_mgmt.task_tag);
> +
> + scsi_req_cancel(task_req->sreq);
> +
> + return 1;
> +}
> +
> +/*
> + * The SRP services request the SRP initiator port issue an SRP_TSK_MGMT
> + * request (see 6.7) with a TASK MANAGEMENT FLAGS field set to indicate
> + * a LOGICAL UNIT RESET function to be sent to the selected SCSI device.
> + */
> +static int vscsi_tsk_mgmt_lun_reset(VSCSIState *s, vscsi_req *req)
> +{
> + int i;
> + vscsi_req *tmpreq;
> +
> + for (i = 0; i < VSCSI_REQ_LIMIT; i++) {
> + tmpreq = &s->reqs[i];
> + if ((tmpreq->iu.srp.cmd.lun == req->iu.srp.tsk_mgmt.lun) &&
> + tmpreq->active && tmpreq->sreq) {
> + scsi_req_cancel(tmpreq->sreq);
> + }
> + }
> +
> + return 1;
> +}
> +
> static int vscsi_process_tsk_mgmt(VSCSIState *s, vscsi_req *req)
> {
> union viosrp_iu *iu = &req->iu;
> - int fn;
> + int ret = 0;
>
> 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;
> + ret = vscsi_tsk_mgmt_abort_task(s, req);
> + break;
> + case SRP_TSK_LUN_RESET:
> + ret = vscsi_tsk_mgmt_lun_reset(s, req);
> 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 */
> - ;
> - } else {
> vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x20, 0);
> vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
> + ret = 1;
> }
> - return !fn;
> + return ret;
> }
>
> static int vscsi_handle_srp_req(VSCSIState *s, vscsi_req *req)
> @@ -998,6 +1041,7 @@ static const struct SCSIBusInfo vscsi_scsi_info = {
> .cancel = vscsi_request_cancelled,
> .save_request = vscsi_save_request,
> .load_request = vscsi_load_request,
> + .free_request = vscsi_free_request,
> };
The addition of free_request is specific to the task management ? Or
something that should be done regardless (ie. in a separate patch) ?
Cheers,
Ben.
> static void spapr_vscsi_reset(VIOsPAPRDevice *dev)
