Re: Review Request: (CLOUDSTACK-1638) Network plugins won't be notified VM migration.

2013-03-19 Thread Chiradeep Vittal

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9871/#review18137
---


Kawai-san, it looks OK since most NetworkElement::release do not do anything, 
but if you look at the other plugins (e.g., NiciraNvp), they destroy the 
logical port on the NVP switch when release is called. Perhaps we need a new 
method in the NetworkElement interface explicitly for migration

- Chiradeep Vittal


On March 15, 2013, 6:51 a.m., Hiroaki Kawai wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/9871/
 ---
 
 (Updated March 15, 2013, 6:51 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
 -
 
   server/src/com/cloud/network/NetworkManager.java 8b6bf9a 
   server/src/com/cloud/network/NetworkManagerImpl.java ba5ab5d 
   server/src/com/cloud/vm/VirtualMachineManagerImpl.java 0aeef0e 
 
 Diff: https://reviews.apache.org/r/9871/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Hiroaki Kawai
 




Re: Review Request: (CLOUDSTACK-1638) Network plugins won't be notified VM migration.

2013-03-15 Thread Prasanna Santhanam

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9871/
---

(Updated March 15, 2013, 6:51 a.m.)


Review request for cloudstack, Hugo Trippaers and Chiradeep Vittal.


Changes
---

add Chiradeep and Hugo as reviewers


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
-

  server/src/com/cloud/network/NetworkManager.java 8b6bf9a 
  server/src/com/cloud/network/NetworkManagerImpl.java ba5ab5d 
  server/src/com/cloud/vm/VirtualMachineManagerImpl.java 0aeef0e 

Diff: https://reviews.apache.org/r/9871/diff/


Testing
---


Thanks,

Hiroaki Kawai



RE: Review Request: (CLOUDSTACK-1638) Network plugins won't be notified VM migration.

2013-03-14 Thread Animesh Chaturvedi
Hiroaki

Can you add Chiradeep and Hugo as reviewers

 -Original Message-
 From: Hiroaki Kawai [mailto:nore...@reviews.apache.org] On Behalf Of
 Hiroaki Kawai
 Sent: Tuesday, March 12, 2013 12:24 AM
 To: cloudstack; Hiroaki Kawai
 Subject: Review Request: (CLOUDSTACK-1638) Network plugins won't be
 notified VM migration.
 
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/9871/
 ---
 
 Review request for cloudstack.
 
 
 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
 -
 
   server/src/com/cloud/network/NetworkManager.java 8b6bf9a
   server/src/com/cloud/network/NetworkManagerImpl.java ba5ab5d
   server/src/com/cloud/vm/VirtualMachineManagerImpl.java 0aeef0e
 
 Diff: https://reviews.apache.org/r/9871/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Hiroaki Kawai



Review Request: (CLOUDSTACK-1638) Network plugins won't be notified VM migration.

2013-03-12 Thread Hiroaki Kawai

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9871/
---

Review request for cloudstack.


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
-

  server/src/com/cloud/network/NetworkManager.java 8b6bf9a 
  server/src/com/cloud/network/NetworkManagerImpl.java ba5ab5d 
  server/src/com/cloud/vm/VirtualMachineManagerImpl.java 0aeef0e 

Diff: https://reviews.apache.org/r/9871/diff/


Testing
---


Thanks,

Hiroaki Kawai