thanks for the review Renee! Renee Danson Sommerfeld wrote: > Hi Alan, > > On Tue, Nov 10, 2009 at 02:29:42PM +0000, Alan Maguire wrote: > >> webrev at: >> >> http://zhadum.east.sun.com/export/ws/amaguire/nwam1-bugs/webrev/ >> > > Just a few comments: > > nit: one of the bugids in the comment is wrong-- s/12856/12586/ > > fixed. > ncp.c > 79-81: the mention of the NCU state events triggered by the NCU > fini events worries me: won't it be the case that we get > fini events, then NCP state event (all enqueued here), > then NCU state events (enqueued when the fini events are > processed)? Seems like things are still out of order. > > On the other hand, you mention in the bug report that the > fini events create and directly process state changes, so > that would mean we would *not* end up with the state events > enqueued after the NCP state event. Assuming that's the > case, you should probably clarify that in this comment. > > 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. > 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. > 1720-1753: Looks like you don't have Anurag's changes for 12479 > in here; be sure to sync and reci before pushing. > > yeah, I was confused by this - not sure what I did to get things into this state. Thanks! updated webrev at: http://zhadum.east.sun.com/export/ws/amaguire/nwam1-bugs/webrev/ Alan
