cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Hiroaki KAWAI <ka...@stratosphere.co.jp>
Subject Re: Review Request: (CLOUDSTACK-1638) Network plugins won't be notified VM migration.
Date Tue, 02 Apr 2013 02:22:41 GMT
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" <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