If we spread a new implementation all over the network elements,
then we might silently break third party network elements that
is not included in apache repository. IMHO, that's the impact.
(2013/04/02 5:30), Chiradeep Vittal wrote:
Yes, please. If you add a no-op implementation for all the existing
network elements then there is no impact.
On 4/1/13 12:28 AM, "Hiroaki Kawai" <[email protected]> wrote:
On March 29, 2013, 8 p.m., Chiradeep Vittal wrote:
I do think an explicit migration interface on NetworkElement is the
right way to do it. This way, network elements can decide explicitly
when and how to handle this state.
Sprinkling
if(!nic.getReservationId().equals(context.getReservationId())){
// migration operation
return;
}
everywhere is error prone:
- Implementors of new NetworkElements are not aware of this
indirectly expressed dependency
- This snippet of code (except for the comment) does not in any way
indicate the operation.
I agree that introducing a new interface is a good solution. But the kind
of interface changes seems to happen in the next cloudstack refactoring,
so I implemented as shown not to change the interface as possible as I
can. If we add a new interface, we must spread that implementation for
that interface to every plugins anyway.
If you do want to add a new interface right now, I'll create another
patch.
- Hiroaki
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9871/#review18531
-----------------------------------------------------------
On March 29, 2013, 1:49 a.m., Hiroaki Kawai wrote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9871/
-----------------------------------------------------------
(Updated March 29, 2013, 1:49 a.m.)
Review request for cloudstack, Hugo Trippaers and Chiradeep Vittal.
Description
-------
The location of the virtual machine is provided by DeployDestination,
which will be passed in NetworkGuru#reserve and NetworkElement#prepare.
During the virtual machine migration, it actually changes
DeployDestination and it looks like that it will tell that event to
network components as it has NetworkManager#prepareNicForMigration. The
problem is that althogh the interface has that method,
NetworkManagerImpl does not tell the DeployDestination changes to
network components.
So IMHO, we need to add calls of NetworkGuru#reserve and
NetworkElement#prepare in NetworkManagerImpl#prepareNicForMigration .
And then, we also need to add calls NetworkGuru#release and
NetworkElement#release after the migration, otherwise the network
resources that plugin reserved will be kept even when the vm leaves off.
Created a first minimum patch to show the concept.
This addresses bug CLOUDSTACK-1638.
Diffs
-----
docs/en-US/plugin-niciranvp-tables.xml 4f81655
plugins/network-elements/nicira-nvp/src/com/cloud/network/NiciraNvpNicMap
pingVO.java 0779e69
plugins/network-elements/nicira-nvp/src/com/cloud/network/element/NiciraN
vpElement.java 1fcccdb
server/src/com/cloud/network/NetworkManager.java 4124b19
server/src/com/cloud/network/NetworkManagerImpl.java a98bdd4
server/src/com/cloud/network/guru/ControlNetworkGuru.java 934cd70
server/src/com/cloud/network/guru/DirectNetworkGuru.java ee824af
server/src/com/cloud/network/guru/DirectPodBasedNetworkGuru.java
354d7ed
server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java
24d24f8
server/src/com/cloud/network/guru/GuestNetworkGuru.java cebfb08
server/src/com/cloud/network/guru/PodBasedNetworkGuru.java b513325
server/src/com/cloud/network/guru/StorageNetworkGuru.java 879d0cd
server/src/com/cloud/vm/VirtualMachineManagerImpl.java 9230f4a
setup/db/create-schema.sql 5b6dc04
Diff: https://reviews.apache.org/r/9871/diff/
Testing
-------
Thanks,
Hiroaki Kawai
