svn commit: r291911 - head/sys/dev/iscsi

[Edward Tomasz Napierała]

On 1207T1134, Steven Hartland wrote:
> 
> 
> On 07/12/2015 11:22, Edward Tomasz Napierała wrote:
> > On 1207T0256, Steven Hartland wrote:
> >> Author: smh
> >> Date: Mon Dec  7 02:56:08 2015
> >> New Revision: 291911
> >> URL: https://svnweb.freebsd.org/changeset/base/291911
> >>
> >> Log:
> >>    Fix panic on shutdown due to iscsi event priority
> >>    
> >>    iscsi's shutdown_pre_sync prio was SHUTDOWN_PRI_FIRST which caused it to
> >>    run before other high priority handlers such as filesystems e.g. ZFS.
> >>    
> >>    This meant the iscsi sessions where removed before the ZFS geom consumer
> >>    was closed, resulting in a panic from g_access calls on debug kernels
> >>    due to negative acr.
> >>    
> >>    Instead use the same as the old iscsi_initiator SHUTDOWN_PRI_DEFAULT-1
> >>    which allows it to run before dashutdown etc but after filesystems.
> > I think this might be backwards.  Have you tested the following scenario:
> >
> > 1. Establish the iSCSI session
> > 2. Mount the LUN somewhere
> > 3. Make the target not reachable (eg ifconfig down), so that the
> >     initiator tries to reconnect
> > 4. Try to shutdown?
> >
> > Basically, the point of this code is to disable reconnects when there
> > won't be any iscsid(8) instance to handle them.  The iscsi_shutdown()
> > isn't supposed to remove working iSCSI sessions.
> I didn't notice your change in head / stable/10 compared to releng, so 
> I'm testing further updates now.

Well, the difference is pretty crucial here - previously, the purpose of
iscsi_shutdown() was to disconnect the sessions during shutdown.  Now it
doesn't do that at all - established connections are kept intact, it just
disables reconnections.  And for this to work, it needs to happen as soon
as possible during shutdown.

> Currently shutdown is run too early and actually doesn't actually 
> terminate any sessions.

What does "currently" mean in this context?

> My current theory is the issue you tried to fix in 286226 is actually 
> caused by the iscsi_maintenance_thread preferring reconnect over 
> terminate, so what you where seeing was the terminate never action for 
> reconnecting sessions. I'm currently trying to validate that theory now.
> 
> If I'm right then the fix should be to revert the majority of 286226 and 
> move the processing of terminate above reconnect in 
> iscsi_maintenance_thread.

I don't think that would work.  The reason that the sessions don't
terminate (in 11-CURRENT) is that they are not supposed to - unmounting
filesystems will write data to LUNs, and we don't want to break that.
The only reason iscsi_shutdown() exists is to prevent hang on shutdown
when there are sessions that require reconnection and there is no way
for it to succeed, because of iscsid is not running.