You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Koushik Das <ko...@citrix.com> on 2012/07/04 08:31:57 UTC

Review Request: Fix for CS-15345

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

Review request for cloudstack, Abhinandan Prateek and Alena Prokharchyk.


Description
-------


    During account cleanup, associated network is accessed without checking whether it is present or not. Added a check to use it only when present.


This addresses bug CS-15345.


Diffs
-----

  server/src/com/cloud/network/NetworkManagerImpl.java 1e677a0 

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


Testing
-------

Yes


Thanks,

Koushik Das


Re: Review Request: Fix for CS-15345

Posted by Koushik Das <ko...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5739/
-----------------------------------------------------------

(Updated July 10, 2012, 12:25 p.m.)


Review request for cloudstack, Abhinandan Prateek, Sheng Yang, and Alena Prokharchyk.


Changes
-------

During account cleanup, associated networks get cleaned up. As part of network cleanup, source nat ip was not getting released (this logic got introduced as part of a fix to persist source nat ip during network restart). Added a fix to release the source nat ip when the network gets destroyed.


Description
-------


    During account cleanup, associated network is accessed without checking whether it is present or not. Added a check to use it only when present.


This addresses bug CS-15345.


Diffs (updated)
-----

  server/src/com/cloud/network/NetworkManagerImpl.java 1e677a0 

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


Testing
-------

Yes


Thanks,

Koushik Das


Re: Review Request: Fix for CS-15345

Posted by Alena Prokharchyk <Al...@citrix.com>.
Koushik, 

I reviewed the fix, it's not correct. We can't just put "if not null"
checks, we have to figure out why network assigned to the IP Address is
Removed. 

When network is getting removed, all ip addresses associated with it have
to be released. If you see ip left around, then it's a bug in network
removal procedure. We have to fix the root cause instead of covering its
symptoms with not null check later on.

Please revert your commit a8c1568b3757bb71dc0a9d087b98ac922e6c629c and see
if you can reproduce/fix the bug in network removal flow.

BTW, you said it's related to VPC, but there was no VPC creation in the
steps to reproduce?

Thank you,
Alena.

On 7/3/12 11:31 PM, "Koushik Das" <ko...@citrix.com> wrote:



>
>
>
>
> 
>  
>   
>    
>     
>      This is an automatically generated e-mail. To reply, visit:
>      https://reviews.apache.org/r/5739/
>     
>    
>   
>   
> 
>  Review request for cloudstack, Abhinandan Prateek and Alena Prokharchyk.
>By Koushik Das.
>Description 
> 
> 
>  
>       During account cleanup, associated network is accessed without
>checking whether it is present or not. Added a check to use it only when
>present.
>  
> Testing 
> 
>  
>   Yes
>  
> 
> Bugs: 
>
>CS-15345
>
>
>Diffs 
>
> 
>* server/src/com/cloud/network/NetworkManagerImpl.java (1e677a0)
>
>View Diff <https://reviews.apache.org/r/5739/diff/>
>
>
>
>
>  
> 
>
>
>
>
>  
> 
>
>
>