Re: [PATCH] scsi_transport_fc: Make 'port_state' writeable

[James Smart <James.Smart@emulex.com>]

Hannes,

I'm finally going through this thread.  I'm a bit uncomfortable about 
making the rport state writeable.  The rport state is tightly tied to 
the driver's discovery engine, and I'm concerned about some of the 
callbacks firing, such as the rport_delete(), and the driver taking an 
action that isn't legit as the driver wasn't the one that called the 
fc_remote_port_delete() call.

I expect, at least in the lpfc driver, we would require a change to work 
with this. Although, we had to add an ugly internal reference counter 
mechanism so we didn't actually early-free structures, that may have 
saved your changing of this.

-- james  s


On 3/15/2013 7:55 AM, Hannes Reinecke wrote:
> On 03/14/2013 07:09 PM, Steffen Maier wrote:
>> Just for my understanding:
>> So this is a bit like writing 0 into the failed attribute of a
>> zfcp_port in sysfs (zfcp_sysfs_port_failed_store()) which forces a
>> port reopen recovery including a sequence of fc_remote_port_delete
>> and fc_remote_port_add?
> Sorta.
> This patch simulates the result of an RSCN where this remote port is 
> missing. So the fast_io_fail / dev_loss_tmo mechanism is triggered 
> (until further notice).
> It'll be reset with the next RSCN or by manually resetting the port 
> state.
>
>> If so, it sounds good to have this functionality for any FC LLD in
>> common code.
>
> That's why I posted this code :-)
> Rationale for this patch is a weird test case with brocade switches;
> there you can actually disable a _target_ port. So the port isn't
> reachable anymore but no RSCN is send.
> And the LLDD is forced into error recovery which'll take _ages_ as 
> each and every command send during error recovery will time out.
>
>> (And I can think about deprecating parts of zfcp code, next we could
>> consider the same for zfcp's forced LUN and adapter recovery or
>> maybe this already exists as sdev's writable state attribute and the
>> adapter recovery can be performed manually by walking all fc_rports
>> writing their port_state.)
>>
> I don't think the sdev 'state' attribute is useable here; it doesn't 
> trigger a rediscovery. You'd want to use 'rescan' here.
> But yeah, that would be one of the goals.
>
> Still waiting for some response to the patch, though.
> Testing has been successful on our side, plus I've posted
> a patchset for multipath-tools utilizing this patch.
>
> So it would be good if it could be included.
>
> James?
>
> Cheers,
>
> Hannes