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
