> 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/NiciraNvpNicMappingVO.java
> 0779e69
>
> plugins/network-elements/nicira-nvp/src/com/cloud/network/element/NiciraNvpElement.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
>
>
