On Tue, Apr 07, 2009 at 07:17:58PM -0400, James Smart wrote:
> Christof Schmitt wrote:
> --- a/drivers/s390/scsi/zfcp_fc.c 2009-04-06 17:23:25.000000000 +0200
> +++ b/drivers/s390/scsi/zfcp_fc.c 2009-04-06 17:24:30.000000000 +0200
> ...
>> +static void zfcp_fc_generic_els_handler(unsigned long data)
>> +{
>> + struct zfcp_els_fc_job *els_fc_job = (struct zfcp_els_fc_job *) data;
>> + struct fc_bsg_job *job = els_fc_job->job;
>> + struct fc_bsg_reply *reply = job->reply;
>> +
>> + if (els_fc_job->els.status) {
>> + /* request rejected or timed out */
>> + reply->reply_data.ctels_reply.status = FC_CTELS_STATUS_REJECT;
>> + goto out;
>> + }
>> +
>> + reply->reply_data.ctels_reply.status = FC_CTELS_STATUS_OK;
>> + reply->reply_payload_rcv_len = blk_rq_bytes(job->req->next_rq);
>
> My intent was to not have the LLDD reference (or even know about) the
> request structures and that bsg exists underneath.
>
> Thus, for the last line above, it should be:
> reply->reply_payload_rcv_len = job->reply_payload->data_len;
>
> Although, I'm a little surprised that your els interface doesn't supply the
> received data length to you, rather than the global assumption that you
> sized the reply buffer for exactly what you always get.
I guess this is one thing we have to investigate. Either use the
data_lan or get the data from the hardware.
>> +struct zfcp_ct_fc_job {
>> + struct zfcp_send_ct ct;
>> + struct fc_bsg_job *job;
>> +};
>> +
>> +static void zfcp_fc_generic_ct_handler(unsigned long data)
>> +{
>> + struct zfcp_ct_fc_job *ct_fc_job = (struct zfcp_ct_fc_job *) data;
>> + struct fc_bsg_job *job = ct_fc_job->job;
>> +
>> + job->reply->reply_data.ctels_reply.status = ct_fc_job->ct.status ?
>> + FC_CTELS_STATUS_REJECT : FC_CTELS_STATUS_OK;
>> + job->state_flags = FC_RQST_STATE_DONE;
>> + job->reply->reply_payload_rcv_len = blk_rq_bytes(job->req->next_rq);
>
> same comment as above.
>
>> + job->job_done(job);
>> +
>> + zfcp_wka_port_put(ct_fc_job->ct.wka_port);
>> +
>> + kfree(ct_fc_job);
>> +}
>
> Are you going to add support for the ELS/CT request timing out (e.g. for
> bsg_timeout callback) ?
We don't have an interface to our hardware to abort ELS or CT
requests. I suggested to remove the bsg_timeout callback, since there
is not much zfcp can do here. Any pending request will timeout in the
SAN or in the HBA if there is not response.
> Everything else looks good, and looks like it wasn't much effort to support.
> Great.
Yes, the result looks good.
With 2.6.30-rc1 being out, the next merge window will be for 2.6.31.
Are you going to push the FC passthrough support for 2.6.31?
--
Christof Schmitt
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
