cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chiradeep Vittal <Chiradeep.Vit...@citrix.com>
Subject Re: Review Request: (CLOUDSTACK-1638) Network plugins won't be notified VM migration.
Date Mon, 01 Apr 2013 20:30:52 GMT
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" <kawai@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
>> 
>>
>


Mime
View raw message