On Wed, Nov 11, 2009 at 05:25:32PM +0000, Alan Maguire wrote:
> >
> As part of the fini, we generate and consume a state
> event directly, so we don't need to go through the process
> of enqueueing then dequeueing (which as you say would
> lead to things getting misordered). As a consequence, the
> finis and state events are all processed and sent prior to
> the NCP state event signifying the switch. I've expanded on
> this in the comments.

Sounds good, and the updated comments help.

> >ncu.c
> >1239: Do we need to do the ncu check on remove of a link, as well?
> >
> 
> Now that you mention it, I think in the removal case we do need
> an NCU check, but I think there's a bit more work needed here.
> If the Automatic NCP is active, there's nothing additional to do
> since we simply remove the NCUs from storage and nwamd.
> An NCU check is needed as you say though since removal
> may have other consequences (a priority group may be invalid
> post-removal etc).
> 
> If the User NCP is active however, the device may be removed
> but the link and interface NCUs remain and we need to get those
> NCUS into the uninitialized/not found state. So I've added a
> function to handle state changes for the User NCP in the case
> that NCUs exist for the added/removed device.
> 
> In the removal case, we transition to the uninitialized/not found
> state (via the  online*/not found state in  cases where the NCU
> is enabled).  In the case that the User NCP has an NCU for a
> device that  was removed in the past but has been added again, we need
> to transition from uninitialized to offline so that the NCU check
> won't skip the uninitialized NCUs. I've tested all this with hotplug
> and it works fine.

Cool, the changes look good.  Just one small nit (and this isn't really
part of your changes, but it's in that code): could you add a little
more to the comment at line 1274 to make it clear that the NCUs are being
removed from the *Automatic* NCP?  I was initially confused, thinkng that
ncph was for the active NCP, not the Automatic one.

Otherwise, the updates look good.

thanks,
renee

Reply via email to