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" <ka...@stratosphere.co.jp> 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 >> >> >