Re: [RFC][PATCH 2/6] fnic: add fnic_scsi.c and fnic_io.h.

Mike Christie
2008-08-25T19:38:30Z

James Smart wrote:
>
>
> Mike Christie wrote:
>> jeykholt@cisco.com wrote:
>>> + * fnic_queuecommand
>>> + * Routine to send a scsi cdb
>>> + * Called with host_lock held and interrupts disabled.
>>> + */
>>> +int fnic_queuecommand(struct scsi_cmnd *sc, void (*done)(struct
>>> scsi_cmnd *))
>>> +{
>>> + struct fc_lport *lp;
>>> + struct fc_rport *rport;
>>> + struct fnic_io_req *io_req;
>>> + struct fnic *fnic;
>>> + struct vnic_wq_copy *wq;
>>> + int ret;
>>> + u32 sg_count;
>>> + unsigned long flags;
>>> +
>>> + rport = starget_to_rport(scsi_target(sc->device));
>>> + ret = fc_remote_port_chkready(rport);
>>> + if (ret) {
>>> + sc->result = ret;
>>> + done(sc);
>>> + return 0;
>>> + }
>>> +
>>> + lp = shost_priv(sc->device->host);
>>> + if (lp->state != LPORT_ST_READY || !(lp->link_status &
>>> FC_LINK_UP)) {
>>> + sc->result = DID_NO_CONNECT << 16;
>>
>> You should use SCSI_MLQUEUE_HOST_BUSY here, becuase we set the link
>> status/state and then will block rports, if a io has passed the rport
>> check then sees the updated state we will fail it when it should be
>> requeued until the fc class decides what to do.
>
> Are you sure ? I would assume that if you passed the chkready() test,

Yes. See fc_linkdown. It sets the status and state then blocks rports.
Or actually it should be deleting/blocking rports but from that thread
on fcoe-devel that you were on you know that that code is not completely
right and is getting fixed.

> but still don't have connectivity to the device, you should be failing
> it with a DID_NO_CONNECT. The chkready(), paired with the
> fc_remote_port_delete()/blocked luns, should cover the temporary losses
> of connectivity.
>

The check above I am commenting on is like lpfc's

if (!ndlp || !NLP_CHK_NODE_ACT(ndlp)) {
cmnd->result = ScsiResult(DID_BUS_BUSY, 0);
goto out_fail_command;
}

or qla2xxxx's

if (atomic_read(&fcport->state) != FCS_ONLINE) {
if (atomic_read(&fcport->state) == FCS_DEVICE_DEAD ||
atomic_read(&ha->loop_state) == LOOP_DEAD) {
cmd->result = DID_NO_CONNECT << 16;
goto qc_fail_command;
}
goto qc_host_busy;
}

where we are in the middle of transistioning. The driver/lib has set
some internal state, and is in the middle of calling the rport delete
function.