You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by bharat kumar <bh...@citrix.com> on 2013/06/03 12:27:47 UTC
Re: Review Request: Cloudstack-2621 [Multiple_IP_Ranges] Failed to delete
guest IP range from a new subnet/C
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11435/
-----------------------------------------------------------
(Updated June 3, 2013, 10:27 a.m.)
Review request for cloudstack, Abhinandan Prateek and Koushik Das.
Changes
-------
updated the diff as per the review comments.
Description
-------
[Multiple_IP_Ranges] Failed to delete guest IP range from a new subnet/C
https://issues.apache.org/jira/browse/CLOUDSTACK-2621
This addresses bug Cloudstack-2621.
Diffs (updated)
-----
server/src/com/cloud/configuration/ConfigurationManagerImpl.java 59e70cf
server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java c71d037
Diff: https://reviews.apache.org/r/11435/diff/
Testing
-------
tested on master.
Thanks,
bharat kumar
Re: Review Request: Cloudstack-2621 [Multiple_IP_Ranges] Failed to delete
guest IP range from a new subnet/C
Posted by bharat kumar <bh...@citrix.com>.
> On June 11, 2013, 5:19 a.m., Koushik Das wrote:
> > server/src/com/cloud/configuration/ConfigurationManagerImpl.java, line 3037
> > <https://reviews.apache.org/r/11435/diff/2/?file=299906#file299906line3037>
> >
> > Instead of using _accountDao.findById(Account.ACCOUNT_ID_SYSTEM) why don't you pass the 'caller' parameter from the calling function
The ip alias is a system managed ip so think it should not belong to any of the caller's account
- bharat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11435/#review21688
-----------------------------------------------------------
On June 3, 2013, 10:27 a.m., bharat kumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11435/
> -----------------------------------------------------------
>
> (Updated June 3, 2013, 10:27 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and Koushik Das.
>
>
> Description
> -------
>
> [Multiple_IP_Ranges] Failed to delete guest IP range from a new subnet/C
> https://issues.apache.org/jira/browse/CLOUDSTACK-2621
>
>
> This addresses bug Cloudstack-2621.
>
>
> Diffs
> -----
>
> server/src/com/cloud/configuration/ConfigurationManagerImpl.java 59e70cf
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java c71d037
>
> Diff: https://reviews.apache.org/r/11435/diff/
>
>
> Testing
> -------
>
> tested on master.
>
>
> Thanks,
>
> bharat kumar
>
>
Re: Review Request: Cloudstack-2621 [Multiple_IP_Ranges] Failed to delete
guest IP range from a new subnet/C
Posted by Koushik Das <ko...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11435/#review21688
-----------------------------------------------------------
server/src/com/cloud/configuration/ConfigurationManagerImpl.java
<https://reviews.apache.org/r/11435/#comment44818>
Where is the txn started? Where is the txn marked as complete?
server/src/com/cloud/configuration/ConfigurationManagerImpl.java
<https://reviews.apache.org/r/11435/#comment44817>
nit: handel -> handle
server/src/com/cloud/configuration/ConfigurationManagerImpl.java
<https://reviews.apache.org/r/11435/#comment44833>
Move this comment to the place where the code is moved
server/src/com/cloud/configuration/ConfigurationManagerImpl.java
<https://reviews.apache.org/r/11435/#comment44835>
Instead of using _accountDao.findById(Account.ACCOUNT_ID_SYSTEM) why don't you pass the 'caller' parameter from the calling function
server/src/com/cloud/configuration/ConfigurationManagerImpl.java
<https://reviews.apache.org/r/11435/#comment44837>
result_final is set to false, but the code is still moving ahead. Previously there was a return So the remaining code should only be executed if (result == true).
- Koushik Das
On June 3, 2013, 10:27 a.m., bharat kumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11435/
> -----------------------------------------------------------
>
> (Updated June 3, 2013, 10:27 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and Koushik Das.
>
>
> Description
> -------
>
> [Multiple_IP_Ranges] Failed to delete guest IP range from a new subnet/C
> https://issues.apache.org/jira/browse/CLOUDSTACK-2621
>
>
> This addresses bug Cloudstack-2621.
>
>
> Diffs
> -----
>
> server/src/com/cloud/configuration/ConfigurationManagerImpl.java 59e70cf
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java c71d037
>
> Diff: https://reviews.apache.org/r/11435/diff/
>
>
> Testing
> -------
>
> tested on master.
>
>
> Thanks,
>
> bharat kumar
>
>
Re: Review Request: Cloudstack-2621 [Multiple_IP_Ranges] Failed to delete
guest IP range from a new subnet/C
Posted by Koushik Das <ko...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11435/#review21791
-----------------------------------------------------------
Ship it!
Ship It!
- Koushik Das
On June 12, 2013, 10:59 a.m., bharat kumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11435/
> -----------------------------------------------------------
>
> (Updated June 12, 2013, 10:59 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and Koushik Das.
>
>
> Description
> -------
>
> [Multiple_IP_Ranges] Failed to delete guest IP range from a new subnet/C
> https://issues.apache.org/jira/browse/CLOUDSTACK-2621
>
>
> This addresses bug Cloudstack-2621.
>
>
> Diffs
> -----
>
> server/src/com/cloud/configuration/ConfigurationManagerImpl.java 59e70cf
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java db4786a
>
> Diff: https://reviews.apache.org/r/11435/diff/
>
>
> Testing
> -------
>
> tested on master.
>
>
> Thanks,
>
> bharat kumar
>
>
Re: Review Request: Cloudstack-2621 [Multiple_IP_Ranges] Failed to delete
guest IP range from a new subnet/C
Posted by bharat kumar <bh...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11435/
-----------------------------------------------------------
(Updated June 17, 2013, 10:01 a.m.)
Review request for cloudstack, Abhinandan Prateek and Koushik Das.
Changes
-------
rebased with master.
Description
-------
[Multiple_IP_Ranges] Failed to delete guest IP range from a new subnet/C
https://issues.apache.org/jira/browse/CLOUDSTACK-2621
This addresses bug Cloudstack-2621.
Diffs (updated)
-----
server/src/com/cloud/configuration/ConfigurationManagerImpl.java 111586d
server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java db4786a
Diff: https://reviews.apache.org/r/11435/diff/
Testing
-------
tested on master.
Thanks,
bharat kumar
Re: Review Request: Cloudstack-2621 [Multiple_IP_Ranges] Failed to delete
guest IP range from a new subnet/C
Posted by bharat kumar <bh...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11435/
-----------------------------------------------------------
(Updated June 12, 2013, 10:59 a.m.)
Review request for cloudstack, Abhinandan Prateek and Koushik Das.
Description
-------
[Multiple_IP_Ranges] Failed to delete guest IP range from a new subnet/C
https://issues.apache.org/jira/browse/CLOUDSTACK-2621
This addresses bug Cloudstack-2621.
Diffs (updated)
-----
server/src/com/cloud/configuration/ConfigurationManagerImpl.java 59e70cf
server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java db4786a
Diff: https://reviews.apache.org/r/11435/diff/
Testing
-------
tested on master.
Thanks,
bharat kumar
Re: Review Request: Cloudstack-2621 [Multiple_IP_Ranges] Failed to delete
guest IP range from a new subnet/C
Posted by bharat kumar <bh...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11435/
-----------------------------------------------------------
(Updated June 12, 2013, 10:28 a.m.)
Review request for cloudstack, Abhinandan Prateek and Koushik Das.
Description
-------
[Multiple_IP_Ranges] Failed to delete guest IP range from a new subnet/C
https://issues.apache.org/jira/browse/CLOUDSTACK-2621
This addresses bug Cloudstack-2621.
Diffs (updated)
-----
server/src/com/cloud/configuration/ConfigurationManagerImpl.java 59e70cf
server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java db4786a
Diff: https://reviews.apache.org/r/11435/diff/
Testing
-------
tested on master.
Thanks,
bharat kumar
Re: Review Request: Cloudstack-2621 [Multiple_IP_Ranges] Failed to delete
guest IP range from a new subnet/C
Posted by bharat kumar <bh...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11435/
-----------------------------------------------------------
(Updated June 11, 2013, 11:16 a.m.)
Review request for cloudstack, Abhinandan Prateek and Koushik Das.
Description
-------
[Multiple_IP_Ranges] Failed to delete guest IP range from a new subnet/C
https://issues.apache.org/jira/browse/CLOUDSTACK-2621
This addresses bug Cloudstack-2621.
Diffs (updated)
-----
server/src/com/cloud/configuration/ConfigurationManagerImpl.java 59e70cf
server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java c71d037
Diff: https://reviews.apache.org/r/11435/diff/
Testing
-------
tested on master.
Thanks,
bharat kumar
Re: Review Request: Cloudstack-2621 [Multiple_IP_Ranges] Failed to delete
guest IP range from a new subnet/C
Posted by bharat kumar <bh...@citrix.com>.
> On June 11, 2013, 8:42 a.m., Koushik Das wrote:
> > server/src/com/cloud/configuration/ConfigurationManagerImpl.java, line 2967
> > <https://reviews.apache.org/r/11435/diff/4/?file=303425#file303425line2967>
> >
> > why not pass the 'caller' parameter to handleIpAliasDelete?
I am not using the caller in the handleIpAliasDeletion
- bharat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11435/#review21700
-----------------------------------------------------------
On June 11, 2013, 6:38 a.m., bharat kumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11435/
> -----------------------------------------------------------
>
> (Updated June 11, 2013, 6:38 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and Koushik Das.
>
>
> Description
> -------
>
> [Multiple_IP_Ranges] Failed to delete guest IP range from a new subnet/C
> https://issues.apache.org/jira/browse/CLOUDSTACK-2621
>
>
> This addresses bug Cloudstack-2621.
>
>
> Diffs
> -----
>
> server/src/com/cloud/configuration/ConfigurationManagerImpl.java 59e70cf
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java c71d037
>
> Diff: https://reviews.apache.org/r/11435/diff/
>
>
> Testing
> -------
>
> tested on master.
>
>
> Thanks,
>
> bharat kumar
>
>
Re: Review Request: Cloudstack-2621 [Multiple_IP_Ranges] Failed to delete
guest IP range from a new subnet/C
Posted by Koushik Das <ko...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11435/#review21700
-----------------------------------------------------------
server/src/com/cloud/configuration/ConfigurationManagerImpl.java
<https://reviews.apache.org/r/11435/#comment44863>
why not pass the 'caller' parameter to handleIpAliasDelete?
server/src/com/cloud/configuration/ConfigurationManagerImpl.java
<https://reviews.apache.org/r/11435/#comment44862>
why is both rollback and commit done?
- Koushik Das
On June 11, 2013, 6:38 a.m., bharat kumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11435/
> -----------------------------------------------------------
>
> (Updated June 11, 2013, 6:38 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and Koushik Das.
>
>
> Description
> -------
>
> [Multiple_IP_Ranges] Failed to delete guest IP range from a new subnet/C
> https://issues.apache.org/jira/browse/CLOUDSTACK-2621
>
>
> This addresses bug Cloudstack-2621.
>
>
> Diffs
> -----
>
> server/src/com/cloud/configuration/ConfigurationManagerImpl.java 59e70cf
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java c71d037
>
> Diff: https://reviews.apache.org/r/11435/diff/
>
>
> Testing
> -------
>
> tested on master.
>
>
> Thanks,
>
> bharat kumar
>
>
Re: Review Request: Cloudstack-2621 [Multiple_IP_Ranges] Failed to delete
guest IP range from a new subnet/C
Posted by bharat kumar <bh...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11435/
-----------------------------------------------------------
(Updated June 11, 2013, 6:38 a.m.)
Review request for cloudstack, Abhinandan Prateek and Koushik Das.
Description
-------
[Multiple_IP_Ranges] Failed to delete guest IP range from a new subnet/C
https://issues.apache.org/jira/browse/CLOUDSTACK-2621
This addresses bug Cloudstack-2621.
Diffs (updated)
-----
server/src/com/cloud/configuration/ConfigurationManagerImpl.java 59e70cf
server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java c71d037
Diff: https://reviews.apache.org/r/11435/diff/
Testing
-------
tested on master.
Thanks,
bharat kumar
Re: Review Request: Cloudstack-2621 [Multiple_IP_Ranges] Failed to delete
guest IP range from a new subnet/C
Posted by bharat kumar <bh...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11435/
-----------------------------------------------------------
(Updated June 11, 2013, 6:33 a.m.)
Review request for cloudstack, Abhinandan Prateek and Koushik Das.
Description
-------
[Multiple_IP_Ranges] Failed to delete guest IP range from a new subnet/C
https://issues.apache.org/jira/browse/CLOUDSTACK-2621
This addresses bug Cloudstack-2621.
Diffs (updated)
-----
server/src/com/cloud/configuration/ConfigurationManagerImpl.java 59e70cf
server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java c71d037
Diff: https://reviews.apache.org/r/11435/diff/
Testing
-------
tested on master.
Thanks,
bharat kumar
Re: Review Request: Cloudstack-2621 [Multiple_IP_Ranges] Failed to delete
guest IP range from a new subnet/C
Posted by bharat kumar <bh...@citrix.com>.
> On June 11, 2013, 5:22 a.m., Koushik Das wrote:
> > server/src/com/cloud/configuration/ConfigurationManagerImpl.java, line 3083
> > <https://reviews.apache.org/r/11435/diff/2/?file=299906#file299906line3083>
> >
> > if result_final == false, why is txn committed?
i missed the txn.rollback here, the txn.commit is okay.
- bharat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11435/#review21692
-----------------------------------------------------------
On June 3, 2013, 10:27 a.m., bharat kumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11435/
> -----------------------------------------------------------
>
> (Updated June 3, 2013, 10:27 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and Koushik Das.
>
>
> Description
> -------
>
> [Multiple_IP_Ranges] Failed to delete guest IP range from a new subnet/C
> https://issues.apache.org/jira/browse/CLOUDSTACK-2621
>
>
> This addresses bug Cloudstack-2621.
>
>
> Diffs
> -----
>
> server/src/com/cloud/configuration/ConfigurationManagerImpl.java 59e70cf
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java c71d037
>
> Diff: https://reviews.apache.org/r/11435/diff/
>
>
> Testing
> -------
>
> tested on master.
>
>
> Thanks,
>
> bharat kumar
>
>
Re: Review Request: Cloudstack-2621 [Multiple_IP_Ranges] Failed to delete
guest IP range from a new subnet/C
Posted by Koushik Das <ko...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11435/#review21692
-----------------------------------------------------------
server/src/com/cloud/configuration/ConfigurationManagerImpl.java
<https://reviews.apache.org/r/11435/#comment44842>
if result_final == false, why is txn committed?
- Koushik Das
On June 3, 2013, 10:27 a.m., bharat kumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11435/
> -----------------------------------------------------------
>
> (Updated June 3, 2013, 10:27 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and Koushik Das.
>
>
> Description
> -------
>
> [Multiple_IP_Ranges] Failed to delete guest IP range from a new subnet/C
> https://issues.apache.org/jira/browse/CLOUDSTACK-2621
>
>
> This addresses bug Cloudstack-2621.
>
>
> Diffs
> -----
>
> server/src/com/cloud/configuration/ConfigurationManagerImpl.java 59e70cf
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java c71d037
>
> Diff: https://reviews.apache.org/r/11435/diff/
>
>
> Testing
> -------
>
> tested on master.
>
>
> Thanks,
>
> bharat kumar
>
>