Renee Danson Sommerfeld wrote: > 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. > > done. thanks!
Alan
