On Fri, 2017-06-23 at 15:02 +0200, Hannes Reinecke wrote:
> -static int zfcp_scsi_eh_target_reset_handler(struct scsi_cmnd *scpnt)
> +/*
> + * Note: We need to select a LUN as the storage array doesn't
> + * necessarily supports LUN 0 and might refuse the target reset.
> + */
> +static int zfcp_scsi_eh_target_reset_handler(struct scsi_target *starget)
A minor comment: I assume that "supports" should have been "support"?
> -static int pmcraid_eh_target_reset_handler(struct scsi_cmnd *scmd)
> +static int pmcraid_eh_target_reset_handler(struct scsi_target *starget)
> {
> - scmd_printk(KERN_INFO, scmd,
> + struct Scsi_Host *shost = dev_to_shost(&starget->dev);
> + struct scsi_device *scsi_dev = NULL, *tmp;
> +
> + shost_for_each_device(tmp, shost) {
> + if ((tmp->channel == starget->channel) &&
> + (tmp->id == starget->id)) {
> + scsi_dev = tmp;
> + break;
> + }
> + }
> + if (!scsi_dev)
> + return FAILED;
> + sdev_printk(KERN_INFO, scsi_dev,
> "Doing target reset due to an I/O command timeout.\n");
> - return pmcraid_reset_device(scmd->device,
> + return pmcraid_reset_device(scsi_dev,
> PMCRAID_INTERNAL_TIMEOUT,
> RESET_DEVICE_TARGET);
> }
Is this sdev_printk() statement really useful? If any kernel messages are
reported due to a reset, shouldn't that be done by the SCSI core instead of
by a SCSI LLD?
> void qedf_wait_for_upload(struct qedf_ctx *qedf)
> diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
> index f02a2ba..443bb29 100644
> --- a/drivers/scsi/qla2xxx/qla_mbx.c
> +++ b/drivers/scsi/qla2xxx/qla_mbx.c
> @@ -1337,7 +1337,6 @@ static int is_rom_cmd(uint16_t cmd)
> struct req_que *req;
> struct rsp_que *rsp;
>
> - l = l;
> vha = fcport->vha;
>
> ql_dbg(ql_dbg_mbx + ql_dbg_verbose, vha, 0x103e,
That's a real gem ...
Although I'm not familiar enough with all the drivers that are modified by
this patch to do an in-depth review, as far as I can see the changes in this
patch look fine to me. Hence:
Reviewed-by: Bart Van Assche <bart.vanassche@xxxxxxx>
