Bringing this back after my refactoring efforts. Let me explain this a little bit better. The core orchestration of network elements belongs to the NetworkManager. It interacts with the orchestration database in a transactional manner. It also 'drives' the network plugins during various state transitions. The network plugins are not expected to make changes to the orchestration tables -- they only make changes to the resources they own and any data they own.
In short, the plugins are not expected to make mutating calls to the NetworkManager or directly modify the core orchestration tables. I've moved the data model queries to the NetworkModel class to make this clearer. If you follow the implement() method in NetworkManagerImpl, you can see that it (a) ensures that a source nat ip is allocated if the source nat service is enabled (b) locks the network row in the database (c) calls the plugin's implement() So, there is no need for plugins to perform tasks (a) and (b) Unfortunately the VirtualNetworkApplianceManager behaves both as a plugin and an orchestrator making it a bad example of how to develop a network plugin. HTH -- Chiradeep On 11/19/12 7:09 PM, "Chiradeep Vittal" <chiradeep.vit...@citrix.com> wrote: >Hi Hugo, > >I was refactoring some code and noticed the following: > > 1. The NiciraNvpElement is present in the checked-in components.xml.in. >This is odd since it is a plugin and should be enabled only if the >installation is using NiciraNvp > 2. In the implement method, a lock is taken against the network. First, >this is unnecessary since the outer NetworkManagerImpl.implement() >already takes this lock. Second, the Network table belongs to the Network >Manager/orchestrator and it is expected that the plugin elements keep >their hands off of these internal tables. > 3. If the implement chain throws an exception or even at a later point >in the workflow, then the shutdown() method is /may be called. Therefore >it is expected that the shutdown method is usually an undo of the >implement. However this rule is not fast ‹ if it makes sense to maintain >some state in implement(), it may be OK to not roll back that state. > >Check it out and let me know if you have any questions. >-- >Chiradeep
