Re: [PATCH 6/7] scsi: Use Scsi_Host as argument for eh_host_reset_handler

[Hannes Reinecke <hare@suse.de>]

On 06/27/2014 12:47 PM, Steffen Maier wrote:
> On 06/27/2014 08:27 AM, Hannes Reinecke wrote:
>> Issuing a host reset should not rely on any commands.
>> So use Scsi_Host as argument for eh_host_reset_handler.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>
>>   drivers/s390/scsi/zfcp_scsi.c             |  3 +-
>
>>   69 files changed, 289 insertions(+), 379 deletions(-)
>
>> diff --git a/drivers/s390/scsi/zfcp_scsi.c
>> b/drivers/s390/scsi/zfcp_scsi.c
>> index dc42c93..fe50f69 100644
>> --- a/drivers/s390/scsi/zfcp_scsi.c
>> +++ b/drivers/s390/scsi/zfcp_scsi.c
>> @@ -281,13 +281,14 @@ static int
>> zfcp_scsi_eh_target_reset_handler(struct scsi_cmnd *scpnt)
>>       return zfcp_task_mgmt_function(scpnt, FCP_TMF_TGT_RESET);
>>   }
>>
>> -static int zfcp_scsi_eh_host_reset_handler(struct scsi_cmnd *scpnt)
>> +static int zfcp_scsi_eh_host_reset_handler(struct Scsi_Host *host)
>>   {
>>       struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(scpnt->device);
>
> Scpnt argument no longer exists, so this line must likely go away to
> compile.
>
>>       struct zfcp_adapter *adapter = zfcp_sdev->port->adapter;
>
> This needs the assignment from the added line below since zfcp_sdev
> no longer exists.
> Alternatively, drop the assignment here, only have the auto variable
> definition, and do the assignment below with the added line.
>
>>       struct fc_rport *rport = zfcp_sdev->port->rport;
>>       int ret;
>>
>> +    adapter = (struct zfcp_adapter *)host->hostdata[0];
>>       zfcp_erp_adapter_reopen(adapter, 0, "schrh_1");
>>       zfcp_erp_wait(adapter);
>>       ret = fc_block_scsi_eh(rport);
>
> A question just for my understanding: Calling fc_block_scsi_eh at
> the end *after* our adapter recovery is OK to wait for rports to
> become unblocked (or fast_io_failed)?
> I would guess so.
>
Hehe. That's actually a question for _you_ :-)

Thing is, 'fc_block_scsi-eh' only waits for the rport to become 
unblocked; the unblocking itself would need to be done from the LLDD.
So I would like to ask the maintainer of the LLDD why this code is 
there; normally I would have expected the 'zfcp_erp_wait' to only
return after the HBA is fully online again.

And in general waiting for an rport during host reset is _really_ a 
bad idea. The rport might not be present after host reset at all,
making the check pretty much pointless.

Can't you check whether the HBA is in a working state after 
zfcp_erp_wait() and return a status based on that?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html